Test/fk pk constraints by imtiazqa · Pull Request #40 · pgEdge/coldfront

imtiazqa · 2026-07-02T13:45:40Z

Tests FK constraint enforcement (outbound + inbound) on a tiered partitioned table before and after the ColdFront view swap. Verified on PG16, PG17, PG18

…_constraint

coderabbitai · 2026-07-02T13:46:02Z

Warning

Review limit reached

@vyruss, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 23 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 49a84683-8e2d-4abd-834f-4a3504f62d7c

📥 Commits

Reviewing files that changed from the base of the PR and between 224e53a and b067576.

📒 Files selected for processing (1)

ci/journey.sh

📝 Walkthrough

Walkthrough

Adds a new E2E journey story, story_fk_constraint, to ci/journey.sh that validates PostgreSQL foreign key enforcement on a partitioned tiered table before and after the archiver view swap, including inbound composite FK checks and cold-path bypass behavior, and wires it into the tiered-mode orchestration sequence.

Changes

FK Constraint Journey Story

Layer / File(s)	Summary
FK constraint story and wiring `ci/journey.sh`	New `story_fk_constraint` creates FK-backed tiered tables, seeds hot and cold rows, checks outbound and inbound FK enforcement before archiving, runs register/archive with a connection-only config, verifies behavior after the view swap, asserts cold-path acceptance for invalid inserts, cleans up created objects, and is invoked before `story_register_cli`.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and matches the PR's focus on foreign key constraint tests for the tiered table journey.
Description check	✅ Passed	The description clearly matches the changes, describing FK enforcement tests before and after the ColdFront view swap.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch test/fk-pk-constraints

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codacy-production · 2026-07-02T13:46:44Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

coderabbitai

🧹 Nitpick comments (1)

ci/journey.sh (1)

1616-1633: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Hardcoded /tmp paths for config/log files.

Static analysis flags the fixed paths (/tmp/journey-archiver.yaml, /tmp/journey-fk-reg.log, /tmp/journey-fk.yaml, /tmp/journey-fk-arch.log) as predictable temp files (CWE-377). Real-world risk is low in an isolated CI container, but using mktemp would harden against symlink/TOCTOU issues and avoid collisions if stories ever run in parallel.

♻️ Example using mktemp

-    if "$ARCHIVER" register --config /tmp/journey-archiver.yaml \
+    local reg_log; reg_log=$(mktemp)
+    if "$ARCHIVER" register --config /tmp/journey-archiver.yaml \
             --table fk_events --period monthly \
             --hot-period "${ret_days} days" \
-            >/tmp/journey-fk-reg.log 2>&1; then
+            >"$reg_log" 2>&1; then
         pass "fk_events registered (FK constraint does not block registration)"
     else
-        fail "fk_events register failed unexpectedly"; tail -5 /tmp/journey-fk-reg.log
+        fail "fk_events register failed unexpectedly"; tail -5 "$reg_log"
     fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/journey.sh` around lines 1616 - 1633, The fk_events journey step uses
hardcoded /tmp paths for both config and log files, which creates predictable
temp-file risks and potential collisions. Update the shell logic around the
ARCHIVER register/run flow to create these files with mktemp instead of fixed
names, and use the generated variables consistently in the cat, register, and
log redirection commands. Keep the existing behavior in ci/journey.sh but
replace the static /tmp/journey-fk-*.{yaml,log} references with unique temp
paths.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ci/journey.sh`:
- Around line 1616-1633: The fk_events journey step uses hardcoded /tmp paths
for both config and log files, which creates predictable temp-file risks and
potential collisions. Update the shell logic around the ARCHIVER register/run
flow to create these files with mktemp instead of fixed names, and use the
generated variables consistently in the cat, register, and log redirection
commands. Keep the existing behavior in ci/journey.sh but replace the static
/tmp/journey-fk-*.{yaml,log} references with unique temp paths.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1be5f799-5971-4d8e-a32b-e26bd8af4f18

📥 Commits

Reviewing files that changed from the base of the PR and between 19e165d and 69489b8.

📒 Files selected for processing (1)

ci/journey.sh

vyruss · 2026-07-07T11:30:54Z

Good test to have. One note: the branches that pass in both the if and the else (cold-path FK, inbound-cold, and the "no primary key" case) always go green, so they can't catch a regression. I have turned them into hard assertions of the actual behaviour.

coderabbitai

🧹 Nitpick comments (1)

ci/journey.sh (1)
1598-1615: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Predictable /tmp paths flagged by static analysis (CWE-377).

Hardcoded, predictable filenames under /tmp are susceptible to symlink/TOCTOU pre-creation by a co-resident local process. This matches the pre-existing convention used throughout the rest of the file (e.g. story_register_cli), so fixing only this new function would be inconsistent; consider a follow-up sweep to mktemp across the whole script if this risk matters in your CI environment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/journey.sh` around lines 1598 - 1615, The new FK story in ci/journey.sh
uses predictable /tmp filenames for its log and YAML files, which can be
pre-created or raced by another local process. Update this block around the
fk_events registration and archiver run to create those temp artifacts with
mktemp (or another secure temp-file helper) and reuse the generated paths for
the register and archiver commands. Keep the change scoped to the fk_events
setup, and ensure any cleanup/trap handling matches the existing journey.sh
temp-file patterns.
Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ci/journey.sh`:
- Around line 1598-1615: The new FK story in ci/journey.sh uses predictable /tmp
filenames for its log and YAML files, which can be pre-created or raced by
another local process. Update this block around the fk_events registration and
archiver run to create those temp artifacts with mktemp (or another secure
temp-file helper) and reuse the generated paths for the register and archiver
commands. Keep the change scoped to the fk_events setup, and ensure any
cleanup/trap handling matches the existing journey.sh temp-file patterns.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2782709f-6dbc-4281-8bef-9f6850461ca6

📥 Commits

Reviewing files that changed from the base of the PR and between 69489b8 and 224e53a.

📒 Files selected for processing (1)

ci/journey.sh

Muhammad Imtiaz added 3 commits July 2, 2026 16:16

test: add story_fk_constraint — FK enforcement on tiered tables

1d9ff85

test: fix story_fk_constraint — partition coverage and PK propagation

8f30754

test: document FK-on-parent-partition as known limitation in story_fk…

69489b8

…_constraint

imtiazqa requested a review from vyruss as a code owner July 2, 2026 13:45

coderabbitai Bot reviewed Jul 2, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 7, 2026

View reviewed changes

test: assert actual FK behavior in the tiered journey story

b067576

vyruss force-pushed the test/fk-pk-constraints branch from 224e53a to b067576 Compare July 7, 2026 12:09

vyruss approved these changes Jul 7, 2026

View reviewed changes

vyruss merged commit f74749f into main Jul 7, 2026
6 checks passed

vyruss deleted the test/fk-pk-constraints branch July 7, 2026 12:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Test/fk pk constraints#40

Test/fk pk constraints#40
vyruss merged 4 commits into
mainfrom
test/fk-pk-constraints

imtiazqa commented Jul 2, 2026

Uh oh!

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Uh oh!

codacy-production Bot commented Jul 2, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

vyruss commented Jul 7, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

imtiazqa commented Jul 2, 2026

Uh oh!

coderabbitai Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

codacy-production Bot commented Jul 2, 2026

Up to standards ✅

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

vyruss commented Jul 7, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading