Repository navigation
test: row_tid refuses a row number below 1, and without the floor row_tid(0) crashes the backend (#1341) - #1391
Conversation
jdatcmd
left a comment
There was a problem hiding this comment.
Reproduced at d16b9dd on PG18:
| run | shell rollup |
test_rollup.py |
|---|---|---|
| this PR | 83/83 | 2 passed |
floor removed (if (false && rowNumber < 1), src/columnar_storage.c:685) |
82 + 1 failed: `got [ |
Only this arm turns red, in both twins, so placing it last does what the PR says. The extra TESTS.md text for test_rollup_phase_one ("each group's TID range holds exactly its rows") describes a check that already exists at test_rollup.py:271. It fixes an omission.
Approving. This PR needs a rebase: main is now at f4a7591, after #1388 (ledger 2464 / 1895 / 569, cluster_tests 541) and #1390. Once it's rebased and CI is green, I'll check the patch against d16b9dd and merge.
🤖 Generated with Claude Code
d16b9dd to
3c88cbc
Compare
|
Rebased onto 57f6e0e (#1401), on top of #1403. The new head is 3c88cbc. Only the count files changed, so this needs your re-approval.
🤖 Generated with Claude Code |
|
Re-run finished on the tree after #1403 (758039f or later). Everything is green, so the head above stands:
🤖 Generated with Claude Code |
3c88cbc to
89762ba
Compare
|
Rebased onto 7da848d (#1432, #1409). The new head is 89762ba; only the count files changed, so please re-approve this head.
🤖 Generated with Claude Code |
89762ba to
857ef8a
Compare
|
Rebased onto 1f7b5da (#1411, #1412, #1417, #1418). The new head is 857ef8a. Only the count files changed, plus TESTS.md section numbers where main took 115, so please re-approve this head.
🤖 Generated with Claude Code |
|
The PG18 re-run on this head (857ef8a, on 1f7b5da with #1412's index-build change) is all green:
🤖 Generated with Claude Code |
|
Rebased onto 37ea557 (#1414, #1425). The new head is 56edb7d. Only the count files changed, so please re-approve this head.
🤖 Generated with Claude Code |
857ef8a to
56edb7d
Compare
… in both twins (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…ed (commandprompt#1341) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
|
Rebased onto f2bd5f1 (#1427). The new head is 95d5c70. Only the count files changed, so please re-approve this head.
🤖 Generated with Claude Code |
56edb7d to
95d5c70
Compare
Part of #1341.
pgcolumnar.row_tidrefuses a row number below 1 (22003, "Row numbers start at 1"), and nothing tested it. This adds the arm in both twins.The floor guards a crash, not just a wrong value
With the floor removed (
if (false && rowNumber < 1)),row_tid(0)crashes the backend (PG17). The server goes into recovery, and in the first draft, where the arm sat in the middle of the rollup suite, the checks after it failed too, one of them apremise:row. So the arm now runs:rollup.sh, andtest_rollup.py(test_row_tid_refuses_a_row_number_below_one).That way a crash takes nothing else down. Under the mutant, the shell is 82 passed + 1 failed (only this arm), and pytest fails only this test (
got '?|?|?'). The teardown error is the crash itself.Evidence
test_rollup.py2 passed, on each of PG15–19.got [||] want [22003|22003|ok]; pytestgot '?|?|?' want '22003|22003|ok'.Bookkeeping
cluster_tests539 → 540 (collection +1 against main).guard_testsis 429/429. A TESTS.md row is added.Not needed: the other half of this #1341 item
The
partition_drop_planNULL cutoff (22023) in both overloads is already asserted atpartition_lifecycle.sh:320and in its twin. It landed with #1350, after the audit was written.🤖 Generated with Claude Code