From f32555b58c70913b2db01a2ef754d77e17042fba Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 01:25:36 +0000 Subject: [PATCH 1/3] test: the documented refusals change nothing (#1341 items 44, 51, 77) CREATE UNLOGGED TABLE and ALTER TABLE ... SET UNLOGGED are refused (0A000) and leave nothing changed; ALTER TABLE ... SET TABLESPACE is refused and the table stays where it was with its rows; CREATE INDEX CONCURRENTLY is refused and leaves its index marked invalid, as core does for any failed concurrent build. Nothing tested any of them. Arms in both twins of am_contracts, each red under its own mutation. limitations.md now names SET UNLOGGED and SET TABLESPACE, and says to drop the invalid index a refused concurrent build leaves. cluster_tests +1. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N --- docs/limitations.md | 9 ++++-- test/am_contracts.sh | 19 ++++++++++++ test/check_ledger.tsv | 4 +++ test/check_ledger_budget.txt | 11 +++++++ test/pytest/TESTS.md | 1 + test/pytest/expected_tests.txt | 3 +- test/pytest/test_am_contracts.py | 50 ++++++++++++++++++++++++++++++++ 7 files changed, 94 insertions(+), 3 deletions(-) diff --git a/docs/limitations.md b/docs/limitations.md index edcaa84cd..b6b822d39 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -657,7 +657,8 @@ other than where the feature is used: `ON CONFLICT DO NOTHING` takes no row lock and works, with or without a conflict target. -`CREATE TABLE` refuses an unlogged columnar table for the same reason. Both cases +`CREATE TABLE` refuses an unlogged columnar table for the same reason, and so does +`ALTER TABLE ... SET UNLOGGED`. Both cases follow one rule. This access method refuses a configuration that it cannot support at the point where you choose it. It does not refuse it at each later use. @@ -783,7 +784,11 @@ therefore still be cheaper as a scan. 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. + block-range index builds are not supported. A refused `CREATE INDEX + CONCURRENTLY` leaves its index behind, marked invalid, as PostgreSQL does for any + concurrent build that fails. Drop that index before you build it again. +- `ALTER TABLE ... SET TABLESPACE` is not supported on a columnar table. It is + refused, and the table stays where it was. ## Constraints on the import path diff --git a/test/am_contracts.sh b/test/am_contracts.sh index 2a7c38bad..1f0d82da7 100755 --- a/test/am_contracts.sh +++ b/test/am_contracts.sh @@ -351,4 +351,23 @@ check "REINDEX after a committed delete of most rows reclaims their entries, as "$(ix_reclaim pgcolumnar)|$(ix_reclaim heap)|$(q "SET enable_seqscan = off; SELECT count(*) FROM ix_pgcolumnar WHERE a > 0;" | tail -1)" \ "reclaimed|reclaimed|10000" +# ---- documented refusals (#1341 items 44, 51, 77) ---------------------------------- +# +# Each is refused where it is chosen, as limitations.md says, and refused cleanly: +# nothing is left changed by the attempt. +rf_count() { q "SELECT count(*) || '|' || sum(a) FROM rf"; } +psql_run "CREATE TABLE rf (a int) USING pgcolumnar; INSERT INTO rf SELECT g FROM generate_series(1, 5000) g;" +check "CREATE UNLOGGED TABLE ... USING pgcolumnar is refused (0A000) and leaves no table" \ + "$(state "CREATE UNLOGGED TABLE rf_u (a int) USING pgcolumnar")|$(q "SELECT count(*) FROM pg_class WHERE relname = 'rf_u'")" "0A000|0" +check "ALTER TABLE ... SET UNLOGGED is refused (0A000), and the table stays logged with its rows" \ + "$(state "ALTER TABLE rf SET UNLOGGED")|$(q "SELECT relpersistence FROM pg_class WHERE relname = 'rf'")|$(rf_count)" "0A000|p|5000|12502500" +pgc_pg "mkdir -p -m 700 '$PGC_WORKDIR/rf_ts'" +psql_run "CREATE TABLESPACE rf_ts LOCATION '$PGC_WORKDIR/rf_ts';" +check "ALTER TABLE ... SET TABLESPACE is refused (0A000), and the table stays where it was with its rows" \ + "$(state "ALTER TABLE rf SET TABLESPACE rf_ts")|$(q "SELECT reltablespace FROM pg_class WHERE relname = 'rf'")|$(rf_count)" "0A000|0|5000|12502500" +# A failed concurrent build leaves its index behind, marked invalid, as core does for +# any CREATE INDEX CONCURRENTLY that fails after its first phase. +check "CREATE INDEX CONCURRENTLY is refused (0A000), and the index it leaves is marked invalid" \ + "$(state "CREATE INDEX CONCURRENTLY rf_cic ON rf (a)")|$(q "SELECT indisvalid FROM pg_index WHERE indexrelid = 'rf_cic'::regclass")" "0A000|f" + pgc_summary diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index dda5e4fa5..4c98ca41b 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -13,6 +13,10 @@ alter_table_scans alter_table_scans and the rewrite kept the live rows and not t 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 ALTER TABLE ... SET TABLESPACE is refused (0A000), and the table stays where it was with its rows 15;16;17;18;19 2026-10-15 relation copy returning without copying (SET TABLESPACE) +am_contracts am_contracts ALTER TABLE ... SET UNLOGGED is refused (0A000), and the table stays logged with its rows 15;16;17;18;19 2026-10-15 the storage hook accepting an unlogged columnar table +am_contracts am_contracts CREATE INDEX CONCURRENTLY is refused (0A000), and the index it leaves is marked invalid 15;16;17;18;19 2026-10-15 the concurrent index validate scan doing nothing +am_contracts am_contracts CREATE UNLOGGED TABLE ... USING pgcolumnar is refused (0A000) and leaves no table 15;16;17;18;19 2026-10-15 the storage hook accepting an unlogged columnar table 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 da01b2ca4..b17a87c23 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2414,4 +2414,15 @@ suites_not_covered 231 # never 2282 + 0 = 2282 # dated 726 + 9 = 735 # 2282 + 735 == 3017 +# +# #1341 items 44, 51 and 77, the documented refusals (jdatcmd): 2282 -> 2282. +# am_contracts gains four checks from five-major runs (31/31 on each of 15-19), all +# dated: CREATE UNLOGGED and SET UNLOGGED by the storage hook accepting an unlogged +# table, CREATE INDEX CONCURRENTLY by its validate scan doing nothing, and SET +# TABLESPACE by the relation copy returning without copying. +# +# ledger rows 3017 + 4 = 3021 +# never 2282 + 0 = 2282 +# dated 735 + 4 = 739 +# 2282 + 739 == 3021 checks_never_observed_red 2282 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 97d277b4a..6659cd10a 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -6925,6 +6925,7 @@ THE WITHDRAWAL IS REACHED ONLY BY A RACE. When one session holds a key uncommitt | `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) | +| `test_the_documented_refusals_change_nothing` | `CREATE UNLOGGED TABLE` and `SET UNLOGGED` are refused (`0A000`) and leave nothing changed; `SET TABLESPACE` is refused and the table stays where it was; `CREATE INDEX CONCURRENTLY` is refused and leaves its index marked invalid, as core does (#1341 items 44, 51, 77) | ## 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 316bd78ae..b84f2d3de 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -689,4 +689,5 @@ guard_tests 435 # test_rollup.py, by collection over the cluster files on this tree. # cluster_tests 577 -> 578 for #1341's test_statement_self_visibility.py, one test, by collection # over the cluster files on this tree. -cluster_tests 578 +# cluster_tests 578 -> 579 for #1341's test_the_documented_refusals_change_nothing in +cluster_tests 579 diff --git a/test/pytest/test_am_contracts.py b/test/pytest/test_am_contracts.py index 5780897d5..7e373c8d2 100644 --- a/test/pytest/test_am_contracts.py +++ b/test/pytest/test_am_contracts.py @@ -448,3 +448,53 @@ def reclaim(am): 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") + + +def test_the_documented_refusals_change_nothing(pgc_conn, expect): + """Each is refused where it is chosen, as limitations.md says, and nothing is left + changed by the attempt (#1341 items 44, 51, 77). A failed concurrent build leaves + its index behind, marked invalid, as core does for any such failure.""" + import os + import shutil + import tempfile + + def state(sql): + try: + with pgc_conn.cursor() as cur: + cur.execute(sql) + return "ok" + except psycopg.Error as e: + return e.sqlstate or "?" + + count = "SELECT count(*) || '|' || sum(a) FROM rf" + with pgc_conn.cursor() as cur: + cur.execute("CREATE TABLE rf (a int) USING pgcolumnar") + cur.execute("INSERT INTO rf SELECT g FROM generate_series(1, 5000) g") + expect.text(f"{state('CREATE UNLOGGED TABLE rf_u (a int) USING pgcolumnar')}|" + f"{_one(pgc_conn, 'SELECT count(*) FROM pg_class WHERE relname = ' + repr('rf_u'))}", + "0A000|0", "CREATE UNLOGGED TABLE ... USING pgcolumnar is refused (0A000) and leaves no table") + expect.text(f"{state('ALTER TABLE rf SET UNLOGGED')}|" + f"{_one(pgc_conn, 'SELECT relpersistence FROM pg_class WHERE relname = ' + repr('rf'))}|" + f"{_one(pgc_conn, count)}", + "0A000|p|5000|12502500", "ALTER TABLE ... SET UNLOGGED is refused (0A000), and the table stays logged with its rows") + + # A tablespace directory the server's own user owns, outside any root-only parent. + ts = tempfile.mkdtemp(prefix="pgc-rf-ts-", dir="/tmp") + os.chmod(ts, 0o700) + if os.geteuid() == 0: + shutil.chown(ts, user="postgres") + try: + with pgc_conn.cursor() as cur: + cur.execute(f"CREATE TABLESPACE rf_ts_py LOCATION '{ts}'") + expect.text(f"{state('ALTER TABLE rf SET TABLESPACE rf_ts_py')}|" + f"{_one(pgc_conn, 'SELECT reltablespace FROM pg_class WHERE relname = ' + repr('rf'))}|" + f"{_one(pgc_conn, count)}", + "0A000|0|5000|12502500", "ALTER TABLE ... SET TABLESPACE is refused (0A000), and the table stays where it was with its rows") + finally: + with pgc_conn.cursor() as cur: + cur.execute("DROP TABLESPACE IF EXISTS rf_ts_py") + shutil.rmtree(ts, ignore_errors=True) + + expect.text(f"{state('CREATE INDEX CONCURRENTLY rf_cic ON rf (a)')}|" + f"{_one(pgc_conn, 'SELECT indisvalid FROM pg_index WHERE indexrelid = ' + repr('rf_cic') + '::regclass')}", + "0A000|False", "CREATE INDEX CONCURRENTLY is refused (0A000), and the index it leaves is marked invalid") From 9fa51758df0fcdbcdd128eae1e70101a729d2555 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 18:13:18 +0000 Subject: [PATCH 2/3] test: the row-identity lock out of SQL's reach, bulk_parallel_writer by hand, and the row_lock_buckets bound (#1341 items 55, 56, 16) Squashed for the rebase onto main; the commits it replaces were: - test: advisory_lock_class's first writer commits on a flag row, not after a sleep (#1341) - test: a bulk UPDATE holds row_lock_buckets row locks, not one per row (#1341 item 16) - test: the row-identity lock is out of SQL's reach, and bulk_parallel_writer set by hand is safe (#1341 items 55, 56) Co-Authored-By: Claude Opus 5.5 --- test/advisory_lock_class.sh | 122 +++++++++++++++++++ test/check_ledger.tsv | 17 +++ test/check_ledger_budget.txt | 24 +++- test/pytest/TESTS.md | 4 +- test/pytest/expected_tests.txt | 4 +- test/pytest/test_advisory_lock_class.py | 154 +++++++++++++++++++++++- 6 files changed, 320 insertions(+), 5 deletions(-) diff --git a/test/advisory_lock_class.sh b/test/advisory_lock_class.sh index db4f5a058..03f3caa8e 100755 --- a/test/advisory_lock_class.sh +++ b/test/advisory_lock_class.sh @@ -245,4 +245,126 @@ check "row_lock_buckets is fixed at server start, so a session cannot change it "$(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 pgcolumnar.row_lock_buckets = 7;" 2>&1 | sed -n 's/^.*ERROR:[[:space:]]*\([0-9A-Z]\{5\}\).*$/\1/p')|$(q "SELECT context || '|' || setting FROM pg_settings WHERE name = 'pgcolumnar.row_lock_buckets'")" \ "55P02|postmaster|1024" +# The documented bound (docs/configuration.md): row_lock_buckets "bounds the row locks +# a transaction holds, so a bulk update cannot exhaust the lock table". One transaction +# updates 200000 rows and then counts its own class-104 locks (one -c string is one +# transaction): one per bucket, not one per row. A row lock per row would not fit the +# default lock table at all. +psql_run "CREATE TABLE rlb (id int, v int) USING pgcolumnar; + INSERT INTO rlb SELECT g, g FROM generate_series(1, 200000) g;" >/dev/null +check "a bulk UPDATE of 200000 rows holds one row-identity lock per bucket, row_lock_buckets of them" \ + "$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At \ + -c "UPDATE rlb SET v = v + 1; + SELECT count(*) || '|' || current_setting('pgcolumnar.row_lock_buckets') FROM pg_locks + WHERE pid = pg_backend_pid() AND locktype = 'advisory' AND objsubid = 104;" 2>&1 | + grep -E '^[0-9]+[|]|ERROR' | tail -1)" "1024|1024" +check "and the UPDATE committed every row" "$(q "SELECT count(*) || '|' || sum(v - id) FROM rlb")" "200000|200000" + +# ---- the row-identity lock cannot be reached from SQL (#1341 item 56) ------------ +# The INSERT arms above ask this of the unique-key and storage-row locks; nothing +# asked it of class 104. An UPDATE is held open and every advisory lock it took is +# read. None may be in field 1 or 2, the only classes SQL can produce. Then a second +# session takes each of those exact tags with pg_advisory_xact_lock(int4, int4), and +# a columnar UPDATE of the same row must not wait on it. An oid above the int4 range +# is addressed by its negative int4, the same 32 bits lockfuncs.c puts in the tag. +rl_wait_for() { # rl_wait_for APPLICATION_NAME -> its pid, once it has named itself + local i pid="" + for i in $(seq 1 100); do + pid="$(q "SELECT pid FROM pg_stat_activity WHERE application_name = '$1'")" + [ -n "$pid" ] && break + sleep 0.1 + done + echo "$pid" +} +rl_end() { # rl_end PID BGPID: end a holder's backend and its psql + q "SELECT pg_terminate_backend($1)" >/dev/null 2>&1 + kill "$2" 2>/dev/null; wait "$2" 2>/dev/null + local i + for i in $(seq 1 60); do + [ "$(q "SELECT count(*) FROM pg_locks WHERE locktype = 'advisory' AND pid <> pg_backend_pid()")" = 0 ] && break + sleep 0.2 + done +} +env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \ + -c "BEGIN; UPDATE rl SET v = v + 1 WHERE id = 7; SET application_name = 'rl_tagger'; SELECT pg_sleep(30); COMMIT;" \ + >/dev/null 2>&1 & +rl_bg=$! +rl_pid="$(rl_wait_for rl_tagger)" +RL_TAGS="$(q "SELECT CASE WHEN classid::bigint > 2147483647 THEN classid::bigint - 4294967296 ELSE classid::bigint END + || ' ' || CASE WHEN objid::bigint > 2147483647 THEN objid::bigint - 4294967296 ELSE objid::bigint END + || ' ' || objsubid + FROM pg_locks WHERE locktype = 'advisory' AND granted AND pid = ${rl_pid:-0}")" +rl_end "${rl_pid:-0}" "$rl_bg" +RL_N="$(printf '%s\n' "$RL_TAGS" | grep -c '[0-9]')" +rl_reach="" +while read -r _c _o _f; do + case "${_f:-}" in 1|2) rl_reach="$rl_reach ($_c,$_o,field4=$_f)" ;; esac +done <<<"$RL_TAGS" +check "the locks an UPDATE takes are not in a SQL-reachable class" \ + "$(if [ "$RL_N" -eq 0 ]; then echo "unknown (no advisory lock seen; holder pid ${rl_pid:-none})" + elif [ -n "$rl_reach" ]; then echo "reachable:$rl_reach" + else echo unreachable; fi)" "unreachable" +rl_grab="BEGIN;" +while read -r _c _o _f; do + [ -z "${_f:-}" ] && continue + rl_grab="$rl_grab SELECT pg_advisory_xact_lock($_c::int, $_o::int);" +done <<<"$RL_TAGS" +env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \ + -c "$rl_grab SET application_name = 'rl_user'; SELECT pg_sleep(30); COMMIT;" >/dev/null 2>&1 & +rl_bg=$! +rl_pid="$(rl_wait_for rl_user)" +check "premise: the other session holds every one of those tags through the SQL functions" \ + "$(q "SELECT count(*) FROM pg_locks WHERE locktype = 'advisory' AND granted AND objsubid = 2 + AND pid = ${rl_pid:-0}")" "$RL_N" +rl_upd="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At \ + -c "SET statement_timeout = '10s';" -c "UPDATE rl SET v = v + 1 WHERE id = 7;" 2>&1)" +case "$rl_upd" in + *timeout*|*canceling*) rl_verdict="BLOCKED by the user lock" ;; + *ERROR*) rl_verdict="ERROR: $(head -1 <<<"$rl_upd")" ;; + *) rl_verdict=ok ;; +esac +check "a user advisory lock on those tags does not block a columnar UPDATE" "$rl_verdict" "ok" +rl_end "${rl_pid:-0}" "$rl_bg" + +# ---- bulk_parallel_writer set by hand still serialises a first write (#1341 item 55) -- +# The setting lets parallel_copy's loaders skip the storage-row creation lock (class +# 102). The reference says setting it by hand is safe, because the skip fires only +# once the storage row is committed. Two sessions with it on each make the first +# write to a committed, empty table: the second must wait for the first and then +# land its row. A skip that did not wait for the row to be committed lets both +# create it, and the second fails on storage_pkey. +# +# The first writer commits only once the suite has looked for the second one waiting +# and then inserted a committed flag row, not after a fixed sleep: a slow second +# writer would otherwise start after the first had committed, and the premise below +# would fail on timing (OffgridwithJD, review of #1415). The pytest twin does the same. +psql_run "CREATE TABLE bw (id int) USING pgcolumnar; CREATE TABLE bw_go (go int) USING heap;" +env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -Atq \ + -c "SET statement_timeout = '60s'; BEGIN; SET LOCAL pgcolumnar.bulk_parallel_writer = on; INSERT INTO bw VALUES (1); SET application_name = 'bw_first'; + DO \$\$ BEGIN WHILE NOT EXISTS (SELECT 1 FROM bw_go) LOOP PERFORM pg_sleep(0.1); END LOOP; END \$\$; COMMIT;" \ + >/dev/null 2>&1 & +bw_bg=$! +bw_pid="$(rl_wait_for bw_first)" +env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At \ + -c "SET pgcolumnar.bulk_parallel_writer = on; SET statement_timeout = '30s';" -c "INSERT INTO bw VALUES (2);" \ + >"$PGC_WORKDIR/bw_second.out" 2>&1 & +bw_bg2=$! +# The second write must be waiting on the first before the first commits, or the arm +# measures two writes in sequence. +bw_waited=no +for _i in $(seq 1 100); do + [ "$(q "SELECT count(*) FROM pg_stat_activity WHERE wait_event_type = 'Lock' AND query LIKE 'INSERT INTO bw%'")" -ge 1 ] && { bw_waited=yes; break; } + sleep 0.1 +done +psql_run "INSERT INTO bw_go VALUES (1);" # release the first writer, whatever the poll saw +check "premise: the second first write waited on the first" "$bw_waited" "yes" +wait "$bw_bg2"; wait "$bw_bg" +bw_out="$(cat "$PGC_WORKDIR/bw_second.out")" +case "$bw_out" in + *ERROR*) bw_verdict="ERROR: $(grep -m1 ERROR <<<"$bw_out")" ;; + *) bw_verdict=ok ;; +esac +check "with bulk_parallel_writer set by hand, a second first write waits for the first and both rows land" \ + "$([ -n "$bw_pid" ] && echo held || echo "no holder")|$bw_verdict|$(q "SELECT count(*) FROM bw")" "held|ok|2" + pgc_summary diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 4c98ca41b..35131afa0 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1,3 +1,20 @@ +advisory_lock_class advisory_lock_class a bulk UPDATE of 200000 rows holds one row-identity lock per bucket, row_lock_buckets of them 15;16;17;18;19 2026-10-15 columnar_row_lock.c: the row-identity bucket taken as the key mod 4294967291, not mod row_lock_buckets (one lock per row) +advisory_lock_class advisory_lock_class a duplicate key is still rejected 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class a user advisory lock on that tag does not block a columnar insert 15;16;17;18;19 2026-10-15 PGCOLUMNAR_LOCKCLASS_UNIQUE_KEY 103 changed to 2, the #430 defect (src/columnar.h) +advisory_lock_class advisory_lock_class a user advisory lock on those tags does not block a columnar UPDATE 15;16;17;18;19 2026-10-15 PGCOLUMNAR_LOCKCLASS_ROW_IDENTITY 104 changed to 2, the class SQL's two-int4 form produces (src/columnar.h) +advisory_lock_class advisory_lock_class an UPDATE holds a row-identity lock (class 104) while enable_row_update_lock is on 15;16;17;18;19 2026-10-15 PGCOLUMNAR_LOCKCLASS_ROW_IDENTITY 104 changed to 2, the class SQL's two-int4 form produces (src/columnar.h) +advisory_lock_class advisory_lock_class and holds none with enable_row_update_lock off 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class and the UPDATE committed every row 15;16;17;18;19 2026-10-15 columnar_row_lock.c: the row-identity bucket taken as the key mod 4294967291, not mod row_lock_buckets (one lock per row) +advisory_lock_class advisory_lock_class premise: the discovering session is gone and holds nothing 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class premise: the insert took an advisory lock we can see 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class premise: the lock is enabled, or nothing below proves anything 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class premise: the other session holds every one of those tags through the SQL functions 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class premise: the other session really holds that exact tag in class 2 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class premise: the second first write waited on the first 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class row_lock_buckets is fixed at server start, so a session cannot change it (55P02) 15;16;17;18;19 never - +advisory_lock_class advisory_lock_class the lock an insert takes is not in a SQL-reachable class 15;16;17;18;19 2026-10-15 PGCOLUMNAR_LOCKCLASS_UNIQUE_KEY 103 changed to 2, the #430 defect (src/columnar.h) +advisory_lock_class advisory_lock_class the locks an UPDATE takes are not in a SQL-reachable class 15;16;17;18;19 2026-10-15 PGCOLUMNAR_LOCKCLASS_ROW_IDENTITY 104 changed to 2, the class SQL's two-int4 form produces (src/columnar.h) +advisory_lock_class advisory_lock_class with bulk_parallel_writer set by hand, a second first write waits for the first and both rows land 15;16;17;18;19 2026-10-15 the storage-row creation lock skipped whenever bulk_parallel_writer is on, not only once the row is committed (src/columnar_metadata.c) 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) diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index b17a87c23..e4aa2dc93 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -15,7 +15,7 @@ # # Adding a check to a suite that is already covered does not move it, which is # what makes it safe to bound. -suites_not_covered 231 +suites_not_covered 230 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -2425,4 +2425,24 @@ suites_not_covered 231 # never 2282 + 0 = 2282 # dated 735 + 4 = 739 # 2282 + 739 == 3021 -checks_never_observed_red 2282 +# +# #1341 items 55, 56 and 16, the row-identity lock out of SQL's reach, +# bulk_parallel_writer set by hand, and the row_lock_buckets bound (jdatcmd): 2282 -> +# 2291, and suites_not_covered 231 -> 230. advisory_lock_class had no rows; it enters +# with all 17 checks from five-major runs (17/17 on each of 15-19). Eight are dated on +# PG17: +# - the row-identity class put at 2: the class-104 arm, the UPDATE's reachable-class +# arm and its contention arm; +# - the unique-key class put back at 2 (#430): the INSERT's reachable-class arm and +# its contention arm; +# - the storage-row lock skipped whenever bulk_parallel_writer is on: the second +# first write fails on storage_pkey; +# - the bucket taken as the raw key, one row lock per row: the bulk UPDATE fails +# with "out of shared memory", which reddens its lock-count arm and its row arm. +# The other nine arrive never: seeding a suite moves the census up by its undated rows. +# +# ledger rows 3021 + 17 = 3038 +# never 2282 + 9 = 2291 +# dated 739 + 8 = 747 +# 2291 + 747 == 3038 +checks_never_observed_red 2291 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 6659cd10a..0b3c32436 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -5397,7 +5397,9 @@ regardless. | `test_the_lock_an_insert_takes_is_not_in_a_sql_reachable_class` | the lock is enabled, the insert's locks are visible, none is SQL-reachable, and the discovering session leaves nothing held | | `test_a_user_advisory_lock_on_that_tag_does_not_block_an_insert` | a user session holding every addressable tag in class 2 does not block a columnar insert | | `test_a_duplicate_key_is_still_rejected` | the lock still does its job, so removing the collision by removing the lock does not pass | -| `test_the_row_identity_lock_and_its_settings` | an UPDATE holds a class-104 row-identity lock with `enable_row_update_lock` on and none with it off; `row_lock_buckets` refuses `SET` (55P02) because it is fixed at server start (#1341) | +| `test_the_row_identity_lock_and_its_settings` | an UPDATE holds a class-104 row-identity lock with `enable_row_update_lock` on and none with it off; `row_lock_buckets` refuses `SET` (55P02) because it is fixed at server start; a 200000-row UPDATE holds `row_lock_buckets` class-104 locks, one per bucket, not one per row (#1341 item 16) | +| `test_the_row_identity_lock_cannot_be_reached_from_sql` | every advisory lock an open UPDATE holds is outside field4 1 and 2, and a second session holding each of those tags through `pg_advisory_xact_lock(int4, int4)` does not block an UPDATE of the same row (#1341 item 56) | +| `test_bulk_parallel_writer_set_by_hand_still_serialises_a_first_write` | two sessions with `bulk_parallel_writer` on each make the first write to a committed, empty table; the second waits for the first, and both rows land (#1341 item 55) | ## 69. test_index_am_support.py: the index methods the page claims must work `docs/features.md` said "`CREATE INDEX` builds btree and hash indexes over a diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index b84f2d3de..75c94bd18 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -690,4 +690,6 @@ guard_tests 435 # cluster_tests 577 -> 578 for #1341's test_statement_self_visibility.py, one test, by collection # over the cluster files on this tree. # cluster_tests 578 -> 579 for #1341's test_the_documented_refusals_change_nothing in -cluster_tests 579 +# cluster_tests 579 -> 581 for #1341 items 56 and 55: test_the_row_identity_lock_cannot_be_reached_from_sql +# and test_bulk_parallel_writer_set_by_hand_still_serialises_a_first_write in +cluster_tests 581 diff --git a/test/pytest/test_advisory_lock_class.py b/test/pytest/test_advisory_lock_class.py index ddbe11dd0..bf3dd3ff9 100644 --- a/test/pytest/test_advisory_lock_class.py +++ b/test/pytest/test_advisory_lock_class.py @@ -244,7 +244,8 @@ def test_a_duplicate_key_is_still_rejected(pgc_conn, expect): def test_the_row_identity_lock_and_its_settings(pgc_cluster, pgc_conn, expect): """An UPDATE serialises on its row through an advisory lock in class 104; the session setting enable_row_update_lock turns that off, and row_lock_buckets is - part of the lock tag, so it is fixed at server start (#1341).""" + part of the lock tag, so it is fixed at server start. A bulk UPDATE holds one lock + per bucket, not one per row (#1341 item 16).""" with pgc_conn.cursor() as cur: cur.execute("CREATE TABLE rl (id int, v int) USING pgcolumnar") cur.execute("INSERT INTO rl SELECT g, g FROM generate_series(1, 100) g") @@ -280,3 +281,154 @@ def row_locks_while_updating(setting): ctx = cur.fetchone()[0] expect.text(f"{refused}|{ctx}", "55P02|postmaster|1024", "row_lock_buckets is fixed at server start, so a session cannot change it (55P02)") + + # The documented bound: row_lock_buckets bounds the row locks a transaction holds. + # One transaction updates 200000 rows, then counts its own class-104 locks. + with pgc_conn.cursor() as cur: + cur.execute("CREATE TABLE rlb (id int, v int) USING pgcolumnar") + cur.execute("INSERT INTO rlb SELECT g, g FROM generate_series(1, 200000) g") + bulk = psycopg.connect(pgc_cluster.dsn(), autocommit=False) + try: + with bulk.cursor() as bc: + bc.execute(f'SET search_path TO "{schema}"') + try: + bc.execute("UPDATE rlb SET v = v + 1") + bc.execute("SELECT count(*) || '|' || current_setting('pgcolumnar.row_lock_buckets') FROM pg_locks" + " WHERE pid = pg_backend_pid() AND locktype = 'advisory' AND objsubid = 104") + held = bc.fetchone()[0] + bulk.commit() + except psycopg.Error as e: + held = f"ERROR: {e.sqlstate}" + bulk.rollback() + finally: + bulk.close() + expect.text(held, "1024|1024", + "a bulk UPDATE of 200000 rows holds one row-identity lock per bucket, row_lock_buckets of them") + with pgc_conn.cursor() as cur: + cur.execute("SELECT count(*) || '|' || sum(v - id) FROM rlb") + expect.text(cur.fetchone()[0], "200000|200000", "and the UPDATE committed every row") + + +def _int4(x): + """An oid as the int4 with the same 32 bits, which is how lockfuncs.c puts a + two-int4 key into the tag.""" + return x - 4294967296 if x > 2147483647 else x + + +def test_the_row_identity_lock_cannot_be_reached_from_sql(pgc_cluster, pgc_conn, expect): + """#1341 item 56: the INSERT arms ask this of the unique-key and storage-row locks, + and nothing asked it of class 104. Every advisory lock an open UPDATE holds is read; + none may be in field 1 or 2. A second session then takes each of those exact tags + through pg_advisory_xact_lock(int4, int4), and an UPDATE of the same row must not + wait on it.""" + with pgc_conn.cursor() as cur: + cur.execute("CREATE TABLE rl (id int, v int) USING pgcolumnar") + cur.execute("INSERT INTO rl SELECT g, g FROM generate_series(1, 100) g") + cur.execute("SELECT current_schema()") + schema = cur.fetchone()[0] + + updater = _holder(pgc_cluster, schema) + try: + with updater.cursor() as uc: + uc.execute("UPDATE rl SET v = v + 1 WHERE id = 7") + uc.execute("SELECT pg_backend_pid()") + upid = uc.fetchone()[0] + with pgc_conn.cursor() as cur: + cur.execute("SELECT classid, objid, objsubid FROM pg_locks " + "WHERE locktype = 'advisory' AND granted AND pid = %s", (upid,)) + tags = cur.fetchall() + finally: + updater.rollback() + updater.close() + + reachable = [t for t in tags if t[2] in SQL_REACHABLE] + expect.text("unknown (no advisory lock seen)" if not tags + else (f"reachable: {reachable}" if reachable else "unreachable"), + "unreachable", "the locks an UPDATE takes are not in a SQL-reachable class") + + user = _holder(pgc_cluster, schema) + try: + with user.cursor() as hc: + for classid, objid, _f in tags: + hc.execute("SELECT pg_advisory_xact_lock(%s::int, %s::int)", + (_int4(classid), _int4(objid))) + hc.execute("SELECT pg_backend_pid()") + hpid = hc.fetchone()[0] + expect.num(_one(pgc_conn, "SELECT count(*) FROM pg_locks WHERE locktype = 'advisory' " + "AND granted AND objsubid = 2 AND pid = %s", (hpid,)), + len(tags), + "premise: the other session holds every one of those tags through the SQL functions") + verdict = "ok" + try: + with pgc_conn.cursor() as cur: + cur.execute("SET statement_timeout = '10s'") + cur.execute("UPDATE rl SET v = v + 1 WHERE id = 7") + except psycopg.errors.QueryCanceled: + verdict = "BLOCKED by the user lock" + except psycopg.Error as exc: + verdict = f"ERROR: {str(exc).splitlines()[0]}" + finally: + with pgc_conn.cursor() as cur: + cur.execute("RESET statement_timeout") + expect.text(verdict, "ok", "a user advisory lock on those tags does not block a columnar UPDATE") + finally: + user.rollback() + user.close() + + +def test_bulk_parallel_writer_set_by_hand_still_serialises_a_first_write( + pgc_cluster, pgc_conn, expect): + """#1341 item 55: the reference says setting bulk_parallel_writer by hand is safe, + because the loaders' skip of the storage-row creation lock fires only once the + storage row is committed. Two sessions with it on each make the first write to a + committed, empty table; the second must wait for the first and then land its row. + A skip that did not wait for the row to be committed lets both create it, and the + second fails on storage_pkey.""" + with pgc_conn.cursor() as cur: + cur.execute("CREATE TABLE bw (id int) USING pgcolumnar") + cur.execute("SELECT current_schema()") + schema = cur.fetchone()[0] + + first = _holder(pgc_cluster, schema) + second = _holder(pgc_cluster, schema) + second.commit() + second.autocommit = True + try: + with first.cursor() as fc: + fc.execute("SET LOCAL pgcolumnar.bulk_parallel_writer = on") + fc.execute("INSERT INTO bw VALUES (1)") + result = {} + + def write_second(): + try: + with second.cursor() as sc: + sc.execute("SET pgcolumnar.bulk_parallel_writer = on") + sc.execute("SET statement_timeout = '30s'") + sc.execute("INSERT INTO bw VALUES (2)") + result["verdict"] = "ok" + except psycopg.Error as exc: + result["verdict"] = f"ERROR: {str(exc).splitlines()[0]}" + + import threading + import time + t = threading.Thread(target=write_second) + t.start() + # The second write must be waiting on the first, not finished, before the + # first commits: otherwise the arm measures two writes in sequence. + waited = "no" + for _ in range(100): + if _one(pgc_conn, "SELECT count(*) FROM pg_stat_activity WHERE wait_event_type = 'Lock' " + "AND query LIKE 'INSERT INTO bw%%'"): + waited = "yes" + break + time.sleep(0.1) + first.commit() + t.join(60) + expect.text(waited, "yes", "premise: the second first write waited on the first") + expect.text(f"{result.get('verdict', 'still running')}|" + f"{_one(pgc_conn, 'SELECT count(*) FROM bw')}", "ok|2", + "with bulk_parallel_writer set by hand, a second first write waits for the first and both rows land") + finally: + first.rollback() + first.close() + second.close() From 2fdb9b43f9b044ac08f371311830e00e0159c566 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 15 Oct 2026 19:57:46 +0000 Subject: [PATCH 3/3] test: a maintenance function that rolls back leaves the table as it was (#1341) A new suite, maintenance_rollback, and its pytest twin. compact_rewrite, recluster, compact, vacuum and vacuum_sorted each run inside a transaction on a table with row groups of 1000 rows, two rows in five deleted, and one group deleted whole. Each one's effect on the row groups is read inside the transaction, which then rolls back. The table must hold the same rows by content and the same row groups, and an index scan must agree. The same then runs under a savepoint, later writes commit, and every function runs for real. 13/13 on each of PG15-19, and the twin on each. Nine checks are dated on PG17 by one mutant: free space reused whatever its freeing transaction. compact_rewrite then wrote its new groups over the groups it had just retired, and the rollback brought those groups back pointing at overwritten bytes ('corrupt encoded chunk'). cluster_tests 565 -> 566. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N Squashed for the rebase onto main; it replaces: - test: maintenance_rollback's rows re-dated on fresh tables, restated on main 1f7b5dad (#1341) - test: maintenance_rollback gives each function a fresh table (#1341) - test: a maintenance function that rolls back leaves the table as it was (#1341) --- test/check_ledger.tsv | 13 +++ test/check_ledger_budget.txt | 16 +++- test/maintenance_rollback.sh | 86 ++++++++++++++++++ test/pytest/TESTS.md | 11 +++ test/pytest/expected_tests.txt | 3 +- test/pytest/test_compare_to_bash.py | 1 + test/pytest/test_maintenance_rollback.py | 107 +++++++++++++++++++++++ test/run_all_versions.sh | 1 + 8 files changed, 236 insertions(+), 2 deletions(-) create mode 100755 test/maintenance_rollback.sh create mode 100644 test/pytest/test_maintenance_rollback.py diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 35131afa0..e457a46df 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -2047,6 +2047,19 @@ latest_reading latest_reading the GROUP BY form returns what DISTINCT ON returns latest_reading latest_reading the LATERAL form returns what DISTINCT ON returns 15;16;17;18;19 2026-10-12 the document's LATERAL recipe orders by ts ascending, so it returns each device's FIRST reading latest_reading latest_reading with the btree the LATERAL form reads under a tenth of the table 15;16;17;18;19 never - latest_reading latest_reading without the btree the LATERAL form reads the table once per device 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback after a savepoint rolls back two rewrites, the rows written later in the transaction commit beside the old ones 15;16;17;18;19 2026-10-15 PgColumnarAllocateFreeSpace reuses space whatever its freeing xid, so a rolled-back maintenance function's freed bytes are overwritten +maintenance_rollback maintenance_rollback after compact rolls back, the table holds the same rows and row groups, by scan and by index 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback after compact_rewrite rolls back, the table holds the same rows and row groups, by scan and by index 15;16;17;18;19 2026-10-15 PgColumnarAllocateFreeSpace reuses space whatever its freeing xid, so a rolled-back maintenance function's freed bytes are overwritten +maintenance_rollback maintenance_rollback after recluster rolls back, the table holds the same rows and row groups, by scan and by index 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback after vacuum rolls back, the table holds the same rows and row groups, by scan and by index 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback after vacuum_sorted rolls back, the table holds the same rows and row groups, by scan and by index 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback and every function then runs for real and keeps the same rows 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback compact changed the row groups inside its transaction 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback compact_rewrite changed the row groups inside its transaction 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback premise: the table holds the rows it was given 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback recluster changed the row groups inside its transaction 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback vacuum changed the row groups inside its transaction 15;16;17;18;19 never - +maintenance_rollback maintenance_rollback vacuum_sorted changed the row groups inside its transaction 15;16;17;18;19 never - native_agg native_agg an aggregate that needs zone maps reads them 15;16;17;18;19 never - native_agg native_agg avg(int4,nulls) 15;16;17;18;19 never - native_agg native_agg control: decoding ts reads the table's blocks 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index e4aa2dc93..8d4e4ac12 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -2445,4 +2445,18 @@ suites_not_covered 230 # never 2282 + 9 = 2291 # dated 739 + 8 = 747 # 2291 + 747 == 3038 -checks_never_observed_red 2291 +# +# #1341, a maintenance function that rolls back leaves the table as it was +# (jdatcmd): 2291 -> 2302. maintenance_rollback enters with all 13 checks from +# five-major runs (13/13 on each of 15-19). Each function now runs on a fresh table +# (OffgridwithJD, review of #1419), so each date was observed on its own arm. Two +# are dated on PG17 by free space reused whatever its freeing xid: compact_rewrite's +# rollback, whose freed bytes the rewrite overwrote, and the savepoint arm. The other +# eleven arrive never, including the other four functions' arms, which that mutant +# does not reach. +# +# ledger rows 3038 + 13 = 3051 +# never 2291 + 11 = 2302 +# dated 747 + 2 = 749 +# 2302 + 749 == 3051 +checks_never_observed_red 2302 diff --git a/test/maintenance_rollback.sh b/test/maintenance_rollback.sh new file mode 100755 index 000000000..ccc030146 --- /dev/null +++ b/test/maintenance_rollback.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# +# A maintenance function that rolls back leaves the table as it was (#1341). +# compact_rewrite, recluster, compact, vacuum and vacuum_sorted each retire row +# groups and most write new ones. Retirement is a catalog delete, so a rollback +# must bring every retired group back, with the bytes it points at intact: the +# space a retired group frees may be reused only once no snapshot can need it, +# and never by the transaction that freed it, which can still roll back. +# +# The table has row groups of 1000 rows, two in five rows deleted, and one group +# deleted whole, so every function has work. Each function runs inside a +# transaction, its effect on the row groups is read there, and the transaction +# rolls back. The table must then hold the same rows, by content, and the same +# row groups, and an index scan must agree. The last arm does the same under a +# savepoint, then commits further writes, and runs every function for real. +# +# EACH FUNCTION GETS A FRESH TABLE. On one shared table, a rollback that corrupts it +# fails every function after it for that reason, not its own (OffgridwithJD, review +# of #1419). +# +# Usage: test/maintenance_rollback.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}" + +build_mr() { # the table, made again: 10 row groups of 1000, 5400 rows left + psql_run "SET pgcolumnar.stripe_row_limit = 1000; + DROP TABLE IF EXISTS mr; + CREATE TABLE mr (id int, v int, w text) USING pgcolumnar; + INSERT INTO mr SELECT g, (g * 7919) % 10007, md5(g::text) FROM generate_series(1, 10000) g; + CREATE INDEX mr_id ON mr (id); + DELETE FROM mr WHERE id % 5 IN (1, 2) OR id BETWEEN 3001 AND 4000;" +} +build_mr + +rows() { # the rows, by content + q "SELECT count(*) || '/' || md5(string_agg(id || ':' || v || ':' || w, ',' ORDER BY id)) FROM mr" | tail -1 +} +RG_SQL="SELECT count(*) || '/' || coalesce(sum(row_count), 0) FROM pgcolumnar.row_group + WHERE storage_id = pgcolumnar.get_storage_id('mr')" +by_index() { # the rows an index scan returns, by content + q "SET enable_seqscan = off; SET enable_bitmapscan = off; + SELECT count(*) || '/' || md5(string_agg(id || ':' || v || ':' || w, ',' ORDER BY id)) + FROM mr WHERE id BETWEEN 1 AND 10000" | grep -E '^[0-9]+/' | tail -1 +} +ROWS0="$(rows)" +RG0="$(q "$RG_SQL")" +check "premise: the table holds the rows it was given" "${ROWS0%%/*}|$RG0" "5400|10/10000" + +# rolled_back FUNCTION -> "|||" +rolled_back() { + local inside + inside="$(q "BEGIN; SELECT $1; $RG_SQL; ROLLBACK;" | grep -E '^[0-9]+/[0-9]+$' | tail -1)" + echo "$inside|$(rows)|$(q "$RG_SQL")|$(by_index)" +} +for f in "pgcolumnar.compact_rewrite('mr', 0.0)" "pgcolumnar.recluster('mr', 'v')" "pgcolumnar.compact('mr')" \ + "pgcolumnar.vacuum('mr')" "pgcolumnar.vacuum_sorted('mr', 'v')"; do + build_mr + got="$(rolled_back "$f")" + name="${f#pgcolumnar.}"; name="${name%%(*}" + check "$name changed the row groups inside its transaction" \ + "$([ "${got%%|*}" != "$RG0" ] && [ -n "${got%%|*}" ] && echo changed || echo "unchanged (${got%%|*})")" "changed" + check "after $name rolls back, the table holds the same rows and row groups, by scan and by index" \ + "${got#*|}" "$ROWS0|$RG0|$ROWS0" +done + +# ---- under a savepoint, then committed work, then every function for real ---------- +build_mr +psql_run "BEGIN; + SAVEPOINT s; SELECT pgcolumnar.compact_rewrite('mr', 0.0); SELECT pgcolumnar.recluster('mr', 'v'); + ROLLBACK TO s; + SET LOCAL pgcolumnar.stripe_row_limit = 1000; + INSERT INTO mr SELECT g, g, 'new' FROM generate_series(20001, 21000) g; + COMMIT;" >/dev/null +ROWS1="$(rows)" +check "after a savepoint rolls back two rewrites, the rows written later in the transaction commit beside the old ones" \ + "$(q "SELECT count(*) FILTER (WHERE w = 'new') || '/' || count(*) FROM mr" | tail -1)|$(by_index | cut -d/ -f1)" "1000/6400|5400" +for f in "pgcolumnar.compact_rewrite('mr', 0.0)" "pgcolumnar.recluster('mr', 'v')" "pgcolumnar.compact('mr')" \ + "pgcolumnar.vacuum('mr')"; do + psql_run "SELECT $f;" >/dev/null +done +check "and every function then runs for real and keeps the same rows" "$(rows)" "$ROWS1" + +pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 0b3c32436..6878307f0 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -165,6 +165,7 @@ behaviour, the source of that number is named. - [117. test_large_values.py: megabyte-scale values are stored exactly](#117-test_large_valuespy-megabyte-scale-values-are-stored-exactly) - [118. test_recovery_minimal_wal.py: tables that skip WAL survive a crash](#118-test_recovery_minimal_walpy-tables-that-skip-wal-survive-a-crash) - [119. test_statement_self_visibility.py: a statement does not see its own writes](#119-test_statement_self_visibilitypy-a-statement-does-not-see-its-own-writes) +- [120. test_maintenance_rollback.py: a maintenance function that rolls back leaves the table as it was](#120-test_maintenance_rollbackpy-a-maintenance-function-that-rolls-back-leaves-the-table-as-it-was) ## 1. How to read a test in here @@ -7021,3 +7022,13 @@ The pytest twin of `test/statement_self_visibility.sh` (#1341), every check by t | test | what it holds | | --- | --- | | `test_a_statement_does_not_see_its_own_writes` | the four shapes run to completion and answer as heap does, and the mid-statement flush | + +## 120. test_maintenance_rollback.py: a maintenance function that rolls back leaves the table as it was + +The pytest twin of `test/maintenance_rollback.sh` (#1341), every check by the shell's name. `compact_rewrite`, `recluster`, `compact`, `vacuum` and `vacuum_sorted` each retire row groups, and most write new ones. Each runs inside a transaction on a table with row groups of 1000 rows, two rows in five deleted, and one group deleted whole. Its effect on the row groups is read inside the transaction, and the transaction rolls back. The table must then hold the same rows by content and the same row groups, and an index scan must agree. The same then runs under a savepoint, later writes commit, and every function runs for real. The rollback is issued as SQL statements, because `pgc_conn` is a watching wrapper and an autocommit flag set on it does not reach the connection. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_a_maintenance_function_that_rolls_back_leaves_the_table_as_it_was` | each function's effect inside its transaction, the table after each rollback by scan and by index, the savepoint case, and the real runs | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 75c94bd18..1dbff96a7 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -692,4 +692,5 @@ guard_tests 435 # cluster_tests 578 -> 579 for #1341's test_the_documented_refusals_change_nothing in # cluster_tests 579 -> 581 for #1341 items 56 and 55: test_the_row_identity_lock_cannot_be_reached_from_sql # and test_bulk_parallel_writer_set_by_hand_still_serialises_a_first_write in -cluster_tests 581 +# cluster_tests 581 -> 582 for #1341's test_maintenance_rollback.py, one test, by collection +cluster_tests 582 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index baa7f5f1b..81553c831 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -114,6 +114,7 @@ "index_fetch_penalty_crossover", "index_fetch_penalty_width", "latest_reading", + "maintenance_rollback", "native_chunk_length_bound", "native_delete_vector_index", "native_delete_visibility_paths", diff --git a/test/pytest/test_maintenance_rollback.py b/test/pytest/test_maintenance_rollback.py new file mode 100644 index 000000000..9d4925209 --- /dev/null +++ b/test/pytest/test_maintenance_rollback.py @@ -0,0 +1,107 @@ +"""A maintenance function that rolls back leaves the table as it was (#1341). The +pytest twin of `test/maintenance_rollback.sh`, every check by the shell's name. + +compact_rewrite, recluster, compact, vacuum and vacuum_sorted each retire row groups, +and most write new ones. Retirement is a catalog delete, so a rollback must bring +every retired group back with the bytes it points at intact. The space a retired +group frees may be reused only once no snapshot can need it, and never by the +transaction that freed it, which can still roll back. + +The table has row groups of 1000 rows, two in five rows deleted, and one group +deleted whole, so every function has work. Each runs inside a transaction, its effect +on the row groups is read there, and the transaction rolls back. The table must then +hold the same rows by content and the same row groups, and an index scan must agree. +The last arms do the same under a savepoint, commit further writes, and run every +function for real. +""" +import psycopg + +ROWS = ("SELECT count(*) || '/' || md5(string_agg(id || ':' || v || ':' || w, ',' ORDER BY id)) " + "FROM mr") +GROUPS = ("SELECT count(*) || '/' || coalesce(sum(row_count), 0) FROM pgcolumnar.row_group " + "WHERE storage_id = pgcolumnar.get_storage_id('mr')") + + +def _one(conn, sql): + """-> the first column of the only row, or the error's first line.""" + try: + with conn.cursor() as cur: + cur.execute(sql) + return cur.fetchone()[0] + except psycopg.Error as exc: + return f"ERROR: {str(exc).splitlines()[0]}" + + +def _by_index(conn): + with conn.cursor() as cur: + cur.execute("SET enable_seqscan = off") + cur.execute("SET enable_bitmapscan = off") + got = _one(conn, ROWS + " WHERE id BETWEEN 1 AND 10000") + with conn.cursor() as cur: + cur.execute("RESET enable_seqscan") + cur.execute("RESET enable_bitmapscan") + return got + + +def _build_mr(conn): + """The table, made again: 10 row groups of 1000, 5400 rows left. Each function + gets a fresh one, so a rollback that corrupts it fails only its own arm.""" + with conn.cursor() as cur: + cur.execute("SET pgcolumnar.stripe_row_limit = 1000") + cur.execute("DROP TABLE IF EXISTS mr") + cur.execute("CREATE TABLE mr (id int, v int, w text) USING pgcolumnar") + cur.execute("INSERT INTO mr SELECT g, (g * 7919) % 10007, md5(g::text) FROM generate_series(1, 10000) g") + cur.execute("CREATE INDEX mr_id ON mr (id)") + cur.execute("DELETE FROM mr WHERE id % 5 IN (1, 2) OR id BETWEEN 3001 AND 4000") + cur.execute("RESET pgcolumnar.stripe_row_limit") + + +def test_a_maintenance_function_that_rolls_back_leaves_the_table_as_it_was(pgc_conn, expect): + funcs = ("compact_rewrite('mr', 0.0)", "recluster('mr', 'v')", "compact('mr')", + "vacuum('mr')", "vacuum_sorted('mr', 'v')") + _build_mr(pgc_conn) + rows0, groups0 = _one(pgc_conn, ROWS), _one(pgc_conn, GROUPS) + expect.text(f"{rows0.split('/')[0]}|{groups0}", "5400|10/10000", + "premise: the table holds the rows it was given") + + for f in funcs: + name = f.split("(")[0] + _build_mr(pgc_conn) + # BEGIN and ROLLBACK as statements: pgc_conn is a watching wrapper, and an + # autocommit flag set on it would not reach the connection, so the + # function would commit and the rollback would do nothing. + with pgc_conn.cursor() as cur: + cur.execute("BEGIN") + try: + cur.execute(f"SELECT pgcolumnar.{f}") + inside = _one(pgc_conn, GROUPS) + finally: + cur.execute("ROLLBACK") + expect.text("changed" if inside != groups0 and "ERROR" not in inside else f"unchanged ({inside})", + "changed", f"{name} changed the row groups inside its transaction") + expect.text(f"{_one(pgc_conn, ROWS)}|{_one(pgc_conn, GROUPS)}|{_by_index(pgc_conn)}", + f"{rows0}|{groups0}|{rows0}", + f"after {name} rolls back, the table holds the same rows and row groups, by scan and by index") + + _build_mr(pgc_conn) + with pgc_conn.cursor() as cur: + cur.execute("BEGIN") + try: + cur.execute("SAVEPOINT s") + cur.execute("SELECT pgcolumnar.compact_rewrite('mr', 0.0)") + cur.execute("SELECT pgcolumnar.recluster('mr', 'v')") + cur.execute("ROLLBACK TO s") + cur.execute("SET LOCAL pgcolumnar.stripe_row_limit = 1000") + cur.execute("INSERT INTO mr SELECT g, g, 'new' FROM generate_series(20001, 21000) g") + cur.execute("COMMIT") + except psycopg.Error: + cur.execute("ROLLBACK") + raise + rows1 = _one(pgc_conn, ROWS) + new = _one(pgc_conn, "SELECT count(*) FILTER (WHERE w = 'new') || '/' || count(*) FROM mr") + expect.text(f"{new}|{str(_by_index(pgc_conn)).split('/')[0]}", "1000/6400|5400", + "after a savepoint rolls back two rewrites, the rows written later in the transaction commit beside the old ones") + for f in ("compact_rewrite('mr', 0.0)", "recluster('mr', 'v')", "compact('mr')", "vacuum('mr')"): + with pgc_conn.cursor() as cur: + cur.execute(f"SELECT pgcolumnar.{f}") + expect.text(_one(pgc_conn, ROWS), rows1, "and every function then runs for real and keeps the same rows") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 7e53924d1..9708bed56 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -141,6 +141,7 @@ SUITES=( logical_decoding_source logical_subscriber maintenance_due + maintenance_rollback native_agg native_agg_addcolumn native_agg_deletes