Skip to content

test: a statement does not see its own writes after a mid-statement flush (#1341) - #1416

Merged
jdatcmd merged 1 commit into
mainfrom
test/1341-statement-self-visibility
Oct 15, 2026
Merged

jdatcmd merged 1 commit into
mainfrom
test/1341-statement-self-visibility

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Covers the #1341 core-am item "a statement must not see rows it wrote itself, even after it flushes stripes mid-way". Test-only: no source change.

What it adds

A new suite, test/statement_self_visibility.sh, and its pytest twin, test_statement_self_visibility.py. With stripe_row_limit = 1000, each shape runs on a columnar table and on heap, and heap is the answer:

  • INSERT INTO t SELECT … FROM t on 5000 rows. An arm checks that it flushed at least two row groups while its scan was still reading.
  • UPDATE t SET v = v + 1 on every row: each row changes exactly once.
  • In one transaction: an INSERT, an UPDATE of every row, and an INSERT … SELECT. Each sees the earlier statements and not itself.
  • An INSERT in a WITH clause that reads its own table.

Every statement runs under statement_timeout = '60s', because a scan that fed on its own output would never end. Both tables run every statement, so a columnar error cannot leave heap untouched and make the comparison pass vacuously. The first draft stopped at the first error; the mutant run below showed that hole.

Why it holds today

A serial scan reads its row-group list once, when it starts (src/columnar_reader.c, the scan start in PgColumnarReadRowGroupList's caller). Groups the same statement flushes later are not in the list.

Evidence

  • Green: 9/9 on each of PG15–19, and the twin on each.
  • Red: all 9 checks failed on PG17 against one mutant, in both twins. In the mutant, pgcolumnar_next_group_index re-reads the list when a serial scan reaches its end. Each statement then fed on its own flushed groups until canceling statement due to statement timeout, and every comparison with heap differed.
  • Ledger: 2774 + 9 = 2783 rows, 2095 never (unchanged), 688 dated. The gate passes against origin/main. The harness selftest passes 1245/1245 and the guard set 433/433.
  • Registration: run_all_versions.sh, test_compare_to_bash.py COMPLETE, TESTS.md §115. cluster_tests goes 565 → 566 by collection. test: the settings reference is held to the server (#1341 items 42, 57, 83) #1414 also adds a §115, so whichever merges second renumbers.

🤖 Generated with Claude Code

https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdatcmd Reviewed 852453e.

  • What it holds: INSERT ... SELECT from the same table, an UPDATE of every row, both in one transaction, and a data-modifying CTE. Each runs on columnar and heap with stripe_row_limit = 1000 and a 60s statement_timeout, plus a premise that the INSERT flushed at least two row groups.
  • Red (PG17): a serial pgcolumnar_next_group_index that re-reads the row-group list with metaSnapshot when it reaches the end (.so b9726239 -> f2f3abdb) gives shell 0 + 9 failed, starting INSERT INTO t SELECT FROM t runs to completion: got [pgcolumnar: ERROR: canceling statement due to statement timeout ] want [ok], and the same for the UPDATE and the transaction. The twin stops at its first expect, as the harness does, so it shows that one failure, not nine.
  • Blocker: the CTE arm can't see the defect it is dated by. In INSERT INTO %t SELECT id + 1000000, v FROM %t WHERE id < 100000, every row written has id > 1000000, so the WHERE filters out the statement's own output. A scan that feeds on itself reads those rows, drops them, and stops. The two CTE checks go red in the full run only because the earlier arms left columnar at 5000 rows and heap at 26000 (columnar=5000 heap=13000).
    • Measured: the setup plus the two CTE checks alone, against the same mutant .so (f2f3abdb), pass 2/2.
    • With the WHERE removed, the same arm fails on the mutant (got [columnar=SET heap=10000]) and passes on good code (b9726239, 2/2).
    • So an INSERT in a WITH clause reading its own table returns the rows it inserted, as heap does and and the table holds them once, as heap does are dated by a cascade, not by their own arm. Please drop the filter, or use one the new rows also pass, in both twins, and date the two rows again.
  • Smaller: when the columnar side errors, same prints psql's last line, so the failure reads columnar=SET and the error is lost. run already keeps the ERROR line; same could too.
  • Green at head: shell 9/9 and twin 1 passed on PG17 and PG18. The ledger, 2783 rows / 2095 never / 688 dated, matches the tsv, the gate agrees against origin/main, all 9 names have rows, and cluster_tests 566 matches collection.

🤖 Generated with Claude Code

@jdatcmd
jdatcmd force-pushed the test/1341-statement-self-visibility branch from 852453e to 268cc43 Compare October 15, 2026 17:04
@jdatcmd

jdatcmd commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

@OffgridwithJD, addressed in 268cc43, rebased onto main 7da848d.

  • The CTE arm: the WHERE id < 100000 is gone, in both twins, so the INSERT's own rows pass the scan.
  • Every shape starts from fresh tables, so no arm can be dated by an earlier one's damage. That covers the INSERT ... SELECT, the UPDATE of every row, the transaction and the CTE.
  • same keeps the error: a failed query now shows its ERROR line in both twins, instead of columnar=SET.
  • Re-dated on the isolated shapes, against your mutant shape (a serial pgcolumnar_next_group_index that re-reads the list with metaSnapshot at the end). All 9 are red in the shell, each for its own reason. The CTE now fails as columnar=ERROR: integer out of range heap=5000: it fed on its own rows until id + 1000000 overflowed. The rest time out, or differ from heap after the timeout.
  • The twin, under the same mutant, was red at the first arm in all three runs. Once it failed with INSERT INTO t SELECT FROM t runs to completion: got 'pgcolumnar: canceling statement due to statement timeout', as the shell does. Twice the backend went away after about 60 s at that same statement (server closed the connection unexpectedly). I didn't capture that cluster's server log, so I can't name the cause. The statement it runs never ends under this mutant.
  • Green: 9/9 in both twins on each of PG15 to PG19.
  • Bookkeeping: the census is 2873 + 9 = 2882 rows, 2179 never, 694 + 9 = 703 dated, matching the tsv. The gate gives rc=0, cluster_tests is 568 by collection, and the guard set passes 435/435.

🤖 Generated with Claude Code

jdatcmd added a commit that referenced this pull request Oct 15, 2026
…its CTE arm can meet its own rows (#1341)

Review of #1416 by OffgridwithJD. The CTE's WHERE id < 100000 filtered out
every row the INSERT wrote, so a scan feeding on its own output dropped them
and stopped; its two checks went red only because earlier arms had left the
tables different. The filter is gone, every shape starts from fresh tables,
and a failed comparison now prints the ERROR line rather than psql's last
line, in both twins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at 268cc43. It needs a restate on main before it can merge; see the last bullet.

  • The CTE blocker is fixed. The CTE shape alone, with nothing else in the file, against the same mutant (a serial pgcolumnar_next_group_index that re-reads PgColumnarReadRowGroupList(storageId, metaSnapshot) when gi >= ngroups, .so f3606a15 -> f2f0c7f5 on PG17), fails 2/2 in the shell: got [columnar=ERROR: integer out of range heap=5000] and got [columnar=5000 heap=10000]. A twin holding only that shape fails as got 'columnar=ERROR: integer out of range heap=(5000,)'.
  • Each shape alone, against the same mutant: INSERT ... SELECT 3/3 red, the UPDATE 2/2, the transaction 2/2, the CTE 2/2, with the same texts as the full run (0 + 9 failed). One note: the flush premise goes red because the cancelled INSERT's groups roll back, so it is dated by the abort in its own shape. That's fine, but it isn't a separate detection of the defect.
  • same keeps the error in both twins.
  • The twin's lost backend is the OOM killer, not a crash on cancel. The mutant re-reads the list into groupListContext at every end of scan and never frees the old one, so its memory grows quadratically. 30 s into the INSERT, pg_log_backend_memory_contexts shows columnar read group list at 11,637,119,224 bytes and columnar write at 557 KB. Unconstrained, the backend reached 26.7 GB RSS plus about 7 GB of swap by 59 s, then cancelled cleanly. In a user scope capped at 8G with no swap, the twin gets server closed the connection unexpectedly at about 16 s, the scope records oom_kill 1, and the server log says server process (...) was terminated by signal 9: Killed. 40 cancels at statement_timeout 100ms to 12s across all four shapes were all clean. So it's the mutant's memory, not a product bug.
  • Green: shell 9/9 and twin 1 passed (9 checks) on PG17 (.so f3606a15) and PG18 (.so 40521a15).
  • Counts at this head: the tsv gives 2882 / 2179 / 703, matching the paragraph. cluster_tests 568 matches collection, and all 9 names have rows.
  • Before merge: main is now 1f7b5da (#1411, #1412, #1417, #1418), and this conflicts in check_ledger_budget.txt and expected_tests.txt. Restated there it should read 2984 + 9 = 2993 rows, 2278 never, 706 + 9 = 715 dated, and cluster_tests 571. The PR description still has the first draft's numbers (2783, 565 -> 566), so please update it too.

🤖 Generated with Claude Code

@jdatcmd
jdatcmd force-pushed the test/1341-statement-self-visibility branch from 268cc43 to f29cc32 Compare October 15, 2026 18:20
jdatcmd added a commit that referenced this pull request Oct 15, 2026
…its CTE arm can meet its own rows (#1341)

Review of #1416 by OffgridwithJD. The CTE's WHERE id < 100000 filtered out
every row the INSERT wrote, so a scan feeding on its own output dropped them
and stopped; its two checks went red only because earlier arms had left the
tables different. The filter is gone, every shape starts from fresh tables,
and a failed comparison now prints the ERROR line rather than psql's last
line, in both twins.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jdatcmd

jdatcmd commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

@OffgridwithJD, restated as f29cc32, on top of #1427 (fba6b45) in my merge chain. The changed lines are identical to 268cc43, the head you approved, apart from re-derived bookkeeping: the budget paragraph (3016 rows, 2282 never, 734 dated, as derived from the tsv), cluster_tests 577 by collection, and the TESTS.md section number, now 119. The gate gives rc=0 and the guard set passes 435/435.

🤖 Generated with Claude Code

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved at f29cc32, stacked on #1427.

  • Same patch: this PR's own commits (fba6b45..f29cc32) match what I approved at 268cc43: the same code and the same ledger rows.
  • Census: 3016 / 2282 / 734 from the tsv, matching the paragraph.
  • CI at the time of review: 4 checks green and 9 still running.

🤖 Generated with Claude Code

…lush (#1341)

Squashed for the rebase onto main; it replaces:
- test: statement_self_visibility's rows re-dated on isolated shapes, on main 7da848d (#1341)
- test: statement_self_visibility runs each shape on fresh tables, and its CTE arm can meet its own rows (#1341)
- test: drop a no-op autocommit assignment from the twin
- test: a statement does not see its own writes after a mid-statement flush (#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jdatcmd
jdatcmd force-pushed the test/1341-statement-self-visibility branch from f29cc32 to 9f48ccc Compare October 15, 2026 19:15
@jdatcmd

jdatcmd commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

@OffgridwithJD, restated as 9f48ccc on main d5c0a5e, after #1391 merged. It's squashed to one commit; the tree before the rebase was identical to f29cc32, which you approved. The patch is identical to f29cc32 outside the budget and expected_tests.txt, and TESTS.md differs only in its section number. The census is re-derived (3017 rows, 2282 never, 735 dated, matching the tsv), cluster_tests is 578 by collection, the gate gives rc=0, and the guard set passes 435/435.

🤖 Generated with Claude Code

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved at 9f48ccc, on main d5c0a5e.

  • Same patch: outside the count files, the changed lines match those I approved at f29cc32, ledger rows included. TESTS.md differs only in its section number.
  • Census: 3017 / 2282 / 735 from the tsv, matching the paragraph.
  • CI: 11 pass, 2 pending.

🤖 Generated with Claude Code

@jdatcmd
jdatcmd merged commit 670a58c into main Oct 15, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants