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/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/am_contracts.sh b/test/am_contracts.sh index 1ebb8c9a6..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" \ @@ -309,4 +333,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..e4106759f 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1,6 +1,22 @@ +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 +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) @@ -29,6 +45,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 - @@ -1995,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 - @@ -2677,6 +2700,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 3a800607d..834f62fee 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2307,4 +2307,72 @@ suites_not_covered 231 # never 2179 + 93 = 2272 # dated 694 + 1 = 695 # 2272 + 695 == 2967 -checks_never_observed_red 2272 +# +# #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 +# +# 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 +# +# #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 +# +# #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 +# +# #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 0e597cec2..dc809e6e4 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -160,6 +160,9 @@ 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) +- [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 @@ -6918,6 +6921,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 @@ -6958,3 +6962,36 @@ 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 | + +## 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 | + +## 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 a7c06924a..604a7114e 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -675,4 +675,12 @@ 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 -> 570 for #1341's test_alter_table_scans.py, one test, by collection +# 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 -> 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_alter_table_scans.py b/test/pytest/test_alter_table_scans.py new file mode 100644 index 000000000..a58329406 --- /dev/null +++ b/test/pytest/test_alter_table_scans.py @@ -0,0 +1,96 @@ +"""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): + 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_am_contracts.py b/test/pytest/test_am_contracts.py index ca4a275d7..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, @@ -418,3 +420,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") diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 6d091fcbd..97b80a28d 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", @@ -141,6 +142,7 @@ "rewrite_storage_oid", "rollup", "scan_decode_cost", + "settings_reference", "sorted_pathkeys", "stats_privilege", "storage_row_repair", @@ -158,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/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 55e8b31b1..90d44388f 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 @@ -133,6 +134,7 @@ SUITES=( inheritance int8_agg_int128 isolation + large_values latest_reading local_open_race_free logical_decoding_cdc_recipe @@ -292,6 +294,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..e55df496b --- /dev/null +++ b/test/settings_reference.sh @@ -0,0 +1,101 @@ +#!/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}" +} +# 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)" +# 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"))" + +_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" \ + "$(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" | 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" | 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;" +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 '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 + 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