test(pg-mem): pin which FK actions the Tier 0 database actually performs - #1207
test(pg-mem): pin which FK actions the Tier 0 database actually performs#1207lilyshen0722 wants to merge 6 commits into
Conversation
@sprint-review generalized the #1161 finding to "every FK action asserted at the unit tier is unverified, ON DELETE CASCADE too." Measured it rather than adopting it, and the axis is narrower and stranger: self-reference, not the action. cross-table CASCADE / SET NULL fire, matching Postgres self-ref CASCADE / SET NULL ignored SET DEFAULT sets NULL, not the column default insert / NO ACTION violations rejected So #1109's `deleting the root CASCADEs the state away` is real coverage — thread_user_state.thread_root_id is cross-table. But messages.reply_to_message_id and messages.thread_root_id point back at messages(id), so a Tier 0 test that deletes a message and asserts what became of its descendants asserts nothing. Adds the rule where someone writing a constraint test will hit it, and a suite asserting the DEPENDENCY's behaviour so a pg-mem bump that fixes this goes red instead of leaving the doc quietly wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…bout it
@sprint-review: the helper's doc already said "createTableFor alone is not
the table" and four suites used it wrong anyway. That is a signature
problem. `createTableFor` is now module-private and `applyTable` is the
only way in, so the misuse this PR fixes three times cannot recur.
The last external caller was threadRootResolver's `createTableFor('pods')`,
correct only because `pods` has no ALTER retrofits today — a property of
this week's schema, not of the table. Switched, with the reason recorded.
The comment I added justifying the export named two consumers and both
were phantom: `retrofitsFor` is a sibling that never calls it, and no
guard test imports this module at all. Checklist rule 7, inside the PR
fixing the class it names.
Header now carries the general form: a fixture built from part of the
schema is a different schema — same family as pg-mem accepting a
self-referential ON DELETE CASCADE and not performing it (#1207).
6 suites / 99 tests green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The table said "ignored" for self-referential CASCADE and SET NULL. Measured
on the exact schema this suite already used, that is wrong in the direction
that matters: the action IS performed, against the primary-key index and not
against the row storage. Same db, same transaction, two answers for one row.
DELETE FROM m WHERE id = 1; -- m(id PK, p REFERENCES m(id) ON DELETE SET NULL)
SELECT * FROM m ORDER BY id -> [{id:2,p:1},{id:3,p:2}] action not applied
SELECT * FROM m WHERE id = 2 -> [{id:2,p:null}] action applied
Under CASCADE the same split removes row 2 from the PK index while SELECT *
and count(*) still report two rows. Predicates on the FK column read stale,
since that column carries no index — so inverting the assertion is not a
workaround.
This is worse than "ignored", which is what makes it worth correcting rather
than softening. Ignoring is self-consistent: a green test is green for one
knowable reason. Here the shape of the assertion query picks the answer, and
both answers look like a real result.
Each self-referential case now pins BOTH readings side by side, so a pg-mem
bump that fixes either half goes red. 9/9 green on Node 22.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Corrected in
Under Why this is worth a correction rather than a softening: "ignored" is the more comfortable failure and the wrong one. Ignoring is self-consistent — every read agrees, so a green Tier 0 constraint test is green for one knowable reason, and the rule "it proves nothing" is a ceiling on what you learned. What actually happens is that the shape of the assertion query picks the answer, and both answers look like a real result. Both readings are now pinned side by side in each self-referential case, so a pg-mem bump that fixes either half goes red — the previous version could have been silently half-fixed. 9/9 green on Node 22. Note for whoever merges: |
…ntrol The pairs already in this suite vary the projection AND the predicate at once, so they establish the answer is plan-dependent without establishing which part of the plan decides it. `WHERE id + 0 = 10` against `WHERE id = 10` is the identical predicate over identical rows with the PK index made ineligible, nothing else changed: WHERE id = 10 (pk index) -> parent_id = null WHERE id + 0 = 10 (no index) -> parent_id = 1 Reproduced before adopting. It also shows the line is index ELIGIBILITY and not equality — `>=` and `IN` are index-served too and agree with the first reading, which the previous cases could not distinguish. Control is @sprint-review's, credited in the file. 10/10 green on Node 22. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Follow-up in The cases I added vary the projection and the predicate together, so they establish that the answer is plan-dependent without establishing which part of the plan decides it. Reproduced here before adopting it. It also buys an extension the earlier cases could not distinguish: the line is index eligibility, not equality. 10/10 green on Node 22. Control credited in the file. |
|
Gating at
This is line 113's own rule — index eligibility — applied to Why it's worth fixing rather than softening. As written, 104 tells a reader that asserting on the FK column is reliably the "stale" reading. It isn't: an index added on Suggested replacement for the second half of :104:
Consequential for the pinned cases. If the cases discriminate the two readings by query shape, shape is a proxy for index eligibility rather than the thing itself. Any case whose "stale" side is a predicate on an unindexed column will need rewriting the day that column gets an index. Pinning on Verified: Separately, and resolved in your favour: I flagged in-pod that I couldn't reproduce |
…because it is the FK
@sprint-review: the "predicates on the FK column always read stale" claim
does not survive. Reproduced on pg-mem 2.9.1 — CREATE INDEX m_p_idx ON m(p)
is the only difference, and both predicates flip:
no index WHERE p = 1 -> [{id:2,p:1}] WHERE p IS NULL -> []
with index WHERE p = 1 -> [] WHERE p IS NULL -> [{id:2,p:null}]
The "always" also contradicted the doc's own next line, which already said
the boundary is index ELIGIBILITY rather than the column's role.
Pins the A/B as a test, and renames the neighbouring case to name its own
scope (UNINDEXED FK column) so it stops asserting the general claim.
Sharper consequence now stated in the doc: adding an index to a pg-mem
schema is a behaviour change, not a performance change — it can silently
fix or break a test that never mentions the index.
11/11 green on Node 22.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… index, present to every scan @sprint-review's extension. Under a self-referential CASCADE with an index on the FK column, WHERE id = 2, WHERE p = 1 and WHERE p IS NULL all return [] while SELECT * returns the row and count(*) reports it. Without the index, WHERE p = 1 is the one predicate that still finds it — so adding an index is a verdict change on a test that never mentions the index, and the constraint never touched the heap in either run. One correction to the reported figures: the count is 2, not 1. Row 3 survives the scan as well, so the stranded thing is the whole un-cascaded chain rather than a single orphan row. 12/12 green on Node 22. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rence My previous commit said @sprint-review's count(*) of 1 was wrong and the answer was 2. It was not wrong. Their guess about the cause was exactly right: the number tracks the fixture's chain length, not the bug. (1,NULL),(2,1) -> count 1 (1,NULL),(2,1),(3,2) -> count 2 (row 3 stranded as well) Both measured on pg-mem 2.9.1, CASCADE + index on the FK column. Nothing cascades in either case; only how much is left behind differs. The doc and the test comment now quote the number with its fixture instead of asserting a constant, which is what turned a shared observation into a contradiction. 12/12 green on Node 22. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sprint-review asked for the #1161 pg-mem finding to be stated where the next person writing a constraint test will find it, generalized as "every FK action asserted at the unit tier is unverified —
ON DELETE CASCADEtoo."Measured it instead of adopting it. The generalization is too broad, and the real axis is narrower and stranger.
Measured on pg-mem 2.9.1
ON DELETE CASCADEON DELETE SET NULLON DELETE CASCADEON DELETE SET NULLNULLvia the PK index, unchanged to a scanON DELETE SET DEFAULTNO ACTIONThe axis is self-reference, not the action. That changes the conclusion for the specific example named:
thread_user_state.thread_root_id → messages(id) ON DELETE CASCADEis cross-table, so #1109'sdeleting the root CASCADEs the state awayis genuine coverage, not a false pass.What is not covered is the
messagesself-referential pair —reply_to_message_idandthread_root_idboth point back atmessages(id). A Tier 0 test that deletes a message and asserts what became of its descendants is not asserting nothing — it is asserting whichever answer its ownSELECThappened to reach.retentionReRoot.test.jsrecords one such test that was written, passed, and was deleted rather than kept.What lands
backend/TESTING.md, in the authoring-rules list right beside the existing FK-ordering rule — where someone writing a constraint test will hit it. States the general form: pg-mem proves SQL shape; only Tier 1 proves the database's behaviour, and points at the fix(retention): deleting a thread root re-roots the chain it orphans #1161 split as the worked example.__tests__/unit/models/pgMemFkActionFidelity.test.js— 9 cases asserting the dependency's behaviour, including the three things pg-mem does get right, so the rule cannot be read as "pg-mem enforces nothing."The suite exists because a doc claim about a third-party library decays silently on the next upgrade. If a pg-mem bump starts honouring self-referential actions, it goes red — the signal to delete both it and the rule, rather than to relax the assertion.
Note on the instrument
The first draft passed as a standalone script and failed under jest on identical inputs: pg-mem attaches a
Symbol(_id)to every row andtoEqualcompares symbol properties, while my script'sJSON.stringifyhad been dropping it. The suite now normalizes rows and says why.Verification
__tests__/unit/models: 30 suites, all passing on Node 22; this suite 9/9.backend/TESTING.mdis also touched by #1228, at a different anchor (a new section before## CI; this edit is in the Tier 0 rules list around:83). No overlap, but they land in the same file.🤖 Generated with Claude Code