From 03c31deb3d58faaedb35e7babbe28fd8bd90ade2 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 16 Oct 2026 23:16:41 +0000 Subject: [PATCH] test: native_gap gains its pytest twin, and its indexed lookup reads the index (#1341) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shell's "indexed lookup after gap" counted with the defaults, which plan an Index Only Scan. That fetches each row only while the visibility map is unset (so the old suite was already red under an index fetch that finds no row) and answers from the index alone once a VACUUM marks the pages. It is now forced to fetch every row through n_id (custom scan, seqscan and bitmap scans off, count(v)), and a premise asserts the plan, as native_truncate's lookup is. A guard, not a fix. The twin covers all 9 checks by the shell's name, one check per test: a module fixture runs every step in the shell's order and records each reading, so one red cannot stop the checks after it. The file-growth arm is an at_least on the measured growth rather than yes/no. Four runs on PG18 (none, gap fill removed as before 6a165821, the test hook advancing nothing, index fetch finding no row) redden the same names in the shell and in the twin as shipped: 0, 6, 1 and 1. Ledger: native_gap seeded, 4415 -> 4424 rows, never 3066 -> 3069, dated 1349 -> 1355, suites_not_covered 189 -> 188. cluster_tests 754 -> 763, TESTS.md ยง177, and the stem leaves unported_suites.txt. Co-Authored-By: Claude Opus 5.5 --- test/check_ledger.tsv | 9 ++ test/check_ledger_budget.txt | 17 +++- test/native_gap.sh | 10 ++- test/pytest/TESTS.md | 21 +++++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_compare_to_bash.py | 1 + test/pytest/test_native_gap.py | 135 ++++++++++++++++++++++++++++ test/pytest/unported_suites.txt | 1 - 8 files changed, 193 insertions(+), 5 deletions(-) create mode 100644 test/pytest/test_native_gap.py diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index fe55cb19..a5e51fee 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -2566,6 +2566,15 @@ native_fetch_projection native_fetch_projection premise: there is a needed-set m native_fetch_projection native_fetch_projection the all-covering projection reconstructs every row 15;16;17;18;19 never - native_fetch_projection native_fetch_projection the projection reads every row to start with 15;16;17;18;19 never - native_fetch_projection native_fetch_projection the wide fixture has 41 columns 15;16;17;18;19 never - +native_gap native_gap baseline parity 15;16;17;18;19 never - +native_gap native_gap baseline row count 15;16;17;18;19 never - +native_gap native_gap gap was physically materialized 15;16;17;18;19 2026-10-16 columnar_storage.c: PgColumnarWriteLogicalData does not fill a gap beyond EOF (6a165821 reverted), so the write lands short of its block and errors;columnar_vacuum.c: debug_advance_reserved_offset advances nothing, so no gap is made +native_gap native_gap indexed lookup after gap 15;16;17;18;19 2026-10-16 columnar_storage.c: PgColumnarWriteLogicalData does not fill a gap beyond EOF (6a165821 reverted), so the write lands short of its block and errors;columnar_tableam.c: an index fetch finds no row +native_gap native_gap parity after delete on gapped layout 15;16;17;18;19 2026-10-16 columnar_storage.c: PgColumnarWriteLogicalData does not fill a gap beyond EOF (6a165821 reverted), so the write lands short of its block and errors +native_gap native_gap parity after gapped insert 15;16;17;18;19 2026-10-16 columnar_storage.c: PgColumnarWriteLogicalData does not fill a gap beyond EOF (6a165821 reverted), so the write lands short of its block and errors +native_gap native_gap parity after post-gap insert 15;16;17;18;19 2026-10-16 columnar_storage.c: PgColumnarWriteLogicalData does not fill a gap beyond EOF (6a165821 reverted), so the write lands short of its block and errors +native_gap native_gap premise: the indexed lookup reads n_id 15;16;17;18;19 never - +native_gap native_gap row count after gapped insert 15;16;17;18;19 2026-10-16 columnar_storage.c: PgColumnarWriteLogicalData does not fill a gap beyond EOF (6a165821 reverted), so the write lands short of its block and errors native_groupagg native_groupagg a NULL key counts as a group in the bucket bound: four groups under a cap of 3 are declined and all answered 15;16;17;18;19 2026-10-16 the bucket bound left out the NULL group and the runtime cap was off, so the node took the query and answered it (mutation, #1339) native_groupagg native_groupagg a cached plan run under a lower cap stops at the runtime cap, with its hint 15;16;17;18;19 never - native_groupagg native_groupagg a date_bin key is bounded as a truncating key is 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 77c228e3..9df35a64 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 189 +suites_not_covered 188 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -3560,4 +3560,17 @@ suites_not_covered 189 # never 3055 + 11 = 3066 # dated 1342 + 7 = 1349 # 3066 + 1349 == 4415 -checks_never_observed_red 3066 +# +# #1341, native_gap seeded, with its twin (OffgridwithJD): 3066 -> 3069, and +# suites_not_covered 189 -> 188. The suite had no rows. It enters with all 9 checks +# from five-major runs (9/9 on each of 15-19). 6 are dated by three mutations: the +# gap fill removed (6a165821's pre-fix write path, where the write lands short of +# its block and errors), the test hook advancing nothing, and an index fetch finding +# no row (the indexed lookup, now forced through n_id with a premise). Three arrive +# never: the two baseline checks and the index premise. +# +# ledger rows 4415 + 9 = 4424 +# never 3066 + 3 = 3069 +# dated 1349 + 6 = 1355 +# 3069 + 1355 == 4424 +checks_never_observed_red 3069 diff --git a/test/native_gap.sh b/test/native_gap.sh index 155a47e1..9b9e81d7 100755 --- a/test/native_gap.sh +++ b/test/native_gap.sh @@ -56,8 +56,16 @@ psql_run "INSERT INTO h SELECT g, md5(g::text) FROM generate_series(6001, 7000) psql_run "INSERT INTO n SELECT g, md5(g::text) FROM generate_series(6001, 7000) g;" check "parity after post-gap insert" "$(hash_n)" "$(hash_h)" psql_run "CREATE INDEX n_id ON n (id);" +# Forced to fetch every row through n_id, and asserted (#1341). With the defaults +# this count plans an Index Only Scan (PG17 and PG18), which fetches each row only +# while the visibility map leaves its page unmarked: once a VACUUM marks them it +# answers from the index alone, whatever the fetch does. count(v), not count(*), +# with the custom scan, seqscan and bitmap scans off, so the plan is an Index Scan. +IDX_SETS="SET enable_seqscan = off; SET enable_bitmapscan = off; SET pgcolumnar.enable_custom_scan = off;" +check "premise: the indexed lookup reads n_id" \ + "$(pgc_uses_row_fetch "$IDX_SETS" "SELECT count(v) FROM n WHERE id BETWEEN 1 AND 7000")" yes check "indexed lookup after gap" \ - "$(q 'SELECT count(*) FROM n WHERE id BETWEEN 1 AND 7000;')" "7000" + "$(q "$IDX_SETS SELECT count(v) FROM n WHERE id BETWEEN 1 AND 7000;" | tail -1)" "7000" # a delete + reader still resolves correctly across the gapped layout. psql_run "DELETE FROM h WHERE id % 7 = 0;" diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index cc0c5544..0500a159 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -222,6 +222,7 @@ behaviour, the source of that number is named. - [174. test_sorted_projection.py: vacuum_sorted reorders storage and changes no answer](#174-test_sorted_projectionpy-vacuum_sorted-reorders-storage-and-changes-no-answer) - [175. test_native_ios.py: an index-only scan skips the fetch on all-visible row groups](#175-test_native_iospy-an-index-only-scan-skips-the-fetch-on-all-visible-row-groups) - [176. test_native_truncate.py: end-truncation shrinks the file and loses no row](#176-test_native_truncatepy-end-truncation-shrinks-the-file-and-loses-no-row) +- [177. test_native_gap.py: a write beyond EOF fills the gap and loses no row](#177-test_native_gappy-a-write-beyond-eof-fills-the-gap-and-loses-no-row) ## 1. How to read a test in here @@ -7836,3 +7837,23 @@ The pytest twin of `test/native_truncate.sh` (ported for #1341), every check by | `test_truncate_shrinks_the_file_and_keeps_every_row` | a truncate with nothing freed returns 0; after delete and compact it returns blocks, the file shrinks, the rows match the heap, and a second truncate returns 0 | | `test_the_table_stays_usable_and_the_guc_turns_it_off` | reinsert and an index lookup through the index after truncation; with the GUC off truncate returns 0 and the file keeps its size, with it on the file shrinks | | `test_truncate_refuses_a_transaction_block_and_reuse_stays_clean` | refused inside BEGIN with the file and rows unchanged; truncate, reinsert and compaction keep parity | + +## 177. test_native_gap.py: a write beyond EOF fills the gap and loses no row + +The pytest twin of `test/native_gap.sh` (ported for #1341), every check by the shell's name. The metapage highwater can sit ahead of the physical EOF, which is the state an aborted or crash-interrupted end-truncation leaves. A write whose target block lies beyond EOF must fill the gap with empty pages and succeed. A test-only hook, bound by the test as the shell binds it, pushes the highwater 50 pages past EOF. The next insert must keep every row against a heap mirror and grow the file by at least 40 pages. The table then stays usable: another insert, an index lookup forced through the index and asserted to read it, and a delete. + +### Every test + +Each check is its own test over a module fixture that runs every step in the shell's order and records each reading, so one red cannot stop the checks after it. + +| test | what it holds | +| --- | --- | +| `test_baseline_row_count` | 3000 rows before the gap | +| `test_baseline_parity` | and they match the heap | +| `test_row_count_after_gapped_insert` | 6000 rows after the insert across a 50-page gap | +| `test_parity_after_gapped_insert` | and they match the heap | +| `test_gap_was_physically_materialized` | the file grew by more than 40 pages | +| `test_parity_after_post_gap_insert` | a later insert keeps parity | +| `test_the_indexed_lookup_reads_n_id` | the forced lookup plans an Index Scan on `n_id` | +| `test_indexed_lookup_after_gap` | and finds all 7000 rows | +| `test_parity_after_delete_on_gapped_layout` | a delete keeps parity | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index e3dbd899..92556a1b 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -823,4 +823,6 @@ guard_tests 440 # one test, by collection over the cluster files on this tree. # cluster_tests 751 -> 754 for #1341's test_native_truncate.py, the twin of native_truncate.sh: # three tests, by collection over the cluster files on this tree. -cluster_tests 754 +# cluster_tests 754 -> 763 for #1341's test_native_gap.py, the twin of native_gap.sh: +# nine tests, one per check, by collection over the cluster files on this tree. +cluster_tests 763 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 9ac04205..1d512bb1 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -135,6 +135,7 @@ "native_fetch_coalesce", "native_fetch_position", "native_fetch_projection", + "native_gap", "native_groupagg", "native_groupagg_batch", "native_index", diff --git a/test/pytest/test_native_gap.py b/test/pytest/test_native_gap.py new file mode 100644 index 00000000..ecc2eb23 --- /dev/null +++ b/test/pytest/test_native_gap.py @@ -0,0 +1,135 @@ +"""The gap-tolerant write path (Phase F). The pytest twin of `test/native_gap.sh`, every +check by the shell's name. + +The metapage highwater can sit ahead of the physical EOF: the state an aborted or +crash-interrupted end-truncation leaves. A write whose target block is beyond EOF fills +the gap with empty pages and succeeds. A test-only hook, bound here as the shell binds +it, pushes the highwater 50 pages past EOF; then the writes across the gap succeed, the +rows match a heap mirror, the file grows by the gap, and the table stays usable: more +inserts, an index read through, a delete. The fixture runs every step and records +each reading, and each check is its own test, so one red cannot hide the next. +""" +import psycopg +import pytest + +HASH = ("SELECT coalesce(md5(string_agg(t, chr(10) ORDER BY t)), $e$EMPTY$e$) " + "FROM (SELECT _row::text AS t FROM ({}) _row) _s") +IDX_SETS = ("SET enable_seqscan = off", "SET enable_bitmapscan = off", "SET pgcolumnar.enable_custom_scan = off") +LOOKUP = "SELECT count(v) FROM n WHERE id BETWEEN 1 AND 7000" + + +def _one(c, sql): + try: + return str(c.execute(sql).fetchone()[0]) + except psycopg.Error as exc: + return f"error: {exc.diag.message_primary}" + + +def _size(c): + return int(c.execute("SELECT pg_relation_size('n')").fetchone()[0]) + + +def _hashes(c): + """-> (columnar, heap) set digests of the rows.""" + return _one(c, HASH.format("SELECT id, v FROM n")), _one(c, HASH.format("SELECT id, v FROM h")) + + +def _both(c, sql): + """Run the statement on the heap mirror, then on the columnar table; a write the + gap refuses is left for the checks to see, as psql_run leaves it.""" + c.execute(sql.format(t="h")) + try: + c.execute(sql.format(t="n")) + except psycopg.Error as exc: + print(f"-- columnar write failed: {exc.diag.message_primary}") + + +@pytest.fixture(scope="module") +def gap(pgc_cluster): + """Every step of the shell, in its order, with each reading recorded rather than + asserted, so each check is its own test and one red cannot stop the rest (#1517 + review): 3000 rows, the highwater pushed 50 pages past EOF, 3000 more across the + gap, 1000 more after it, an index read through, a delete.""" + db = "pgc_gap" + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {db} WITH (FORCE)") + a.execute(f"CREATE DATABASE {db}") + try: + with psycopg.connect(pgc_cluster.dsn(dbname=db), autocommit=True) as c: + c.execute("CREATE EXTENSION pgcolumnar") + # the internal test hook, deliberately not in the shipped catalog + c.execute("CREATE FUNCTION pgcolumnar.debug_advance_reserved_offset(regclass, int) " + "RETURNS void AS 'pgcolumnar', 'pgcolumnar_debug_advance_reserved_offset' LANGUAGE C") + c.execute("CREATE TABLE h (id int, v text)") + c.execute("CREATE TABLE n (id int, v text) USING pgcolumnar") + c.execute("SELECT pgcolumnar.set_options('n', stripe_row_limit => 1000, chunk_group_row_limit => 1000)") + r = {} + _both(c, "INSERT INTO {t} SELECT g, md5(g::text) FROM generate_series(1, 3000) g") + r["baseline count"], r["baseline"] = _one(c, "SELECT count(*) FROM n"), _hashes(c) + before = _size(c) + + # the highwater 50 pages past the physical EOF: the next write targets beyond it + c.execute("SELECT pgcolumnar.debug_advance_reserved_offset('n', 50)") + _both(c, "INSERT INTO {t} SELECT g, md5(g::text) FROM generate_series(3001, 6000) g") + r["gapped count"], r["gapped"] = _one(c, "SELECT count(*) FROM n"), _hashes(c) + r["growth"] = _size(c) - before + print(f"-- file size: before-gap={before} after-gap={before + r['growth']}") + + _both(c, "INSERT INTO {t} SELECT g, md5(g::text) FROM generate_series(6001, 7000) g") + r["post-gap"] = _hashes(c) + c.execute("CREATE INDEX n_id ON n (id)") + # forced to fetch every row through n_id: by default this count is an Index + # Only Scan, which stops fetching once a VACUUM marks the pages all-visible + for s in IDX_SETS: + c.execute(s) + try: + r["plan"] = "\n".join(row[0] for row in c.execute(f"EXPLAIN (COSTS OFF) {LOOKUP}")) + r["lookup"] = _one(c, LOOKUP) + finally: + for g in ("enable_seqscan", "enable_bitmapscan", "pgcolumnar.enable_custom_scan"): + c.execute(f"RESET {g}") + + _both(c, "DELETE FROM {t} WHERE id % 7 = 0") + r["deleted"] = _hashes(c) + yield r + finally: + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {db} WITH (FORCE)") + + +def test_baseline_row_count(gap, expect): + expect.text(gap["baseline count"], "3000", "baseline row count") + + +def test_baseline_parity(gap, expect): + expect.text(*gap["baseline"], "baseline parity") + + +def test_row_count_after_gapped_insert(gap, expect): + expect.text(gap["gapped count"], "6000", "row count after gapped insert") + + +def test_parity_after_gapped_insert(gap, expect): + expect.text(*gap["gapped"], "parity after gapped insert") + + +def test_gap_was_physically_materialized(gap, expect): + # the 50-page gap and the new rows, with slack: at least 40 pages of growth + expect.at_least(gap["growth"], 40 * 8192 + 1, "gap was physically materialized") + + +def test_parity_after_post_gap_insert(gap, expect): + expect.text(*gap["post-gap"], "parity after post-gap insert") + + +def test_the_indexed_lookup_reads_n_id(gap, expect): + expect.text("yes" if "Index Scan using n_id" in gap["plan"] else "no", "yes", + "premise: the indexed lookup reads n_id") + + +def test_indexed_lookup_after_gap(gap, expect): + expect.text(gap["lookup"], "7000", "indexed lookup after gap") + + +def test_parity_after_delete_on_gapped_layout(gap, expect): + expect.text(*gap["deleted"], "parity after delete on gapped layout") diff --git a/test/pytest/unported_suites.txt b/test/pytest/unported_suites.txt index ae0bd9e6..8f7c7cc4 100644 --- a/test/pytest/unported_suites.txt +++ b/test/pytest/unported_suites.txt @@ -88,7 +88,6 @@ native_fetch_interrupt native_fetch_sort_context native_fold_deferral native_fold_skipguard -native_gap native_groupagg_wide_cost native_late_materialization native_lazy_slot