From e8a1af59dfeb161c30b1efb6c08537d64489ba89 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 01:20:33 +0000 Subject: [PATCH 1/8] fix: REINDEX reclaims the entries of deleted rows no snapshot can see (#1407) older snapshot still finds a recently deleted row by a new index. But it emitted every deleted row, where heap's build skips a tuple dead to all. So REINDEX, documented as what reclaims stale entries, reclaimed none: 9 rows in 10 deleted, and the rebuilt index kept its size where heap's shrank to a tenth. The build now also reads each row group's delete marks under a horizon snapshot (xmin = xmax = the oldest non-removable xid, curcid 0), which sees exactly the transactions committed before every running snapshot began, and leaves out a row deleted there. The snapshot is registered: PG 18+ asserts it in the visibility check. am_contracts gains the arm in both twins, red on main (kept:2260992-> 2260992); with the horizon moved to the next xid, the older-snapshot arm goes red instead. limitations.md says when REINDEX keeps an entry. cluster_tests +1. Fixes #1407. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- docs/limitations.md | 4 +++- src/columnar_reader.c | 37 ++++++++++++++++++++++++++++++++ src/columnar_reader.h | 1 + src/columnar_tableam.c | 22 +++++++++++++++++++ test/am_contracts.sh | 18 ++++++++++++++++ test/check_ledger.tsv | 1 + test/check_ledger_budget.txt | 10 +++++++++ test/pytest/TESTS.md | 1 + test/pytest/expected_tests.txt | 4 +++- test/pytest/test_am_contracts.py | 28 ++++++++++++++++++++++++ 10 files changed, 124 insertions(+), 2 deletions(-) diff --git a/docs/limitations.md b/docs/limitations.md index 9bd7666fb..edcaa84cd 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -779,7 +779,9 @@ therefore still be cheaper as a scan. ## Indexes - Stale index entries left by deletes and updates are filtered on fetch and - reclaimed by `REINDEX`, not removed opportunistically. + reclaimed by `REINDEX`, not removed opportunistically. `REINDEX` keeps the entry + of a deleted row only while a transaction that began before the delete is still + running, as it does on heap. - `CREATE INDEX CONCURRENTLY` (the concurrent validate path) and partial block-range index builds are not supported. diff --git a/src/columnar_reader.c b/src/columnar_reader.c index 2c9b0d003..6fa69e843 100644 --- a/src/columnar_reader.c +++ b/src/columnar_reader.c @@ -296,6 +296,15 @@ struct PgColumnarReadState bool emitDeleted; bool lastRowDeleted; + /* + * With emitDeleted, a row deleted under THIS snapshot too is dead to every + * transaction, and is not returned (#1407). The index build passes one that + * sees only transactions older than every running snapshot. + */ + Snapshot deadHorizon; + char *deadMask; + uint32 deadMaskLen; + MemoryContext readContext; /* whole scan */ MemoryContext stripeContext; /* reset per stripe */ MemoryContext groupContext; /* reset per chunk group (decompressed) */ @@ -2833,6 +2842,17 @@ pgcolumnar_native_load_group(PgColumnarReadState *rs) &rs->nativeDeleteMask, &rs->nativeDeleteMaskLen); } + rs->deadMask = NULL; + rs->deadMaskLen = 0; + if (rs->deadHorizon != NULL && rs->nativeDeleteMask != NULL) + { + List *deadList = PgColumnarReadDeleteVectorList(rs->storageId, + rg->groupNumber, + rs->deadHorizon); + + pgcolumnar_merge_delete_vectors(deadList, (uint32) ((rg->rowCount + 7) / 8), + &rs->deadMask, &rs->deadMaskLen); + } /* * The chunk metadata is read before the data (#338) because it carries the @@ -3458,6 +3478,9 @@ pgcolumnar_native_next_row(PgColumnarReadState *rs, Datum *values, bool *nulls, if (deleted && !rs->emitDeleted) continue; /* row deleted: cursors advanced, do not emit */ + if (deleted && rs->deadMask != NULL && + dv_row_deleted(rs->deadMask, rs->deadMaskLen, rs->rowInGroup - 1)) + continue; /* dead to every transaction: no snapshot needs it */ rs->lastRowDeleted = deleted; return true; @@ -3479,6 +3502,20 @@ PgColumnarReadSetEmitDeleted(PgColumnarReadState *readState, bool emit) readState->emitDeleted = emit; } +/* + * PgColumnarReadSetDeadHorizon + * With PgColumnarReadSetEmitDeleted, leave out the deleted rows that are + * deleted under `horizon` too (#1407). The index build passes a snapshot that + * sees only transactions older than every running snapshot, so a row deleted + * there is dead to all and needs no index entry, as heap's build skips a dead + * tuple. Without it every deleted row was indexed, and REINDEX reclaimed none. + */ +void +PgColumnarReadSetDeadHorizon(PgColumnarReadState *readState, Snapshot horizon) +{ + readState->deadHorizon = horizon; +} + bool PgColumnarReadLastRowDeleted(PgColumnarReadState *readState) { diff --git a/src/columnar_reader.h b/src/columnar_reader.h index c211ddd1d..5feac316d 100644 --- a/src/columnar_reader.h +++ b/src/columnar_reader.h @@ -45,6 +45,7 @@ extern bool PgColumnarReadFoldColumn(PgColumnarReadState *readState, int attidx, int16 *attlen, const uint32 **vecRawLen); extern void PgColumnarReadSetEmitDeleted(PgColumnarReadState *readState, bool emit); +extern void PgColumnarReadSetDeadHorizon(PgColumnarReadState *readState, Snapshot horizon); extern bool PgColumnarReadLastRowDeleted(PgColumnarReadState *readState); extern void PgColumnarReadSetProjection(PgColumnarReadState *readState, Bitmapset *projectedColumns); diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index d2e0b8101..6c7d94207 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -32,6 +32,7 @@ #include "catalog/storage.h" #include "commands/defrem.h" #include "commands/extension.h" +#include "storage/procarray.h" #include "commands/trigger.h" #include "catalog/dependency.h" #include "access/htup_details.h" @@ -2419,6 +2420,8 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel, IndexBuildCallback callback, void *callback_state, TableScanDesc scan) { + SnapshotData deadHorizon; + Snapshot deadHorizonReg; PgColumnarReadState *readState; bool ownReadState; Bitmapset *projected; @@ -2547,6 +2550,24 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel, /* deleted rows too, flagged, so the build can index them as not alive (#1341) */ PgColumnarReadSetEmitDeleted(readState, true); + /* + * BUT NOT A ROW DEAD TO EVERY TRANSACTION (#1407). The horizon sees exactly the + * transactions committed before every running snapshot began, so a row deleted + * under it is one no snapshot can see, and heap's build skips the like. With + * every deleted row indexed, REINDEX -- documented as what reclaims stale + * entries -- reclaimed none: 9 rows in 10 deleted, and the rebuilt index kept + * its size where heap's shrank to a tenth. curcid 0 keeps this transaction's + * own writes out of it; a row it deleted is not dead to all anyway. + */ + memset(&deadHorizon, 0, sizeof(deadHorizon)); + deadHorizon.snapshot_type = SNAPSHOT_MVCC; + deadHorizon.xmin = GetOldestNonRemovableTransactionId(table_rel); + deadHorizon.xmax = deadHorizon.xmin; + deadHorizon.curcid = FirstCommandId; + /* registered: a visibility check asserts it (PG 18+), and the copy outlives this frame */ + deadHorizonReg = RegisterSnapshot(&deadHorizon); + PgColumnarReadSetDeadHorizon(readState, deadHorizonReg); + /* * Say which branch ran and how wide the reader it produced will actually * read, so a test can assert the projection NARROWED rather than infer it @@ -2624,6 +2645,7 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel, */ if (scan != NULL) table_endscan(scan); + UnregisterSnapshot(deadHorizonReg); return reltuples; } diff --git a/test/am_contracts.sh b/test/am_contracts.sh index 1ebb8c9a6..900d6a514 100755 --- a/test/am_contracts.sh +++ b/test/am_contracts.sh @@ -309,4 +309,22 @@ psql_run "INSERT INTO ibu VALUES (5);" check "a unique index builds over a key whose other holder is a deleted row" \ "$(state "CREATE UNIQUE INDEX ibu_k ON ibu (k);")|$(q "SET enable_seqscan = off; SELECT count(*) FROM ibu WHERE k = 5;" | tail -1)" "ok|1" +# But only rows some snapshot can still see (#1407). Every deleted row went in, so +# REINDEX, documented as what reclaims stale entries, reclaimed none: 9 rows in 10 +# deleted, and the rebuilt index kept its size where heap's shrank to a tenth. +for am in pgcolumnar heap; do + psql_run "CREATE TABLE ix_$am (a int) USING $am; INSERT INTO ix_$am SELECT g FROM generate_series(1, 100000) g; + CREATE INDEX ix_${am}_a ON ix_$am (a); DELETE FROM ix_$am WHERE a % 10 <> 0;" +done +ix_reclaim() { # ix_reclaim AM -> reclaimed, when REINDEX takes the index under half its size + local before after + before="$(q "SELECT pg_relation_size('ix_${1}_a')")" + psql_run "REINDEX INDEX ix_${1}_a;" + after="$(q "SELECT pg_relation_size('ix_${1}_a')")" + [ -n "$before" ] && [ -n "$after" ] && [ "$after" -lt $((before / 2)) ] && echo reclaimed || echo "kept:$before->$after" +} +check "REINDEX after a committed delete of most rows reclaims their entries, as heap's does" \ + "$(ix_reclaim pgcolumnar)|$(ix_reclaim heap)|$(q "SET enable_seqscan = off; SELECT count(*) FROM ix_pgcolumnar WHERE a > 0;" | tail -1)" \ + "reclaimed|reclaimed|10000" + pgc_summary diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index f5b5c01b4..234896d0a 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1,6 +1,7 @@ am_contracts am_contracts ON CONFLICT DO NOTHING skips the duplicate and inserts the new row 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 am_contracts am_contracts ON CONFLICT DO UPDATE is refused as unsupported (0A000), and changes nothing 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 am_contracts am_contracts ON CONFLICT DO UPDATE with no conflicting row inserts it 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 +am_contracts am_contracts REINDEX after a committed delete of most rows reclaims their entries, as heap's does 15;16;17;18;19 2026-10-15 the index build indexing every deleted row (main before #1407's fix) am_contracts am_contracts a DO NOTHING racing an uncommitted insert of the same key waits, then skips it 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 am_contracts am_contracts a TRUNCATE rolled back at transaction level restores every row 15;16;17;18;19 2026-10-14 TRUNCATE empties the old storage in place (RelationTruncate before the swap) am_contracts am_contracts a TRUNCATE rolled back to a savepoint restores every row, and the transaction goes on 15;16;17;18;19 2026-10-14 TRUNCATE empties the old storage in place (RelationTruncate before the swap) diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 3a800607d..ba141ef3f 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2307,4 +2307,14 @@ suites_not_covered 231 # never 2179 + 93 = 2272 # dated 694 + 1 = 695 # 2272 + 695 == 2967 +# +# #1407, REINDEX reclaims the entries of rows no snapshot sees (jdatcmd): 2272 -> +# 2272. am_contracts gains one check from five-major runs (28/28 on each of 15-19), +# dated 695 + 1 = 696 +# its size where heap's shrank to a tenth. +# +# ledger rows 2967 + 1 = 2968 +# never 2272 + 0 = 2272 +# dated 695 + 1 = 696 +# 2272 + 696 == 2968 checks_never_observed_red 2272 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 0e597cec2..896de9e4e 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -6918,6 +6918,7 @@ THE WITHDRAWAL IS REACHED ONLY BY A RACE. When one session holds a key uncommitt | `test_a_read_inside_an_aborted_inner_block_keeps_the_outer_statements_rows` | a read of the table inside an EXCEPTION block writes out the enclosing INSERT's or DELETE's buffered rows and marks, and keeps them until the block resolves (#1360). On abort the rows, the read's own count and the index's view all match heap; so do a read in a nested block that commits into an aborted one, and a block that deletes a row by index before its read. A block that commits keeps every outer row once. The cost is linear: at twice the rows, the file is under 2.5 times the size; and one read early in a 5000-row insert leaves at most 3 row groups | | `test_an_insert_racing_an_uncommitted_delete_of_its_key` | an insert of a key whose delete (or key-changing update) is uncommitted waits, then fails with 23505 if the delete rolls back and succeeds if it commits: never two rows with the key. Before #1341's fix the rollback left two | | `test_an_index_built_after_a_delete_serves_an_older_snapshot` | a REPEATABLE READ snapshot from before a committed DELETE still finds the row through an index built after it (an Index Scan, asserted), and a new snapshot does not; a UNIQUE index still builds over a key whose other holder is deleted. Before #1341's fix the old snapshot's lookup found 0 | +| `test_reindex_reclaims_the_entries_of_rows_no_snapshot_sees` | after a committed delete of 9 rows in 10, with no older snapshot, `REINDEX` takes the index under half its size, as it does on heap, and the index still finds the 10,000 live rows (#1407) | ## 111. test_objstore_credentials.py: object-store credentials live on a user mapping, and the environment is a privilege diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index a7c06924a..2f717dc17 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -675,4 +675,6 @@ guard_tests 435 # over the cluster files on this tree. # cluster_tests 567 -> 568 for #1341's test_a_float_sum_that_overflows_raises_as_core_does in # test_native_groupagg.py, by collection over the cluster files on this tree. -cluster_tests 568 +# cluster_tests 568 -> 569 for #1407's test_reindex_reclaims_the_entries_of_rows_no_snapshot_sees in +# test_am_contracts.py, by collection over the cluster files on this tree. +cluster_tests 569 diff --git a/test/pytest/test_am_contracts.py b/test/pytest/test_am_contracts.py index ca4a275d7..ce881ea4a 100644 --- a/test/pytest/test_am_contracts.py +++ b/test/pytest/test_am_contracts.py @@ -418,3 +418,31 @@ def test_an_index_built_after_a_delete_serves_an_older_snapshot(pgc_cluster, pgc five = cur.fetchone()[0] cur.execute("RESET enable_seqscan") expect.text(f"{built}|{five}", "ok|1", "a unique index builds over a key whose other holder is a deleted row") + + +def test_reindex_reclaims_the_entries_of_rows_no_snapshot_sees(pgc_conn, expect): + """An index build holds deleted rows only while some snapshot can still see them + (#1407). It held every deleted row, so REINDEX, documented as what reclaims stale + entries, reclaimed none: 9 rows in 10 deleted, and the rebuilt index kept its size + where heap's shrank to a tenth. Heap is run alongside as the control.""" + with pgc_conn.cursor() as cur: + for am in ("pgcolumnar", "heap"): + cur.execute(f"CREATE TABLE ix_{am} (a int) USING {am}") + cur.execute(f"INSERT INTO ix_{am} SELECT g FROM generate_series(1, 100000) g") + cur.execute(f"CREATE INDEX ix_{am}_a ON ix_{am} (a)") + cur.execute(f"DELETE FROM ix_{am} WHERE a % 10 <> 0") + + def reclaim(am): + before = _one(pgc_conn, f"SELECT pg_relation_size('ix_{am}_a')") + with pgc_conn.cursor() as cur: + cur.execute(f"REINDEX INDEX ix_{am}_a") + after = _one(pgc_conn, f"SELECT pg_relation_size('ix_{am}_a')") + return "reclaimed" if after < before // 2 else f"kept:{before}->{after}" + + with pgc_conn.cursor() as cur: + col, heap = reclaim("pgcolumnar"), reclaim("heap") + cur.execute("SET enable_seqscan = off") + found = _one(pgc_conn, "SELECT count(*) FROM ix_pgcolumnar WHERE a > 0") + cur.execute("RESET enable_seqscan") + expect.text(f"{col}|{heap}|{found}", "reclaimed|reclaimed|10000", + "REINDEX after a committed delete of most rows reclaims their entries, as heap's does") From 7aa53fa6a28eefd8142f547c350bdcc896d098eb Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 03:12:01 +0000 Subject: [PATCH 2/8] test: am_contracts' older-snapshot arm waits for its events instead of sleeping The arm from #1364 held a REPEATABLE READ snapshot, slept four seconds, and assumed the DELETE and CREATE INDEX finished inside them. On a loaded box they did not: the plan was taken before the index existed, and 'premise: the older snapshot's lookup is an index scan' read 0 on PG16. A 5-second delay before CREATE INDEX reproduces it on PG17 every time. Now a gate session holds an advisory lock. Session 1 takes its snapshot, names itself, and queues for the lock. The script waits for the name, deletes, builds the index, then ends the gate. With the same delay the suite passes 28/28. A new premise in both twins says the older snapshot counted all 10000 rows, so it predates the delete. The arm still goes red when the index build skips deleted rows again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- test/am_contracts.sh | 28 ++++++++++++++++++++++++++-- test/check_ledger.tsv | 1 + test/check_ledger_budget.txt | 13 ++++++++++++- test/pytest/test_am_contracts.py | 2 ++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/test/am_contracts.sh b/test/am_contracts.sh index 900d6a514..2a7c38bad 100755 --- a/test/am_contracts.sh +++ b/test/am_contracts.sh @@ -280,17 +280,41 @@ check "an insert of a key an uncommitted UPDATE moved away fails when the update # as on heap, which indexes rows not yet dead to every snapshot. psql_run "CREATE TABLE ib (id int, v int) USING pgcolumnar;" psql_run "INSERT INTO ib SELECT g, g FROM generate_series(1, 10000) g;" +# ORDERED BY EVENTS, NOT SLEEPS. Session 1 used to sleep four seconds and assume the +# DELETE and the index build fitted inside them; on a loaded box they did not, the +# plan was taken before the index existed, and the premise read 0 (PG16, 2026-10-15). +# Now a gate session holds an advisory lock. Session 1 takes its snapshot, names +# itself, and queues for that lock; the script waits for the name, deletes, builds, +# and only then ends the gate, so session 1 plans after the index exists. _ib_out="$PGC_WORKDIR/ib_rr.out" +ib_wait_for() { # ib_wait_for APPLICATION_NAME -> its pid, once it has named itself + local i pid="" + for i in $(seq 1 300); do + pid="$(q "SELECT pid FROM pg_stat_activity WHERE application_name = '$1'")" + [ -n "$pid" ] && break + sleep 0.1 + done + echo "$pid" +} +env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \ + -c "SELECT pg_advisory_lock(4242); SET application_name = 'ib_gate'; SELECT pg_sleep(600);" >/dev/null 2>&1 & +_ib_gate=$! +_ib_gate_pid="$(ib_wait_for ib_gate)" env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq -v ON_ERROR_STOP=1 \ - -c "BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT count(*) FROM ib; SELECT pg_sleep(4); + -c "BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT count(*) FROM ib; SET application_name = 'ib_snap'; + SELECT pg_advisory_lock_shared(4242); SET LOCAL enable_seqscan = off; SET LOCAL enable_bitmapscan = off; EXPLAIN (COSTS OFF) SELECT count(*) FROM ib WHERE id = 4242; SELECT 'rows:' || count(*) FROM ib WHERE id = 4242; COMMIT;" > "$_ib_out" 2>&1 & _ib_s1=$! -sleep 1 +_ib_snap_pid="$(ib_wait_for ib_snap)" psql_run "DELETE FROM ib WHERE id = 4242;" psql_run "CREATE INDEX ib_id ON ib (id);" +q "SELECT pg_terminate_backend(${_ib_gate_pid:-0})" >/dev/null +wait "$_ib_gate" 2>/dev/null wait "$_ib_s1" +check "premise: the older snapshot was taken before the delete" \ + "$([ -n "$_ib_gate_pid" ] && [ -n "$_ib_snap_pid" ] && head -1 "$_ib_out" || echo "no (gate ${_ib_gate_pid:-none}, snapshot ${_ib_snap_pid:-none})")" "10000" check "premise: the index exists and is valid" \ "$(q "SELECT indisvalid FROM pg_index WHERE indexrelid = 'ib_id'::regclass")" "t" check "premise: the older snapshot's lookup is an index scan" \ diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 234896d0a..ecf5fdc39 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -30,6 +30,7 @@ am_contracts am_contracts and the primary key still refuses a duplicate of a res am_contracts am_contracts and the unique index answers the key with that one row 15;16;17;18;19 never - am_contracts am_contracts premise: the conflict table holds ten rows 15;16;17;18;19 never - am_contracts am_contracts premise: the index exists and is valid 15;16;17;18;19 never - +am_contracts am_contracts premise: the older snapshot was taken before the delete 15;16;17;18;19 never - am_contracts am_contracts premise: the older snapshot's lookup is an index scan 15;16;17;18;19 never - am_contracts am_contracts premise: the racing sessions contended for keys 15;16;17;18;19 never - am_contracts am_contracts premise: the truncate table holds 5000 rows 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index ba141ef3f..78e175337 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2317,4 +2317,15 @@ suites_not_covered 231 # never 2272 + 0 = 2272 # dated 695 + 1 = 696 # 2272 + 696 == 2968 -checks_never_observed_red 2272 +# +# am_contracts' older-snapshot arm ordered by events, not sleeps (jdatcmd): 2272 -> +# 2273. One premise enters, that the older snapshot predates the delete, from +# five-major runs (28/28 on each of 15-19). A premise is never dated, by #1236. +# The arm it guards stays dated: an index build that skips deleted rows again +# reddens it on PG17 under the new ordering. +# +# ledger rows 2968 + 1 = 2969 +# never 2272 + 1 = 2273 +# dated 696 + 0 = 696 +# 2273 + 696 == 2969 +checks_never_observed_red 2273 diff --git a/test/pytest/test_am_contracts.py b/test/pytest/test_am_contracts.py index ce881ea4a..5780897d5 100644 --- a/test/pytest/test_am_contracts.py +++ b/test/pytest/test_am_contracts.py @@ -378,6 +378,7 @@ def test_an_index_built_after_a_delete_serves_an_older_snapshot(pgc_cluster, pgc oc.execute(f'SET search_path TO "{schema}"') oc.execute("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ") oc.execute("SELECT count(*) FROM ib") + before = oc.fetchone()[0] with pgc_conn.cursor() as cur: cur.execute("DELETE FROM ib WHERE id = 4242") cur.execute("CREATE INDEX ib_id ON ib (id)") @@ -390,6 +391,7 @@ def test_an_index_built_after_a_delete_serves_an_older_snapshot(pgc_cluster, pgc old.rollback() finally: old.close() + expect.num(before, 10000, "premise: the older snapshot was taken before the delete") expect.text(str(_one(pgc_conn, "SELECT indisvalid FROM pg_index WHERE indexrelid = 'ib_id'::regclass")), "True", "premise: the index exists and is valid") expect.num(len(re.findall(r"Index (Only )?Scan using ib_id", plan)), 1, From fc6a07afd2e8e879cf9678dbad6d5bf7566d9c46 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 03:24:23 +0000 Subject: [PATCH 3/8] test: ALTER TABLE forms that read every row see the live rows and only those (#1341) A new suite, alter_table_scans, and its pytest twin. A columnar table keeps a deleted row in its file until compaction, so each form must see the live rows and only those. On a columnar table and on heap with the same rows: - SET NOT NULL, ADD PRIMARY KEY and VALIDATE CONSTRAINT are refused over a NULL, a duplicate and a breaking row (23502, 23505, 23514), and each succeeds once that row is deleted; - ALTER COLUMN TYPE and ADD COLUMN with a volatile default rewrite the table, keep the live rows and leave the deleted ones gone; - afterwards the primary key finds id 1 once, through the index. 15/15 on each of PG15-19, and the twin on each. Ten checks are dated on PG17 by two mutants: every scan returning deleted rows, and the index build marking every row alive. cluster_tests 565 -> 566 by collection. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- test/alter_table_scans.sh | 82 ++++++++++++++++++++++ test/check_ledger.tsv | 15 +++++ test/check_ledger_budget.txt | 18 ++++- test/pytest/TESTS.md | 11 +++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_alter_table_scans.py | 97 +++++++++++++++++++++++++++ test/pytest/test_compare_to_bash.py | 1 + test/run_all_versions.sh | 1 + 8 files changed, 227 insertions(+), 2 deletions(-) create mode 100755 test/alter_table_scans.sh create mode 100644 test/pytest/test_alter_table_scans.py diff --git a/test/alter_table_scans.sh b/test/alter_table_scans.sh new file mode 100755 index 000000000..57c03ba51 --- /dev/null +++ b/test/alter_table_scans.sh @@ -0,0 +1,82 @@ +#!/usr/bin/env bash +# +# ALTER TABLE forms that read every existing row through the access method (#1341): +# SET NOT NULL, ADD PRIMARY KEY, VALIDATE CONSTRAINT, and the two that rewrite the +# table, ALTER COLUMN TYPE and ADD COLUMN with a volatile default. No test ran them. +# +# A columnar table keeps a deleted row in its file until compaction, so the question +# each form has to answer is the same: does it see the live rows, and only those? +# Each form runs on a columnar table and on heap with the same rows. First a row +# violates it, and both must refuse it with the same SQLSTATE. Then that row is +# deleted, and both must accept it: a deleted NULL, duplicate or negative value +# that the scan still saw would refuse it. Each rewrite must keep the live rows and +# leave the deleted ones gone. +# +# Usage: test/alter_table_scans.sh [PG_CONFIG] +# Written fresh for pgColumnar. + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +outcome() { # outcome SQL -> ok, or the SQLSTATE the statement failed with + local out st + out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \ + -X -qtA -v ON_ERROR_STOP=1 -v VERBOSITY=sqlstate -c "$1" 2>&1)" + st="$(sed -n 's/^ERROR: *\([0-9A-Z]\{5\}\).*/\1/p' <<<"$out")" + echo "${st:-ok}" +} +both() { # both SQL -> "|", %t standing for the table + echo "$(outcome "${1//%t/at_pgcolumnar}")|$(outcome "${1//%t/at_heap}")" +} +answers() { # answers SQL -> "|", %t standing for the table + echo "$(q "${1//%t/at_pgcolumnar}" | tail -1)|$(q "${1//%t/at_heap}" | tail -1)" +} + +# 3000 ordinary rows; then a NULL in v, a second id 1, and a negative v. +for am in pgcolumnar heap; do + psql_run "CREATE TABLE at_$am (id int, v int, w text) USING $am; + INSERT INTO at_$am SELECT g, g, 'x' FROM generate_series(1, 3000) g; + INSERT INTO at_$am VALUES (5000, NULL, 'null'), (1, -1, 'dup');" +done + +# ---- each form refuses the row that violates it ---------------------------------- +check "SET NOT NULL is refused over a NULL (23502), as on heap" \ + "$(both "ALTER TABLE %t ALTER COLUMN v SET NOT NULL")" "23502|23502" +check "ADD PRIMARY KEY is refused over a duplicate (23505), as on heap" \ + "$(both "ALTER TABLE %t ADD PRIMARY KEY (id)")" "23505|23505" +check "a CHECK added NOT VALID is accepted, as on heap" \ + "$(both "ALTER TABLE %t ADD CONSTRAINT v_pos CHECK (v > 0) NOT VALID")" "ok|ok" +check "and VALIDATE CONSTRAINT is refused over the row that breaks it (23514), as on heap" \ + "$(both "ALTER TABLE %t VALIDATE CONSTRAINT v_pos")" "23514|23514" + +# ---- once those rows are deleted, each form accepts -------------------------------- +psql_run "DELETE FROM at_pgcolumnar WHERE v IS NULL OR v < 0; DELETE FROM at_heap WHERE v IS NULL OR v < 0;" +check "the deleted rows are gone from both tables, which hold the same rows" \ + "$(answers "SELECT count(*) || '/' || sum(v) FROM %t")" "3000/4501500|3000/4501500" +check "SET NOT NULL succeeds once the NULL row is deleted, as on heap" \ + "$(both "ALTER TABLE %t ALTER COLUMN v SET NOT NULL")" "ok|ok" +check "ADD PRIMARY KEY succeeds once the duplicate is deleted, as on heap" \ + "$(both "ALTER TABLE %t ADD PRIMARY KEY (id)")" "ok|ok" +check "VALIDATE CONSTRAINT succeeds once the breaking row is deleted, as on heap" \ + "$(both "ALTER TABLE %t VALIDATE CONSTRAINT v_pos")" "ok|ok" +check "and the constraint is recorded as validated" \ + "$(answers "SELECT convalidated FROM pg_constraint WHERE conrelid = '%t'::regclass AND conname = 'v_pos'")" "t|t" + +# ---- the two rewrites keep the live rows and only those ---------------------------- +check "ALTER COLUMN TYPE rewrites the table, as on heap" \ + "$(both "ALTER TABLE %t ALTER COLUMN v TYPE bigint")" "ok|ok" +check "and the rewrite kept the live rows and not the deleted ones" \ + "$(answers "SELECT count(*) || '/' || sum(v) || '/' || (SELECT format_type(atttypid, NULL) FROM pg_attribute WHERE attrelid = '%t'::regclass AND attname = 'v') FROM %t")" "3000/4501500/bigint|3000/4501500/bigint" +check "ADD COLUMN with a volatile default rewrites the table, as on heap" \ + "$(both "ALTER TABLE %t ADD COLUMN z float8 DEFAULT random()")" "ok|ok" +check "and that rewrite gave every live row a value and brought back no deleted one" \ + "$(answers "SELECT count(*) || '/' || count(z) || '/' || count(*) FILTER (WHERE v IS NULL OR v < 0) FROM %t")" \ + "3000/3000/0|3000/3000/0" +IDX="SET enable_seqscan = off; SET enable_bitmapscan = off;" +check "the lookup of id 1 is an index scan on the primary key" \ + "$(for t in at_pgcolumnar at_heap; do q "$IDX EXPLAIN (COSTS OFF) SELECT count(*) FROM $t WHERE id = 1" | grep -c "Index.* Scan using ${t}_pkey"; done | paste -sd'|')" "1|1" +check "the primary key still finds id 1 once, through the index, after both rewrites" \ + "$(answers "$IDX SELECT count(*) || '/' || min(w) FROM %t WHERE id = 1")" "1/x|1/x" + +pgc_summary diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index ecf5fdc39..33edf3635 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1,3 +1,18 @@ +alter_table_scans alter_table_scans ADD COLUMN with a volatile default rewrites the table, as on heap 15;16;17;18;19 2026-10-15 every scan returns deleted rows, the reader's delete filter disabled (src/columnar_reader.c) +alter_table_scans alter_table_scans ADD PRIMARY KEY is refused over a duplicate (23505), as on heap 15;16;17;18;19 never - +alter_table_scans alter_table_scans ADD PRIMARY KEY succeeds once the duplicate is deleted, as on heap 15;16;17;18;19 2026-10-15 the index build marks every row alive, deleted ones included (src/columnar_tableam.c) +alter_table_scans alter_table_scans ALTER COLUMN TYPE rewrites the table, as on heap 15;16;17;18;19 2026-10-15 every scan returns deleted rows, the reader's delete filter disabled (src/columnar_reader.c) +alter_table_scans alter_table_scans SET NOT NULL is refused over a NULL (23502), as on heap 15;16;17;18;19 never - +alter_table_scans alter_table_scans SET NOT NULL succeeds once the NULL row is deleted, as on heap 15;16;17;18;19 2026-10-15 every scan returns deleted rows, the reader's delete filter disabled (src/columnar_reader.c) +alter_table_scans alter_table_scans VALIDATE CONSTRAINT succeeds once the breaking row is deleted, as on heap 15;16;17;18;19 2026-10-15 every scan returns deleted rows, the reader's delete filter disabled (src/columnar_reader.c) +alter_table_scans alter_table_scans a CHECK added NOT VALID is accepted, as on heap 15;16;17;18;19 never - +alter_table_scans alter_table_scans and VALIDATE CONSTRAINT is refused over the row that breaks it (23514), as on heap 15;16;17;18;19 never - +alter_table_scans alter_table_scans and that rewrite gave every live row a value and brought back no deleted one 15;16;17;18;19 2026-10-15 every scan returns deleted rows, the reader's delete filter disabled (src/columnar_reader.c) +alter_table_scans alter_table_scans and the constraint is recorded as validated 15;16;17;18;19 2026-10-15 every scan returns deleted rows, the reader's delete filter disabled (src/columnar_reader.c) +alter_table_scans alter_table_scans and the rewrite kept the live rows and not the deleted ones 15;16;17;18;19 2026-10-15 every scan returns deleted rows, the reader's delete filter disabled (src/columnar_reader.c) +alter_table_scans alter_table_scans the deleted rows are gone from both tables, which hold the same rows 15;16;17;18;19 2026-10-15 every scan returns deleted rows, the reader's delete filter disabled (src/columnar_reader.c) +alter_table_scans alter_table_scans the lookup of id 1 is an index scan on the primary key 15;16;17;18;19 2026-10-15 the index build marks every row alive, deleted ones included (src/columnar_tableam.c) +alter_table_scans alter_table_scans the primary key still finds id 1 once, through the index, after both rewrites 15;16;17;18;19 never - am_contracts am_contracts ON CONFLICT DO NOTHING skips the duplicate and inserts the new row 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 am_contracts am_contracts ON CONFLICT DO UPDATE is refused as unsupported (0A000), and changes nothing 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 am_contracts am_contracts ON CONFLICT DO UPDATE with no conflicting row inserts it 15;16;17;18;19 2026-10-14 speculative insert refused as unsupported, as before #1341 diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 78e175337..91205e853 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2328,4 +2328,20 @@ suites_not_covered 231 # never 2272 + 1 = 2273 # dated 696 + 0 = 696 # 2273 + 696 == 2969 -checks_never_observed_red 2273 +# +# #1341, ALTER TABLE forms that read every row see the live rows and only those +# (jdatcmd): 2273 -> 2278. alter_table_scans enters with all 15 checks from +# five-major runs (15/15 on each of 15-19). Ten are dated on PG17 by two mutants: +# - every scan returns deleted rows: the deleted-rows-gone arm, the three +# acceptances after the delete but ADD PRIMARY KEY's, the validated flag, +# and both rewrites and their row arms; +# - the index build marks every row alive: ADD PRIMARY KEY after the delete, +# and the index lookup that needs that key. +# The other five arrive never: the three refusals before the delete, the NOT VALID +# add, and the final lookup. +# +# ledger rows 2969 + 15 = 2984 +# never 2273 + 5 = 2278 +# dated 696 + 10 = 706 +# 2278 + 706 == 2984 +checks_never_observed_red 2278 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 896de9e4e..4fe8466af 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -160,6 +160,7 @@ behaviour, the source of that number is named. - [112. test_debug_hook_privilege.py: the debug metadata mutators are owner-only](#112-test_debug_hook_privilegepy-the-debug-metadata-mutators-are-owner-only) - [113. test_vacuum_lock_privilege.py: maintenance refuses a non-owner before taking its lock](#113-test_vacuum_lock_privilegepy-maintenance-refuses-a-non-owner-before-taking-its-lock) - [114. test_vm_privilege.py: the visibility-map entry points are owner-only and refuse heap](#114-test_vm_privilegepy-the-visibility-map-entry-points-are-owner-only-and-refuse-heap) +- [115. test_alter_table_scans.py: ALTER TABLE forms see the live rows and only those](#115-test_alter_table_scanspy-alter-table-forms-see-the-live-rows-and-only-those) ## 1. How to read a test in here @@ -6959,3 +6960,13 @@ The pytest twin of `test/vm_privilege.sh` (#748, ported for #1341), every check | test | what it holds | | --- | --- | | `test_the_visibility_map_entry_points` | the premises, the two refusal layers and the damage they prevent, the owner controls, the heap refusals and sweep, and the coverage scan | + +## 115. test_alter_table_scans.py: ALTER TABLE forms see the live rows and only those + +The pytest twin of `test/alter_table_scans.sh` (#1341), every check by the shell's name. A columnar table keeps a deleted row in its file until compaction, so every ALTER TABLE form that reads the existing rows must see the live ones and only those. `SET NOT NULL`, `ADD PRIMARY KEY` and `VALIDATE CONSTRAINT` run on a columnar table and on heap with the same rows. First a row violates each form, and both tables refuse it with the same SQLSTATE (23502, 23505, 23514). Then that row is deleted, and both accept the form. `ALTER COLUMN TYPE` and `ADD COLUMN` with a volatile default rewrite the table: each keeps the live rows and leaves the deleted ones gone. After both rewrites the primary key still finds id 1 once, through the index. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_alter_table_forms_see_the_live_rows_and_only_those` | the three refusals and three acceptances against heap, both rewrites, and the index lookup after them | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 2f717dc17..24e5249df 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -677,4 +677,6 @@ guard_tests 435 # test_native_groupagg.py, by collection over the cluster files on this tree. # cluster_tests 568 -> 569 for #1407's test_reindex_reclaims_the_entries_of_rows_no_snapshot_sees in # test_am_contracts.py, by collection over the cluster files on this tree. -cluster_tests 569 +# cluster_tests 569 -> 570 for #1341's test_alter_table_scans.py, one test, by collection +# over the cluster files on this tree. +cluster_tests 570 diff --git a/test/pytest/test_alter_table_scans.py b/test/pytest/test_alter_table_scans.py new file mode 100644 index 000000000..42de34fbd --- /dev/null +++ b/test/pytest/test_alter_table_scans.py @@ -0,0 +1,97 @@ +"""ALTER TABLE forms that read every existing row through the access method (#1341). +The pytest twin of `test/alter_table_scans.sh`, every check by the shell's name. + +A columnar table keeps a deleted row in its file until compaction, so each form has +to see the live rows and only those. SET NOT NULL, ADD PRIMARY KEY and VALIDATE +CONSTRAINT run on a columnar table and on heap with the same rows: first a row +violates each, and both refuse it with the same SQLSTATE; then that row is deleted, +and both accept it. ALTER COLUMN TYPE and ADD COLUMN with a volatile default rewrite +the table, and must keep the live rows and leave the deleted ones gone. +""" +import psycopg + +AMS = ("pgcolumnar", "heap") + + +def _outcome(conn, sql): + try: + with conn.cursor() as cur: + cur.execute(sql) + return "ok" + except psycopg.Error as exc: + return exc.sqlstate or f"no sqlstate: {exc}" + + +def _both(conn, sql): + return "|".join(_outcome(conn, sql.replace("%t", f"at_{am}")) for am in AMS) + + +def _answers(conn, sql, setup=()): + out = [] + for am in AMS: + with conn.cursor() as cur: + for s in setup: + cur.execute(s) + cur.execute(sql.replace("%t", f"at_{am}")) + out.append("/".join(str(x) for x in cur.fetchone())) + cur.execute("RESET enable_seqscan"); cur.execute("RESET enable_bitmapscan") + return "|".join(out) + + +def test_alter_table_forms_see_the_live_rows_and_only_those(pgc_conn, expect): + pgc_conn.autocommit = True + with pgc_conn.cursor() as cur: + for am in AMS: + cur.execute(f"CREATE TABLE at_{am} (id int, v int, w text) USING {am}") + cur.execute(f"INSERT INTO at_{am} SELECT g, g, 'x' FROM generate_series(1, 3000) g") + cur.execute(f"INSERT INTO at_{am} VALUES (5000, NULL, 'null'), (1, -1, 'dup')") + + expect.text(_both(pgc_conn, "ALTER TABLE %t ALTER COLUMN v SET NOT NULL"), "23502|23502", + "SET NOT NULL is refused over a NULL (23502), as on heap") + expect.text(_both(pgc_conn, "ALTER TABLE %t ADD PRIMARY KEY (id)"), "23505|23505", + "ADD PRIMARY KEY is refused over a duplicate (23505), as on heap") + expect.text(_both(pgc_conn, "ALTER TABLE %t ADD CONSTRAINT v_pos CHECK (v > 0) NOT VALID"), "ok|ok", + "a CHECK added NOT VALID is accepted, as on heap") + expect.text(_both(pgc_conn, "ALTER TABLE %t VALIDATE CONSTRAINT v_pos"), "23514|23514", + "and VALIDATE CONSTRAINT is refused over the row that breaks it (23514), as on heap") + + with pgc_conn.cursor() as cur: + for am in AMS: + cur.execute(f"DELETE FROM at_{am} WHERE v IS NULL OR v < 0") + expect.text(_answers(pgc_conn, "SELECT count(*), sum(v) FROM %t"), "3000/4501500|3000/4501500", + "the deleted rows are gone from both tables, which hold the same rows") + expect.text(_both(pgc_conn, "ALTER TABLE %t ALTER COLUMN v SET NOT NULL"), "ok|ok", + "SET NOT NULL succeeds once the NULL row is deleted, as on heap") + expect.text(_both(pgc_conn, "ALTER TABLE %t ADD PRIMARY KEY (id)"), "ok|ok", + "ADD PRIMARY KEY succeeds once the duplicate is deleted, as on heap") + expect.text(_both(pgc_conn, "ALTER TABLE %t VALIDATE CONSTRAINT v_pos"), "ok|ok", + "VALIDATE CONSTRAINT succeeds once the breaking row is deleted, as on heap") + expect.text(_answers(pgc_conn, "SELECT convalidated FROM pg_constraint " + "WHERE conrelid = '%t'::regclass AND conname = 'v_pos'"), "True|True", + "and the constraint is recorded as validated") + + expect.text(_both(pgc_conn, "ALTER TABLE %t ALTER COLUMN v TYPE bigint"), "ok|ok", + "ALTER COLUMN TYPE rewrites the table, as on heap") + expect.text(_answers(pgc_conn, "SELECT count(*), sum(v), (SELECT format_type(atttypid, NULL) " + "FROM pg_attribute WHERE attrelid = '%t'::regclass AND attname = 'v') FROM %t"), + "3000/4501500/bigint|3000/4501500/bigint", + "and the rewrite kept the live rows and not the deleted ones") + expect.text(_both(pgc_conn, "ALTER TABLE %t ADD COLUMN z float8 DEFAULT random()"), "ok|ok", + "ADD COLUMN with a volatile default rewrites the table, as on heap") + expect.text(_answers(pgc_conn, "SELECT count(*), count(z), count(*) FILTER (WHERE v IS NULL OR v < 0) FROM %t"), + "3000/3000/0|3000/3000/0", + "and that rewrite gave every live row a value and brought back no deleted one") + + idx = ("SET enable_seqscan = off", "SET enable_bitmapscan = off") + plans = [] + for am in AMS: + with pgc_conn.cursor() as cur: + for s in idx: + cur.execute(s) + cur.execute(f"EXPLAIN (COSTS OFF) SELECT count(*) FROM at_{am} WHERE id = 1") + plan = "\n".join(r[0] for r in cur.fetchall()) + cur.execute("RESET enable_seqscan"); cur.execute("RESET enable_bitmapscan") + plans.append(str(int(f"Scan using at_{am}_pkey" in plan and "Index" in plan))) + expect.text("|".join(plans), "1|1", "the lookup of id 1 is an index scan on the primary key") + expect.text(_answers(pgc_conn, "SELECT count(*), min(w) FROM %t WHERE id = 1", idx), "1/x|1/x", + "the primary key still finds id 1 once, through the index, after both rewrites") diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 6d091fcbd..1031c529a 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -86,6 +86,7 @@ # The shape is `SHELL_REFERENCES`' in `test_harness_deps.py`, asserted in both # directions for the same reason: a one-way list rots into a permanent exemption. COMPLETE = ["advisory_lock_class", + "alter_table_scans", "fdw_plan_privilege", "am_contracts", "analyze_differential", diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 55e8b31b1..e043153e6 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -42,6 +42,7 @@ SUITES=( advisory_lock_class alter_am_cleanup alter_column_type + alter_table_scans am_contracts analyze_differential analyze_function From de6e61d10a081f1dd2161d9babd17b083e72ee8d Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 03:38:04 +0000 Subject: [PATCH 4/8] test: drop a no-op autocommit assignment from the twin pgc_conn is a write-watching wrapper, so setting .autocommit on it sets an attribute on the wrapper and never reaches the connection (found while writing #1419). The line here set it to the value it already had, so it did nothing, but it read as if it mattered. The twin still passes on PG17. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- test/pytest/test_alter_table_scans.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/pytest/test_alter_table_scans.py b/test/pytest/test_alter_table_scans.py index 42de34fbd..a58329406 100644 --- a/test/pytest/test_alter_table_scans.py +++ b/test/pytest/test_alter_table_scans.py @@ -39,7 +39,6 @@ def _answers(conn, sql, setup=()): def test_alter_table_forms_see_the_live_rows_and_only_those(pgc_conn, expect): - pgc_conn.autocommit = True with pgc_conn.cursor() as cur: for am in AMS: cur.execute(f"CREATE TABLE at_{am} (id int, v int, w text) USING {am}") From 292fa3b441cdebe7dd9046ca8de52a30d9080489 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 01:53:35 +0000 Subject: [PATCH 5/8] test: the settings reference is held to the server, and two settings are set (#1341 items 42, 57, 83) docs/configuration.md is compared with pg_settings both ways by a new suite, settings_reference, and its pytest twin: - every setting the server defines is documented, except those it marks NOT_IN_SAMPLE (developer hooks and internal switches); - every documented setting exists; - each documented default is the server's boot_val; - each row's condition (superuser, only at server start, Reloadable, or none) is the server's context. Item 57: a role that is not a superuser, on its own login, is refused enable_end_truncation with 42501, and a superuser sets it. Item 83: compression_level reaches the zstd codec, so the same text stores smaller at level 19 than at level 1. 8/8 on each of PG15-19, and the twin 4/4. All 8 checks are dated on PG17 by seven mutants (two GUC contexts, a default, try_zstd ignoring the level, and three edits to the reference). cluster_tests 564 -> 568 by collection. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- test/check_ledger.tsv | 8 ++ test/check_ledger_budget.txt | 18 ++++ test/pytest/TESTS.md | 14 +++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_compare_to_bash.py | 1 + test/pytest/test_settings_reference.py | 133 +++++++++++++++++++++++++ test/run_all_versions.sh | 1 + test/settings_reference.sh | 96 ++++++++++++++++++ 8 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 test/pytest/test_settings_reference.py create mode 100755 test/settings_reference.sh diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 33edf3635..44ebe14e8 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -2694,6 +2694,14 @@ server_file_privilege server_file_privilege the pg_write_server_files role gets server_file_privilege server_file_privilege the pg_write_server_files role gets past the gate: export_parquet 15;16;17;18;19 never - server_file_privilege server_file_privilege the pg_write_server_files role gets past the gate: parallel_export_parquet 15;16;17;18;19 never - server_file_privilege server_file_privilege the pg_write_server_files role gets past the gate: tier_partition 15;16;17;18;19 never - +settings_reference settings_reference a role that is not a superuser cannot set enable_end_truncation (42501) 15;16;17;18;19 2026-10-15 enable_end_truncation defined PGC_POSTMASTER instead of PGC_SUSET (src/columnar_tableam.c);enable_end_truncation defined PGC_USERSET instead of PGC_SUSET (src/columnar_tableam.c) +settings_reference settings_reference and a superuser can 15;16;17;18;19 2026-10-15 enable_end_truncation defined PGC_POSTMASTER instead of PGC_SUSET (src/columnar_tableam.c) +settings_reference settings_reference compression_level reaches the zstd codec: level 19 stores less than level 1 15;16;17;18;19 2026-10-15 try_zstd passes level 1 to ZSTD_compress, not the setting (src/columnar_compression.c) +settings_reference settings_reference each documented condition is the server's context for the setting 15;16;17;18;19 2026-10-15 enable_end_truncation defined PGC_POSTMASTER instead of PGC_SUSET (src/columnar_tableam.c);enable_end_truncation defined PGC_USERSET instead of PGC_SUSET (src/columnar_tableam.c);the compression_level row renamed compression_levl in docs/configuration.md +settings_reference settings_reference each documented default is the server's 15;16;17;18;19 2026-10-15 compression_level default 3 changed to 5 in the GUC and its C variable (src/columnar_tableam.c);the compression_level row renamed compression_levl in docs/configuration.md +settings_reference settings_reference every documented setting exists on the server 15;16;17;18;19 2026-10-15 the compression_level row renamed compression_levl in docs/configuration.md +settings_reference settings_reference every setting the server defines is documented, the developer hooks aside 15;16;17;18;19 2026-10-15 the backticks around every setting name removed from docs/configuration.md;the compression_level row deleted from docs/configuration.md;the compression_level row renamed compression_levl in docs/configuration.md +settings_reference settings_reference the reference documents at least thirty settings, so the comparisons below read something 15;16;17;18;19 2026-10-15 the backticks around every setting name removed from docs/configuration.md storage_row_repair storage_row_repair REFRESH MATERIALIZED VIEW repairs the view's row 15;16;17;18;19 2026-10-12 PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row storage_row_repair storage_row_repair a same-type ALTER COLUMN ... USING repairs the table's row 15;16;17;18;19 2026-10-12 PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row storage_row_repair storage_row_repair after both repairs the notes' query finds nothing 15;16;17;18;19 2026-10-12 PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row;the release notes' query is edited from NOT EXISTS to EXISTS diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 91205e853..10785ac40 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2344,4 +2344,22 @@ suites_not_covered 231 # never 2273 + 5 = 2278 # dated 696 + 10 = 706 # 2278 + 706 == 2984 +# +# #1341 items 42, 57 and 83, the settings reference held to the server: 2095 stays +# 2095. The settings_reference suite enters with 8 checks from five-major runs (8/8 +# on each of 15-19), and all 8 are dated on PG17 by seven mutants: +# - enable_end_truncation defined PGC_USERSET: the condition arm and the 42501 +# refusal go red; +# - enable_end_truncation defined PGC_POSTMASTER: the condition arm, the refusal +# (55P02) and the superuser control go red; +# - compression_level's default 3 changed to 5: the default arm goes red; +# - try_zstd passing level 1, not the setting: the level arm goes red; +# - the compression_level row deleted from the reference: the undocumented arm; +# - the row renamed compression_levl: the existence arm and three others; +# - the backticks removed from every name: the thirty-settings premise. +# +# ledger rows 2984 + 8 = 2992 +# never 2278 + 0 = 2278 +# dated 706 + 8 = 714 +# 2278 + 714 == 2992 checks_never_observed_red 2278 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 4fe8466af..225d17b24 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -161,6 +161,7 @@ behaviour, the source of that number is named. - [113. test_vacuum_lock_privilege.py: maintenance refuses a non-owner before taking its lock](#113-test_vacuum_lock_privilegepy-maintenance-refuses-a-non-owner-before-taking-its-lock) - [114. test_vm_privilege.py: the visibility-map entry points are owner-only and refuse heap](#114-test_vm_privilegepy-the-visibility-map-entry-points-are-owner-only-and-refuse-heap) - [115. test_alter_table_scans.py: ALTER TABLE forms see the live rows and only those](#115-test_alter_table_scanspy-alter-table-forms-see-the-live-rows-and-only-those) +- [116. test_settings_reference.py: the settings reference matches the server](#116-test_settings_referencepy-the-settings-reference-matches-the-server) ## 1. How to read a test in here @@ -6970,3 +6971,16 @@ The pytest twin of `test/alter_table_scans.sh` (#1341), every check by the shell | test | what it holds | | --- | --- | | `test_alter_table_forms_see_the_live_rows_and_only_those` | the three refusals and three acceptances against heap, both rewrites, and the index lookup after them | + +## 116. test_settings_reference.py: the settings reference matches the server + +The pytest twin of `test/settings_reference.sh` (#1341 items 42, 57 and 83), every check by the shell's name. It reads each `pgcolumnar.*` row of `docs/configuration.md` and compares it with `pg_settings` both ways. Every setting the server defines is documented, except the ones the server marks `NOT_IN_SAMPLE` (developer hooks and internal switches). Every documented setting exists. Each documented default is the server's `boot_val`. Each row's condition ("superuser", "only at server start", "Reloadable", or none of these) is the server's context. A role that is not a superuser, on its own login, is refused `enable_end_truncation` with 42501; a superuser sets it. `compression_level` reaches the zstd codec: the same text stores smaller at level 19 than at level 1. A build without zstd cannot run that test. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_the_reference_and_the_server_name_the_same_settings` | at least thirty documented settings, none undocumented, none documented that the server lacks | +| `test_each_documented_default_and_condition_is_the_servers` | each documented default and condition matches `boot_val` and `context` | +| `test_only_a_superuser_sets_enable_end_truncation` | the 42501 refusal on a real login, and the superuser control | +| `test_compression_level_reaches_the_zstd_codec` | level 19 stores less than level 1 | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 24e5249df..e4bf75ebc 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -679,4 +679,6 @@ guard_tests 435 # test_am_contracts.py, by collection over the cluster files on this tree. # cluster_tests 569 -> 570 for #1341's test_alter_table_scans.py, one test, by collection # over the cluster files on this tree. -cluster_tests 570 +# cluster_tests 570 -> 574 for #1341's test_settings_reference.py, four tests, by collection +# over the cluster files on this tree. +cluster_tests 574 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 1031c529a..ca5ec24d0 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -142,6 +142,7 @@ "rewrite_storage_oid", "rollup", "scan_decode_cost", + "settings_reference", "sorted_pathkeys", "stats_privilege", "storage_row_repair", diff --git a/test/pytest/test_settings_reference.py b/test/pytest/test_settings_reference.py new file mode 100644 index 000000000..7acb6b057 --- /dev/null +++ b/test/pytest/test_settings_reference.py @@ -0,0 +1,133 @@ +"""docs/configuration.md is held to the server, both ways (#1341 item 42), and the two +settings no test had set are set (items 57 and 83). The pytest twin of +`test/settings_reference.sh`, written independently: it reads the reference's +tables cell by cell, where the shell twin splits each row with awk. + +THE CONDITION. Each row says, in its description, the one thing a reader must know +before setting it: "superuser", "only at server start", or "Reloadable". Absent all +three, the setting is one any user may change. The server's `pg_settings.context` +is the truth each is compared to: superuser, postmaster, sighup, user. + +THE DEVELOPER HOOKS. A setting the server marks NOT_IN_SAMPLE (rollup_pause, +tier_fault_after, bulk_parallel_writer) is not for a reader: the reference may +document it, and then it is compared like any other, but need not. + +REAL LOGINS, NOT `SET ROLE`: a failed SET ROLE leaves the session as the superuser, +and the refusal arm would then read as allowed for the wrong reason. +""" +import pathlib +import re + +DOC = pathlib.Path(__file__).resolve().parents[2] / "docs" / "configuration.md" +PLAIN = "t_sr_plain" + + +def _documented(): + """-> {name: (default, condition)} for every `pgcolumnar.*` row of the reference.""" + rows = {} + for line in DOC.read_text().splitlines(): + if not line.startswith("| `pgcolumnar."): + continue + cells = [c.strip() for c in line.strip().strip("|").split(" | ")] + name = cells[0].strip("`") + default = re.sub(r"\s*\(empty\)$", "", cells[2]).strip("`") + if default == "''": + default = "" + if re.search(r"[Ss]uperuser", line): + cond = "superuser" + elif "only at server start" in line: + cond = "postmaster" + elif "Reloadable" in line: + cond = "sighup" + else: + cond = "user" + rows[name] = (default, cond) + return rows + + +def _server(conn): + """-> {name: (boot_val, context, public)} for every pgcolumnar.* setting.""" + with conn.cursor() as cur: + cur.execute("SELECT name, boot_val, context, " + "NOT ('NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name))) " + "FROM pg_settings WHERE name LIKE 'pgcolumnar.%'") + return {n: (b, c, p) for n, b, c, p in cur.fetchall()} + + +def _names(xs): + return " ".join(sorted(xs)) or "none" + + +def test_the_reference_and_the_server_name_the_same_settings(pgc_conn, expect): + doc, srv = _documented(), _server(pgc_conn) + expect.at_least(len(doc), 30, + "the reference documents at least thirty settings, so the comparisons below read something") + public = {n for n, (_, _, p) in srv.items() if p} + expect.text(_names(public - doc.keys()), "none", + "every setting the server defines is documented, the developer hooks aside") + expect.text(_names(doc.keys() - srv.keys()), "none", + "every documented setting exists on the server") + + +def test_each_documented_default_and_condition_is_the_servers(pgc_conn, expect): + doc, srv = _documented(), _server(pgc_conn) + both = doc.keys() & srv.keys() + expect.text(_names(f"{n}: doc {doc[n][0]!r}, server {srv[n][0]!r}" + for n in both if doc[n][0] != srv[n][0]), "none", + "each documented default is the server's") + expect.text(_names(f"{n}: doc {doc[n][1]}, server {srv[n][1]}" + for n in both if doc[n][1] != srv[n][1]), "none", + "each documented condition is the server's context for the setting") + + +def _as(cluster, role, sql): + """-> None, or the error from `sql` on a fresh login as `role`.""" + import psycopg + + dsn = f"host=127.0.0.1 port={cluster.port} user={role} dbname=postgres" + try: + with psycopg.connect(dsn, autocommit=True) as conn: + conn.execute(sql) + except psycopg.Error as exc: + return exc + return None + + +def test_only_a_superuser_sets_enable_end_truncation(pgc_cluster, pgc_conn, expect): + with pgc_conn.cursor() as cur: + cur.execute(f"SELECT 1 FROM pg_roles WHERE rolname = '{PLAIN}'") + if cur.fetchone() is None: + cur.execute(f"CREATE ROLE {PLAIN} NOSUPERUSER LOGIN") + err = _as(pgc_cluster, PLAIN, "SET pgcolumnar.enable_end_truncation = on") + if err is None: + expect.text("allowed", "42501", + "a role that is not a superuser cannot set enable_end_truncation (42501)") + else: + expect.sqlstate(err, "42501", + "a role that is not a superuser cannot set enable_end_truncation (42501)") + with pgc_conn.cursor() as cur: + cur.execute("SET pgcolumnar.enable_end_truncation = on") + cur.execute("SHOW pgcolumnar.enable_end_truncation") + expect.text(cur.fetchone()[0], "on", "and a superuser can") + + +def test_compression_level_reaches_the_zstd_codec(pgc_conn, expect): + """The same text at level 1 and level 19: the higher level stores less.""" + with pgc_conn.cursor() as cur: + cur.execute("SELECT 'zstd' = ANY (enumvals) FROM pg_settings " + "WHERE name = 'pgcolumnar.compression'") + if not cur.fetchone()[0]: + expect.cannot_run("MISSING_DEPENDENCY", "no zstd codec", + name="this build has no zstd codec") + return + size = {} + for lvl in (1, 19): + cur.execute("SET pgcolumnar.compression = zstd") + cur.execute(f"SET pgcolumnar.compression_level = {lvl}") + cur.execute(f"CREATE TABLE sr_l{lvl} (t text) USING pgcolumnar") + cur.execute(f"INSERT INTO sr_l{lvl} SELECT string_agg(md5((g * 7 + k)::text), ' ') " + "FROM generate_series(1, 20000) g, generate_series(1, 3) k GROUP BY g") + cur.execute(f"SELECT pg_relation_size('sr_l{lvl}')") + size[lvl] = cur.fetchone()[0] + expect.text("less" if size[19] < size[1] else f"{size[19]} vs {size[1]}", "less", + "compression_level reaches the zstd codec: level 19 stores less than level 1") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index e043153e6..42746c3cd 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -293,6 +293,7 @@ SUITES=( scan_decode_cost scan_direction server_file_privilege + settings_reference smoke sort_status sort_status_privilege diff --git a/test/settings_reference.sh b/test/settings_reference.sh new file mode 100755 index 000000000..f2a9bfa74 --- /dev/null +++ b/test/settings_reference.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# +# docs/configuration.md is the reference for every pgcolumnar.* setting: its name, +# type, default, and the one condition a reader must know before setting it +# (needs superuser, settable only at server start, reloadable). This suite holds +# the document to the server, both ways, so a setting added, renamed, re-defaulted +# or moved to another context without the reference following it turns red +# (#1341 item 42). It also sets the two settings no test had touched: a role that +# is not a superuser is refused enable_end_truncation (item 57), and +# compression_level reaches what zstd stores (item 83). +# +# Usage: test/settings_reference.sh [PG_CONFIG] +# Written fresh for pgColumnar. + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +DOC="$PGC_SRCDIR/docs/configuration.md" + +# doc_rows -> "name|default|condition" for each settings-table row, where condition +# is what the row's text says: superuser, postmaster (only at server start), +# sighup (reloadable), or user. +doc_rows() { + awk -F' [|] ' '/^[|] `pgcolumnar\./ { + name = $1; sub(/^[|] `/, "", name); sub(/`.*/, "", name) + def = $3; gsub(/`/, "", def); sub(/ *\(empty\)/, "", def); if (def == "'"''"'") def = "" + desc = $0; cond = "user" + if (desc ~ /[Ss]uperuser/) cond = "superuser" + else if (desc ~ /only at server start/) cond = "postmaster" + else if (desc ~ /Reloadable/) cond = "sighup" + print name "|" def "|" cond + }' "$DOC" | LC_ALL=C sort -t'|' -k1,1 +} +# server_rows [public] -> the same shape from pg_settings. With "public", a setting +# the server marks NOT_IN_SAMPLE is left out: those are developer hooks and internal +# switches (rollup_pause, tier_fault_after, bulk_parallel_writer), which the reference +# may document but need not. +server_rows() { + local only="" + [ "${1:-}" = public ] && only="AND NOT ('NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name)))" + q "SELECT name || '|' || boot_val || '|' || context FROM pg_settings + WHERE name LIKE 'pgcolumnar.%' $only ORDER BY name COLLATE \"C\";" +} +listed() { # stdin lines -> one space-separated line, or "none" when there are none + local x; x="$(tr '\n' ' ' | sed 's/ *$//')"; echo "${x:-none}" +} +docd="$(doc_rows)" +srv="$(server_rows)" +pub="$(server_rows public)" +# the server's rows for the settings the reference documents, to compare field by field +srv_docd="$(LC_ALL=C join -t'|' -o 2.1,2.2,2.3 <(cut -d'|' -f1 <<<"$docd") <(cat <<<"$srv"))" + +check_num "the reference documents at least thirty settings, so the comparisons below read something" \ + "$([ "$(grep -c . <<<"$docd")" -ge 30 ] && echo 1 || echo 0)" "1" +check "every setting the server defines is documented, the developer hooks aside" \ + "$(LC_ALL=C comm -13 <(cut -d'|' -f1 <<<"$docd") <(cut -d'|' -f1 <<<"$pub") | listed)" "none" +check "every documented setting exists on the server" \ + "$(LC_ALL=C comm -23 <(cut -d'|' -f1 <<<"$docd") <(cut -d'|' -f1 <<<"$srv") | listed)" "none" +check "each documented default is the server's" \ + "$(LC_ALL=C comm -3 <(cut -d'|' -f1,2 <<<"$docd") <(cut -d'|' -f1,2 <<<"$srv_docd") | tr -d '\t' | listed)" "none" +check "each documented condition is the server's context for the setting" \ + "$(LC_ALL=C comm -3 <(cut -d'|' -f1,3 <<<"$docd") <(cut -d'|' -f1,3 <<<"$srv_docd") | tr -d '\t' | listed)" "none" + +# ---- enable_end_truncation needs superuser (item 57) ------------------------------- +psql_run "DROP ROLE IF EXISTS sr_plain; CREATE ROLE sr_plain LOGIN NOSUPERUSER;" +as_plain() { # as_plain SQL -> ok, the SQLSTATE it fails with, or noconnect + local out rc st + out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U sr_plain -d "$PGC_DB" \ + -X -qtA -v ON_ERROR_STOP=1 -v VERBOSITY=sqlstate -c "$1" 2>&1)" + rc=$? + st="$(sed -n 's/^ERROR: *\([0-9A-Z]\{5\}\).*/\1/p' <<<"$out" | head -1)" + if [ -n "$st" ]; then echo "$st"; elif [ "$rc" = 0 ]; then echo ok; else echo noconnect; fi +} +check "a role that is not a superuser cannot set enable_end_truncation (42501)" \ + "$(as_plain "SET pgcolumnar.enable_end_truncation = on")" "42501" +check "and a superuser can" \ + "$(q "SET pgcolumnar.enable_end_truncation = on; SHOW pgcolumnar.enable_end_truncation;" | tail -1)" "on" + +# ---- compression_level reaches what zstd stores (item 83) --------------------------- +# The same rows written at level 1 and at level 19: zstd's higher level must store +# less. A build without zstd cannot run this. +if ! q "SELECT enumvals FROM pg_settings WHERE name = 'pgcolumnar.compression'" | grep -q zstd; then + pgc_skip zstd "this build has no zstd codec" +fi +for lvl in 1 19; do + psql_run "SET pgcolumnar.compression = zstd; SET pgcolumnar.compression_level = $lvl; + CREATE TABLE sr_l$lvl (t text) USING pgcolumnar; + INSERT INTO sr_l$lvl SELECT string_agg(md5((g * 7 + k)::text), ' ') + FROM generate_series(1, 20000) g, generate_series(1, 3) k GROUP BY g;" +done +check "compression_level reaches the zstd codec: level 19 stores less than level 1" \ + "$(q "SELECT CASE WHEN pg_relation_size('sr_l19') < pg_relation_size('sr_l1') THEN 'less' + ELSE pg_relation_size('sr_l19') || ' vs ' || pg_relation_size('sr_l1') END")" "less" + +pgc_summary From 342dd60047b065c80ed28feef015809bfbb5b5b9 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 02:24:14 +0000 Subject: [PATCH 6/8] test: settings_reference's premise carries its count, and no captured string is piped into grep -q (#1341) The harness selftest refused two lines. The thirty-settings premise reduced the row count to 1 or 0, so a red said nothing about how many rows were parsed (rule 540); it now reads 'no (N rows parsed from DOC)'. The zstd test piped q into grep -q, an early-exit reader (rule 080); it now asks the server whether zstd is among the enum values. The selftest was 2 failed of 1245 on PG17 and is now 0 failed; the suite is 8/8 on each of PG15-19. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- test/settings_reference.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/settings_reference.sh b/test/settings_reference.sh index f2a9bfa74..a931fc903 100755 --- a/test/settings_reference.sh +++ b/test/settings_reference.sh @@ -51,8 +51,9 @@ pub="$(server_rows public)" # the server's rows for the settings the reference documents, to compare field by field srv_docd="$(LC_ALL=C join -t'|' -o 2.1,2.2,2.3 <(cut -d'|' -f1 <<<"$docd") <(cat <<<"$srv"))" -check_num "the reference documents at least thirty settings, so the comparisons below read something" \ - "$([ "$(grep -c . <<<"$docd")" -ge 30 ] && echo 1 || echo 0)" "1" +_ndoc="$(grep -c . <<<"$docd")" +check "the reference documents at least thirty settings, so the comparisons below read something" \ + "$([ "$_ndoc" -ge 30 ] && echo yes || echo "no ($_ndoc rows parsed from $DOC)")" "yes" check "every setting the server defines is documented, the developer hooks aside" \ "$(LC_ALL=C comm -13 <(cut -d'|' -f1 <<<"$docd") <(cut -d'|' -f1 <<<"$pub") | listed)" "none" check "every documented setting exists on the server" \ @@ -80,7 +81,7 @@ check "and a superuser can" \ # ---- compression_level reaches what zstd stores (item 83) --------------------------- # The same rows written at level 1 and at level 19: zstd's higher level must store # less. A build without zstd cannot run this. -if ! q "SELECT enumvals FROM pg_settings WHERE name = 'pgcolumnar.compression'" | grep -q zstd; then +if [ "$(q "SELECT 'zstd' = ANY (enumvals) FROM pg_settings WHERE name = 'pgcolumnar.compression'")" != t ]; then pgc_skip zstd "this build has no zstd codec" fi for lvl in 1 19; do From 1e4898fd33fcb79b74e71683126dc11ec0a1ddf6 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 16:08:23 +0000 Subject: [PATCH 7/8] test: settings_reference sorts each side again after its cut, so comm names only what differs (#1341) comm compares whole lines, but the reference's rows were sorted on the name field alone. In C order `autovacuum|` sorts after `autovacuum_compact_threshold|`, so the default and condition arms fed comm unsorted input: a red arm named ten entries for one wrong row, and every run logged comm's sort warnings. Found by OffgridwithJD in review. With "Reloadable" taken out of the pgcolumnar.autovacuum row, the condition arm went from ten entries and six warnings to exactly `pgcolumnar.autovacuum|sighup pgcolumnar.autovacuum|user` and none. A wrong default on the same row names only that row. Green stays 8/8 on PG17 and PG18, with no warnings. Co-Authored-By: Claude Opus 5.5 --- test/settings_reference.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/settings_reference.sh b/test/settings_reference.sh index a931fc903..e55df496b 100755 --- a/test/settings_reference.sh +++ b/test/settings_reference.sh @@ -45,6 +45,10 @@ server_rows() { listed() { # stdin lines -> one space-separated line, or "none" when there are none local x; x="$(tr '\n' ' ' | sed 's/ *$//')"; echo "${x:-none}" } +# comm compares WHOLE lines, so each side of a two-field comparison is sorted again +# after its cut: rows sorted on the name alone put `autovacuum|` after +# `autovacuum_compact_threshold|` in C order, and comm then pairs the wrong lines +# and names settings that are correct (OffgridwithJD, review of #1414). docd="$(doc_rows)" srv="$(server_rows)" pub="$(server_rows public)" @@ -59,9 +63,9 @@ check "every setting the server defines is documented, the developer hooks aside check "every documented setting exists on the server" \ "$(LC_ALL=C comm -23 <(cut -d'|' -f1 <<<"$docd") <(cut -d'|' -f1 <<<"$srv") | listed)" "none" check "each documented default is the server's" \ - "$(LC_ALL=C comm -3 <(cut -d'|' -f1,2 <<<"$docd") <(cut -d'|' -f1,2 <<<"$srv_docd") | tr -d '\t' | listed)" "none" + "$(LC_ALL=C comm -3 <(cut -d'|' -f1,2 <<<"$docd" | LC_ALL=C sort) <(cut -d'|' -f1,2 <<<"$srv_docd" | LC_ALL=C sort) | tr -d '\t' | listed)" "none" check "each documented condition is the server's context for the setting" \ - "$(LC_ALL=C comm -3 <(cut -d'|' -f1,3 <<<"$docd") <(cut -d'|' -f1,3 <<<"$srv_docd") | tr -d '\t' | listed)" "none" + "$(LC_ALL=C comm -3 <(cut -d'|' -f1,3 <<<"$docd" | LC_ALL=C sort) <(cut -d'|' -f1,3 <<<"$srv_docd" | LC_ALL=C sort) | tr -d '\t' | listed)" "none" # ---- enable_end_truncation needs superuser (item 57) ------------------------------- psql_run "DROP ROLE IF EXISTS sr_plain; CREATE ROLE sr_plain LOGIN NOSUPERUSER;" From a22a224361d7b40f9c5943ae3f7adda43591d2d6 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 05:27:17 +0000 Subject: [PATCH 8/8] test: megabyte-scale values are stored exactly (#1341) A columnar table has no TOAST table: the writer detoasts every value and stores it inline. The largest value any test stored was 70 KB. A new suite, large_values, and its pytest twin. Values go into heap and are copied to a columnar table, and the two must agree by length and md5: 1 MB and two 16 MB incompressible values, a 200 MB compressible one, and an UPDATE that takes a value heap has TOASTed. An opt-in arm (PGC_RUN_OVER_1GB=1) checks that a chunk over 1 GB is refused with 54000, commits nothing and leaves the server up; it needs several GB of memory, so CI skips it. It passed when run by hand on PG17 (38 s). The twin does not carry it, and test_compare_to_bash.py declares the pair incomplete. 5/5 plus the skip on each of PG15-19, and the twin on each. Three checks are dated on PG17 by a mutant that stores a TOASTed value as its pointer. Skipping only the write path's detoast reddened nothing, because the encoder flattens again. cluster_tests 565 -> 566. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- test/check_ledger.tsv | 6 +++ test/check_ledger_budget.txt | 15 ++++++- test/large_values.sh | 62 +++++++++++++++++++++++++++++ test/pytest/TESTS.md | 11 +++++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_compare_to_bash.py | 5 +++ test/pytest/test_large_values.py | 53 ++++++++++++++++++++++++ test/run_all_versions.sh | 1 + 8 files changed, 155 insertions(+), 2 deletions(-) create mode 100755 test/large_values.sh create mode 100644 test/pytest/test_large_values.py diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 44ebe14e8..e4106759f 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -2012,6 +2012,12 @@ inheritance inheritance premise: while its leaf is an ordinary relation 15;16;17 inheritance inheritance row identities match the heap 15;16;17;18;19 never - inheritance inheritance the child is visible through the parent 15;16;17;18;19 never - inheritance inheritance ungrouped vector aggregation also sees the child rows 15;16;17;18;19 never - +large_values large_values a chunk over 1 GB is refused with 54000, commits nothing, and the server stays up 15;16;17;18;19 never - +large_values large_values an UPDATE that takes a TOASTed heap value stores it exactly in the columnar row 15;16;17;18;19 2026-10-15 a TOASTed value stored as its pointer: the write path and the encoder both skip pg_detoast_datum for an external value (src/columnar_write_state.c, src/columnar_reader.c) +large_values large_values and the other rows are unchanged 15;16;17;18;19 2026-10-15 a TOASTed value stored as its pointer: the write path and the encoder both skip pg_detoast_datum for an external value (src/columnar_write_state.c, src/columnar_reader.c) +large_values large_values premise: heap holds a 1 MB, two 16 MB and a 200 MB value 15;16;17;18;19 never - +large_values large_values premise: heap stores the new 16 MB value TOASTed, not inline 15;16;17;18;19 never - +large_values large_values the values copied into a columnar table are stored exactly, by length and md5 15;16;17;18;19 2026-10-15 a TOASTed value stored as its pointer: the write path and the encoder both skip pg_detoast_datum for an external value (src/columnar_write_state.c, src/columnar_reader.c) latest_reading latest_reading premise: the document's index exists 15;16;17;18;19 never - latest_reading latest_reading premise: the document's three recipe blocks were found 15;16;17;18;19 never - latest_reading latest_reading premise: the indexed LATERAL form read the table at all 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 10785ac40..834f62fee 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2362,4 +2362,17 @@ suites_not_covered 231 # never 2278 + 0 = 2278 # dated 706 + 8 = 714 # 2278 + 714 == 2992 -checks_never_observed_red 2278 +# +# #1341, megabyte-scale values are stored exactly (jdatcmd): 2278 -> 2281. +# large_values enters with all 6 checks from five-major runs (5/5 plus the opt-in arm +# skipped, on each of 15-19). Three are dated on PG17 by one mutant: a TOASTed value +# stored as its pointer, both the write path and the encoder skipping +# pg_detoast_datum for an external value. The other three arrive never: two premises +# and the opt-in chunk-over-1-GB arm, which passed when run by hand on PG17 +# (PGC_RUN_OVER_1GB=1: 54000, nothing committed, the server up). +# +# ledger rows 2992 + 6 = 2998 +# never 2278 + 3 = 2281 +# dated 714 + 3 = 717 +# 2281 + 717 == 2998 +checks_never_observed_red 2281 diff --git a/test/large_values.sh b/test/large_values.sh new file mode 100755 index 000000000..2d88f8712 --- /dev/null +++ b/test/large_values.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +# +# Megabyte-scale values are stored exactly (#1341). A columnar table has no TOAST +# table: the writer detoasts every value and stores it inline in the value stream. +# The largest value any test stored was 70 KB. +# +# Each value goes into a heap table first and is copied to a columnar one, and the +# two must agree on each value's length and md5: +# - 1 MB and two 16 MB incompressible values (md5 text), and a 200 MB value that +# compresses to almost nothing; +# - an UPDATE that takes a value heap has TOASTed and stores it in a columnar row. +# +# A chunk over 1 GB is past what one allocation may hold. An opt-in arm +# (PGC_RUN_OVER_1GB=1, about 1.1 GB of incompressible text in one chunk group) checks +# that the INSERT is refused with 54000, commits nothing, and leaves the server up. +# It is opt-in because it needs several GB of memory, more than a CI runner should +# spend on one arm. +# +# Usage: test/large_values.sh [PG_CONFIG] +# Written fresh for pgColumnar. + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +mb() { # mb N SALT -> an SQL expression for N MB of incompressible text + echo "(SELECT string_agg(md5(g::text || '$2'), '') FROM generate_series(1, $1 * 32768) g)" +} +DIGEST="SELECT string_agg(id || ':' || length(big) || ':' || md5(big), ' ' ORDER BY id) FROM" + +psql_run "CREATE TABLE lv_heap (id int, big text); + CREATE TABLE lv (id int, big text) USING pgcolumnar; + INSERT INTO lv_heap VALUES (1, $(mb 1 a)), (2, $(mb 16 b)), (3, $(mb 16 c)), + (4, repeat('z', 200 * 1024 * 1024));" +check "premise: heap holds a 1 MB, two 16 MB and a 200 MB value" \ + "$(q "SELECT string_agg(length(big)::text, ',' ORDER BY id) FROM lv_heap")" "1048576,16777216,16777216,209715200" +check "the values copied into a columnar table are stored exactly, by length and md5" \ + "$(psql_run "INSERT INTO lv SELECT * FROM lv_heap;" >/dev/null 2>&1; q "$DIGEST lv")" "$(q "$DIGEST lv_heap")" + +# ---- an UPDATE from a TOASTed heap value ---------------------------------------------- +psql_run "UPDATE lv_heap SET big = big || 'x' WHERE id = 2;" +check "premise: heap stores the new 16 MB value TOASTed, not inline" \ + "$(q "SELECT pg_column_size(big) < length(big) OR pg_column_size(big) > 8192 FROM lv_heap WHERE id = 2")" "t" +check "an UPDATE that takes a TOASTed heap value stores it exactly in the columnar row" \ + "$(psql_run "UPDATE lv SET big = h.big FROM lv_heap h WHERE lv.id = h.id AND h.id = 2;" >/dev/null 2>&1 + q "$DIGEST lv WHERE id = 2")" "$(q "$DIGEST lv_heap WHERE id = 2")" +check "and the other rows are unchanged" "$(q "$DIGEST lv WHERE id <> 2")" "$(q "$DIGEST lv_heap WHERE id <> 2")" + +# ---- over 1 GB in one chunk: refused cleanly (opt-in) --------------------------------- +OVER="a chunk over 1 GB is refused with 54000, commits nothing, and the server stays up" +if [ "${PGC_RUN_OVER_1GB:-0}" = 1 ]; then + psql_run "CREATE TABLE lv_huge (id int, big text) USING pgcolumnar;" + out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At \ + -v VERBOSITY=sqlstate -c "SET statement_timeout = '900s'; SET pgcolumnar.compression = none;" \ + -c "INSERT INTO lv_huge SELECT k, $(mb 1 "k' || k || '") FROM generate_series(1, 1100) k;" 2>&1)" + st="$(sed -n 's/^ERROR: *\([0-9A-Z]\{5\}\).*/\1/p' <<<"$out" | tail -1)" + check "$OVER" "${st:-committed}|$(q "SELECT count(*) FROM lv_huge" | tail -1)|$(q "SELECT 1" | tail -1)" "54000|0|1" +else + check_skip "$OVER" "SKIP $OVER (set PGC_RUN_OVER_1GB=1; it needs several GB of memory)" "opt-in heavy arm" +fi + +pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 225d17b24..dc809e6e4 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -162,6 +162,7 @@ behaviour, the source of that number is named. - [114. test_vm_privilege.py: the visibility-map entry points are owner-only and refuse heap](#114-test_vm_privilegepy-the-visibility-map-entry-points-are-owner-only-and-refuse-heap) - [115. test_alter_table_scans.py: ALTER TABLE forms see the live rows and only those](#115-test_alter_table_scanspy-alter-table-forms-see-the-live-rows-and-only-those) - [116. test_settings_reference.py: the settings reference matches the server](#116-test_settings_referencepy-the-settings-reference-matches-the-server) +- [117. test_large_values.py: megabyte-scale values are stored exactly](#117-test_large_valuespy-megabyte-scale-values-are-stored-exactly) ## 1. How to read a test in here @@ -6984,3 +6985,13 @@ The pytest twin of `test/settings_reference.sh` (#1341 items 42, 57 and 83), eve | `test_each_documented_default_and_condition_is_the_servers` | each documented default and condition matches `boot_val` and `context` | | `test_only_a_superuser_sets_enable_end_truncation` | the 42501 refusal on a real login, and the superuser control | | `test_compression_level_reaches_the_zstd_codec` | level 19 stores less than level 1 | + +## 117. test_large_values.py: megabyte-scale values are stored exactly + +The pytest twin of `test/large_values.sh` (#1341), every check by the shell's name but one. A columnar table has no TOAST table: the writer detoasts each value and stores it inline. A 1 MB value, two 16 MB incompressible values and a 200 MB compressible one go into heap and are copied to a columnar table; then an UPDATE takes a value heap has TOASTed. Each must agree with heap by length and md5. The shell's opt-in arm for a chunk over 1 GB (`PGC_RUN_OVER_1GB=1`, refused with 54000, nothing committed, server up) is not ported, and `test_compare_to_bash.py` declares the pair incomplete for it. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_megabyte_values_are_stored_exactly` | the four values and the TOASTed update against heap, and the rows the update left alone | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index e4bf75ebc..604a7114e 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -681,4 +681,6 @@ guard_tests 435 # over the cluster files on this tree. # cluster_tests 570 -> 574 for #1341's test_settings_reference.py, four tests, by collection # over the cluster files on this tree. -cluster_tests 574 +# cluster_tests 574 -> 575 for #1341's test_large_values.py, one test, by collection +# over the cluster files on this tree. +cluster_tests 575 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index ca5ec24d0..97b80a28d 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -160,6 +160,11 @@ # about the PORT rather than a licence: the standing arm does not grade it, so the # reason is the only thing standing between a declared gap and a forgotten one. INCOMPLETE = { + # #1341's megabyte values: every arm is ported except the opt-in chunk over 1 GB + # (PGC_RUN_OVER_1GB=1), which needs several GB of memory and which this layer, + # having no SKIP verdict for an arm a run chooses not to make, does not carry. + "large_values": "the 1 MB to 200 MB round trips and the TOASTed-update arms are " + "ported; the opt-in chunk-over-1-GB refusal is not", # #1293 ported the arms it added: min/max over timestamptz read no data, and # stay right after deletes. The int4/text parity, sum/avg, parallel-plan and # fallback arms of the shell suite predate it and are #432's to port. diff --git a/test/pytest/test_large_values.py b/test/pytest/test_large_values.py new file mode 100644 index 000000000..e3825473d --- /dev/null +++ b/test/pytest/test_large_values.py @@ -0,0 +1,53 @@ +"""Megabyte-scale values are stored exactly (#1341). The pytest twin of +`test/large_values.sh`, every check by the shell's name except one. + +A columnar table has no TOAST table: the writer detoasts each value and stores it +inline. Each value goes into heap first and is copied to a columnar table, and the +two must agree on each value's length and md5: 1 MB and two 16 MB incompressible +values and a 200 MB compressible one, then an UPDATE that takes a value heap has +TOASTed. + +NOT PORTED: the shell's opt-in arm for a chunk over 1 GB (PGC_RUN_OVER_1GB=1). It +needs several GB of memory, and this layer has no SKIP verdict for an arm a run +chooses not to make; test_compare_to_bash.py declares the pair incomplete for it. +""" + + +def _one(conn, sql): + with conn.cursor() as cur: + cur.execute(sql) + return cur.fetchone()[0] + + +def _mb(n, salt): + return f"(SELECT string_agg(md5(g::text || '{salt}'), '') FROM generate_series(1, {n} * 32768) g)" + + +DIGEST = "SELECT string_agg(id || ':' || length(big) || ':' || md5(big), ' ' ORDER BY id) FROM " + + +def test_megabyte_values_are_stored_exactly(pgc_conn, expect): + with pgc_conn.cursor() as cur: + cur.execute("CREATE TABLE lv_heap (id int, big text)") + cur.execute("CREATE TABLE lv (id int, big text) USING pgcolumnar") + cur.execute(f"INSERT INTO lv_heap VALUES (1, {_mb(1, 'a')}), (2, {_mb(16, 'b')}), (3, {_mb(16, 'c')}), " + f"(4, repeat('z', 200 * 1024 * 1024))") + expect.text(_one(pgc_conn, "SELECT string_agg(length(big)::text, ',' ORDER BY id) FROM lv_heap"), + "1048576,16777216,16777216,209715200", + "premise: heap holds a 1 MB, two 16 MB and a 200 MB value") + with pgc_conn.cursor() as cur: + cur.execute("INSERT INTO lv SELECT * FROM lv_heap") + expect.text(_one(pgc_conn, DIGEST + "lv"), _one(pgc_conn, DIGEST + "lv_heap"), + "the values copied into a columnar table are stored exactly, by length and md5") + + with pgc_conn.cursor() as cur: + cur.execute("UPDATE lv_heap SET big = big || 'x' WHERE id = 2") + expect.text(str(_one(pgc_conn, "SELECT pg_column_size(big) < length(big) OR pg_column_size(big) > 8192 " + "FROM lv_heap WHERE id = 2")), + "True", "premise: heap stores the new 16 MB value TOASTed, not inline") + with pgc_conn.cursor() as cur: + cur.execute("UPDATE lv SET big = h.big FROM lv_heap h WHERE lv.id = h.id AND h.id = 2") + expect.text(_one(pgc_conn, DIGEST + "lv WHERE id = 2"), _one(pgc_conn, DIGEST + "lv_heap WHERE id = 2"), + "an UPDATE that takes a TOASTed heap value stores it exactly in the columnar row") + expect.text(_one(pgc_conn, DIGEST + "lv WHERE id <> 2"), _one(pgc_conn, DIGEST + "lv_heap WHERE id <> 2"), + "and the other rows are unchanged") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 42746c3cd..90d44388f 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -134,6 +134,7 @@ SUITES=( inheritance int8_agg_int128 isolation + large_values latest_reading local_open_race_free logical_decoding_cdc_recipe