diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index f0e7bbd10..1c9500e6a 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -4290,6 +4290,19 @@ range_pruning range_pruning premise: the range type collates differently from it range_pruning range_pruning so does a containment probe 15;16;17;18;19 never - range_pruning range_pruning the overlap qual is pushed down, where it used to be dropped 15;16;17;18;19 never - range_pruning range_pruning the same for a containment probe far to its right 15;16;17;18;19 2026-10-17 columnar_write_state.c: the writer ignores an unbounded range, so its unit records only the largest finite upper bound +recluster_gate recluster_gate a repeat recluster by the new key is a no-op 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster_online skips record_online_sorted_extent, so no extent or key is recorded;recluster's #415 self-gate removed, so every call rewrites +recluster_gate recluster_gate a third identical recluster is also a no-op 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster_online skips record_online_sorted_extent, so no extent or key is recorded;recluster's #415 self-gate removed, so every call rewrites +recluster_gate recluster_gate and it folds the new data back into the run (0 appended after) 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster's #415 gate over-applied (if (true || sameKey ...)), so every recluster returns 0 and does nothing;columnar_vacuum.c: recluster_online skips record_online_sorted_extent, so no extent or key is recorded;the gate ignores groups appended past the recorded run +recluster_gate recluster_gate and it leaves the physical layout byte-identical (group offsets unmoved) 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster_online skips record_online_sorted_extent, so no extent or key is recorded;recluster's #415 self-gate removed, so every call rewrites +recluster_gate recluster_gate and sort_status now reports the new key 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster's #415 gate over-applied (if (true || sameKey ...)), so every recluster returns 0 and does nothing;columnar_vacuum.c: recluster_online skips record_online_sorted_extent, so no extent or key is recorded;the gate ignores the key, so a new key reads as already clustered +recluster_gate recluster_gate no-op recluster preserved the exact row set (columnar == heap mirror) 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster_online skips the first sorted row it writes back +recluster_gate recluster_gate premise: the insert appended unsorted groups 15;16;17;18;19 never - +recluster_gate recluster_gate recluster after appends does real work again (>0) 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster's #415 gate over-applied (if (true || sameKey ...)), so every recluster returns 0 and does nothing;the gate ignores groups appended past the recorded run +recluster_gate recluster_gate recluster by a NEW key is NOT a no-op -- it re-clusters (>0) 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster's #415 gate over-applied (if (true || sameKey ...)), so every recluster returns 0 and does nothing;the gate ignores the key, so a new key reads as already clustered +recluster_gate recluster_gate recluster by the same key on an unchanged table reclusters 0 groups 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster_online skips record_online_sorted_extent, so no extent or key is recorded;recluster's #415 self-gate removed, so every call rewrites +recluster_gate recluster_gate sort_status now reports the clustering key (was NULL before #415) 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster's #415 gate over-applied (if (true || sameKey ...)), so every recluster returns 0 and does nothing;columnar_vacuum.c: recluster_online skips record_online_sorted_extent, so no extent or key is recorded +recluster_gate recluster_gate the first recluster does real work (>0 groups) 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster's #415 gate over-applied (if (true || sameKey ...)), so every recluster returns 0 and does nothing +recluster_gate recluster_gate the run covers the whole relation (0 appended groups) 15;16;17;18;19 2026-10-21 columnar_vacuum.c: recluster's #415 gate over-applied (if (true || sameKey ...)), so every recluster returns 0 and does nothing;columnar_vacuum.c: recluster_online skips record_online_sorted_extent, so no extent or key is recorded recovery recovery premise: s2's INSERT ran in its open transaction before the crash 15;16;17;18;19 never - recovery recovery s1 aggregate 15;16;17;18;19 2026-10-16 columnar_storage.c: a stripe data page written with no WAL record recovery recovery s1 col count 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 92c1f333c..aa0ea0d13 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 125 +suites_not_covered 124 # # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become # one. @@ -4963,4 +4963,25 @@ suites_not_covered 125 # never 3648 + 2 = 3650 # dated 1925 + 5 = 1930 # 3650 + 1930 == 5580 -checks_never_observed_red 3650 +# +# #1341, recluster_gate seeded, with its twin (jdatcmd): 3505 -> 3506, and +# suites_not_covered 149 -> 148. The suite had no rows. It enters with 13 checks from +# five-major runs (13/13 on each of 15-19, shell and twin alike), after two fixes. +# The row-set check compared rg_t with a heap copied from rg_t, so it agreed with +# whatever the reclusters did: with a row corrupted before the copy, it passed. The +# mirror is now built from the generator. And the layout comparison agreed on two +# failed reads; it now needs both. Three checks lost their premise: prefix under +# #1339 (OffgridwithJD's review): the first recluster's return, sort_status's +# recorded key (#415) and the recorded extent (#311) are recluster's own behaviour, +# and a one-line change reddens each. 12 are dated by six mutations, each reddening +# the same names in both twins: the gate removed, appended groups ignored, the key +# ignored, no extent or key recorded, the gate over-applied so every call does +# nothing, and the first sorted row dropped on write-back. 1 arrives never: the +# premise that the insert appended unsorted groups, green under the gate removed, no +# record, the gate over-applied and the dropped row. +# +# ledger rows 5580 + 13 = 5593 +# never 3650 + 1 = 3651 +# dated 1930 + 12 = 1942 +# 3651 + 1942 == 5593 +checks_never_observed_red 3651 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 8c1297e60..bbdbfc758 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -288,6 +288,7 @@ behaviour, the source of that number is named. - [240. test_fsst_margin.py: the FSST keep/drop margin decides, and never changes content](#240-test_fsst_marginpy-the-fsst-keepdrop-margin-decides-and-never-changes-content) - [241. test_index_delete_liveness.py: nbtree's deletion passes never destroy an entry for a live row](#241-test_index_delete_livenesspy-nbtrees-deletion-passes-never-destroy-an-entry-for-a-live-row) - [242. test_rewrite_group_scan.py: a rewrite reads each row group once, not once per row](#242-test_rewrite_group_scanpy-a-rewrite-reads-each-row-group-once-not-once-per-row) +- [243. test_recluster_gate.py: recluster skips its rewrite when the table is already this key's run](#243-test_recluster_gatepy-recluster-skips-its-rewrite-when-the-table-is-already-this-keys-run) ## 1. How to read a test in here @@ -9675,3 +9676,27 @@ A rewrite reads `finished` only when compact_rewrite returned a positive count. | `test_a_group_under_the_cap_still_rewrites_correctly` | a group under the cap still rewrites correctly | | `test_the_index_lookup_reads_the_index` | the index lookup's plan reads rgs_id | | `test_an_index_lookup_finds_a_rewritten_row` | an index lookup finds a rewritten row | + +## 243. test_recluster_gate.py: recluster skips its rewrite when the table is already this key's run + +The pytest twin of `test/recluster_gate.sh` (ported for #1341), every check by the shell's name. recluster returns the number of row groups it rewrote, and its self-gate (#415) makes a repeat on an unchanged table rewrite nothing; an append or a new key makes it work again. On a private database. + +### Every test + +The row-set check reads a heap mirror built from the generator, not copied from the table under test. A layout comparison needs both readings. + +| test | what it holds | +| --- | --- | +| `test_the_first_gate_recluster_does_real_work` | the first recluster does real work (>0 groups) | +| `test_sort_status_reports_the_clustering_key` | sort_status now reports the clustering key (was NULL before #415) | +| `test_the_recluster_run_covers_the_whole_relation` | the run covers the whole relation (0 appended groups) | +| `test_a_repeat_on_an_unchanged_table_reclusters_nothing` | recluster by the same key on an unchanged table reclusters 0 groups | +| `test_the_layout_is_byte_identical` | and it leaves the physical layout byte-identical (group offsets unmoved) | +| `test_a_third_identical_recluster_is_a_no_op` | a third identical recluster is also a no-op | +| `test_the_insert_appended_unsorted_groups` | premise: the insert appended unsorted groups | +| `test_a_recluster_after_appends_does_work` | recluster after appends does real work again (>0) | +| `test_recluster_folds_the_tail_back_into_the_run` | and it folds the new data back into the run (0 appended after) | +| `test_a_recluster_by_a_new_key_does_work` | recluster by a NEW key is NOT a no-op -- it re-clusters (>0) | +| `test_sort_status_reports_the_new_key` | and sort_status now reports the new key | +| `test_a_repeat_by_the_new_key_is_a_no_op` | a repeat recluster by the new key is a no-op | +| `test_the_no_op_reclusters_preserved_the_row_set` | no-op recluster preserved the exact row set (columnar == heap mirror) | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 71f8bdf80..eb97df84e 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -983,4 +983,6 @@ guard_tests 467 # 9 tests, one per check, by collection over the cluster files. # cluster_tests 2162 -> 2169 for #1341's test_rewrite_group_scan.py, the twin of rewrite_group_scan.sh: # 7 tests, one per check, by collection over the cluster files. -cluster_tests 2169 +# cluster_tests 2169 -> 2182 for #1341's test_recluster_gate.py, the twin of recluster_gate.sh: +# 13 tests, one per check, by collection over the cluster files. +cluster_tests 2182 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index bbf8f3890..23df4bd7e 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -91,6 +91,7 @@ "smoke", "sort_status", "sort_status_privilege", + "recluster_gate", "rewrite_group_scan", "index_delete_liveness", "fsst_margin", diff --git a/test/pytest/test_recluster_gate.py b/test/pytest/test_recluster_gate.py new file mode 100644 index 000000000..6323e7b8f --- /dev/null +++ b/test/pytest/test_recluster_gate.py @@ -0,0 +1,153 @@ +"""pgcolumnar.recluster() skips its rewrite when the table is already this key's run with +nothing appended (#415). The pytest twin of `test/recluster_gate.sh`, every check by the +shell's name (#1341). + +recluster returns the number of row groups it rewrote. Asserted: the first recluster +does work and records the key; repeating it on an unchanged table rewrites nothing and +leaves the layout byte-identical; an append makes it work again and fold the tail into +the run; a new key re-clusters and is recorded; and the no-op calls left the row set +exactly as the generator wrote it. + +THE ROW-SET CHECK reads a heap mirror built from the generator, not one copied from the +table under test: a copy agrees with whatever the reclusters did, and with a row +corrupted before the copy the shell's old form passed. A layout comparison needs both +readings, since two failed reads agreed. + +ON A PRIVATE DATABASE, built in the shell's order by a module fixture. +""" +import psycopg +import pytest + +from pgc_vacuity import QUERY_ERROR, query_error + +DB = "pgc_recluster_gate" +CG = 1000 +ROWSET = "SELECT count(*)||'/'||sum(a)||'/'||sum(b)||'/'||md5(string_agg(pad,',' ORDER BY b)) FROM {t}" +LAYOUT = ("SELECT md5(string_agg(stripeid::text||':'||fileoffset::text||':'||rowcount::text, ',' " + "ORDER BY stripeid)) FROM pgcolumnar.stats('rg_t')") + + +class _Db: + def __init__(self, dsn): + self.dsn = dsn + + def run(self, sql): + try: + with psycopg.connect(self.dsn, autocommit=True) as conn: + conn.execute(sql) + except psycopg.Error as exc: + print(f"-- step failed: {str(exc).splitlines()[0]}") + + def q(self, sql): + """-> the first column as psql -At prints it, or a query_error().""" + try: + with psycopg.connect(self.dsn, autocommit=True) as conn: + row = conn.execute(sql).fetchone() + except psycopg.Error as exc: + return query_error(str(exc).splitlines()[0] if str(exc) else type(exc).__name__) + if row is None or row[0] is None: + return "" + return ("t" if row[0] else "f") if isinstance(row[0], bool) else str(row[0]) + + def recluster(self, key): + v = self.q(f"SELECT pgcolumnar.recluster('rg_t','{key}')") + return int(v) if v.isdigit() else v + + +def _work(n): + return ("yes" if n > 0 else "no") if isinstance(n, int) else f"no ({n})" + + +@pytest.fixture(scope="module") +def rc(pgc_cluster): + 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: + d = _Db(pgc_cluster.dsn(dbname=DB)) + d.run("CREATE EXTENSION IF NOT EXISTS pgcolumnar") + r = {} + d.run("CREATE TABLE rg_t (a int, b int, pad text) USING pgcolumnar") + d.run(f"SELECT pgcolumnar.set_options('rg_t', chunk_group_row_limit => {CG})") + d.run("INSERT INTO rg_t SELECT (g*2654435761)::bigint % 1000, g, md5(g::text) FROM generate_series(1,20000) g") + r["first"] = d.recluster("a") + r["key"] = d.q("SELECT sort_key::text FROM pgcolumnar.sort_status('rg_t')") + r["appended"] = d.q("SELECT appended_groups FROM pgcolumnar.sort_status('rg_t')") + before = d.q(LAYOUT) + r["second"] = d.recluster("a") + after = d.q(LAYOUT) + ok = all(v and not v.startswith(QUERY_ERROR) for v in (before, after)) + r["layout"] = ("same" if before == after else "rewritten") if ok \ + else f"no reading: [{before}] -> [{after}]" + r["third"] = d.recluster("a") + d.run("INSERT INTO rg_t SELECT (g*2654435761)::bigint % 1000, g, md5(g::text) FROM generate_series(1,5000) g") + r["tail"] = d.q("SELECT appended_groups > 0 FROM pgcolumnar.sort_status('rg_t')") + r["after append"] = d.recluster("a") + r["folded"] = d.q("SELECT appended_groups FROM pgcolumnar.sort_status('rg_t')") + r["new key"] = d.recluster("b") + r["key b"] = d.q("SELECT sort_key::text FROM pgcolumnar.sort_status('rg_t')") + r["repeat b"] = d.recluster("b") + # the mirror comes from the generator: g = 1..20000, then 1..5000 again + d.run("CREATE TABLE rg_h AS " + "SELECT ((g*2654435761)::bigint % 1000)::int AS a, g AS b, md5(g::text) AS pad " + "FROM generate_series(1,20000) g UNION ALL " + "SELECT ((g*2654435761)::bigint % 1000)::int, g, md5(g::text) FROM generate_series(1,5000) g") + r["rowset"], r["mirror"] = d.q(ROWSET.format(t="rg_t")), d.q(ROWSET.format(t="rg_h")) + yield r + finally: + with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a: + a.execute(f"DROP DATABASE IF EXISTS {DB} WITH (FORCE)") + + +# Not premises (#1339): the next three read recluster's own work, its recorded key (#415) +# and its recorded extent (#311), and a one-line change to recluster reddens each. +def test_the_first_gate_recluster_does_real_work(rc, expect): + expect.text(_work(rc["first"]), "yes", "the first recluster does real work (>0 groups)") + + +def test_sort_status_reports_the_clustering_key(rc, expect): + expect.text(rc["key"], "{a}", "sort_status now reports the clustering key (was NULL before #415)") + + +def test_the_recluster_run_covers_the_whole_relation(rc, expect): + expect.text(rc["appended"], "0", "the run covers the whole relation (0 appended groups)") + + +def test_a_repeat_on_an_unchanged_table_reclusters_nothing(rc, expect): + expect.num(rc["second"], 0, "recluster by the same key on an unchanged table reclusters 0 groups") + + +def test_the_layout_is_byte_identical(rc, expect): + expect.text(rc["layout"], "same", "and it leaves the physical layout byte-identical (group offsets unmoved)") + + +def test_a_third_identical_recluster_is_a_no_op(rc, expect): + expect.num(rc["third"], 0, "a third identical recluster is also a no-op") + + +def test_the_insert_appended_unsorted_groups(rc, expect): + expect.text(rc["tail"], "t", "premise: the insert appended unsorted groups") + + +def test_a_recluster_after_appends_does_work(rc, expect): + expect.text(_work(rc["after append"]), "yes", "recluster after appends does real work again (>0)") + + +def test_recluster_folds_the_tail_back_into_the_run(rc, expect): + expect.text(rc["folded"], "0", "and it folds the new data back into the run (0 appended after)") + + +def test_a_recluster_by_a_new_key_does_work(rc, expect): + expect.text(_work(rc["new key"]), "yes", "recluster by a NEW key is NOT a no-op -- it re-clusters (>0)") + + +def test_sort_status_reports_the_new_key(rc, expect): + expect.text(rc["key b"], "{b}", "and sort_status now reports the new key") + + +def test_a_repeat_by_the_new_key_is_a_no_op(rc, expect): + expect.num(rc["repeat b"], 0, "a repeat recluster by the new key is a no-op") + + +def test_the_no_op_reclusters_preserved_the_row_set(rc, expect): + expect.text(rc["rowset"], rc["mirror"], "no-op recluster preserved the exact row set (columnar == heap mirror)") diff --git a/test/pytest/unported_suites.txt b/test/pytest/unported_suites.txt index 4f2a0c826..53dfc66d7 100644 --- a/test/pytest/unported_suites.txt +++ b/test/pytest/unported_suites.txt @@ -103,6 +103,5 @@ qual_order_selectivity read_stream reader_buffer_reuse recluster_extent -recluster_gate row_triggers scan_direction diff --git a/test/recluster_gate.sh b/test/recluster_gate.sh index 89555ed8e..379014924 100755 --- a/test/recluster_gate.sh +++ b/test/recluster_gate.sh @@ -34,11 +34,13 @@ physlayout() { q "SELECT md5(string_agg(stripeid::text||':'||fileoffset::text||' # ---- fixture: cluster once by (a) -------------------------------------------- mk rg_t FIRST="$(q "SELECT pgcolumnar.recluster('rg_t','a');")" -check "premise: the first recluster does real work (>0 groups)" \ +# Not premises (#1339): these three read recluster's own work, its recorded key (#415) +# and its recorded extent (#311), and a one-line change to recluster reddens each. +check "the first recluster does real work (>0 groups)" \ "$([ "${FIRST:-0}" -gt 0 ] && echo yes || echo no)" "yes" -check "premise: sort_status now reports the clustering key (was NULL before #415)" \ +check "sort_status now reports the clustering key (was NULL before #415)" \ "$(q "SELECT sort_key::text FROM pgcolumnar.sort_status('rg_t');")" "{a}" -check "premise: the run covers the whole relation (0 appended groups)" \ +check "the run covers the whole relation (0 appended groups)" \ "$(q "SELECT appended_groups FROM pgcolumnar.sort_status('rg_t');")" "0" # ---- the gate: a second recluster by the same key is a no-op ----------------- @@ -46,8 +48,10 @@ PHYS_BEFORE="$(physlayout rg_t)" SECOND="$(q "SELECT pgcolumnar.recluster('rg_t','a');")" PHYS_AFTER="$(physlayout rg_t)" check_num "recluster by the same key on an unchanged table reclusters 0 groups" "$SECOND" "0" +# Two failed layout reads agreed, so a missing reading is "no reading", not "same". check "and it leaves the physical layout byte-identical (group offsets unmoved)" \ - "$([ "$PHYS_BEFORE" = "$PHYS_AFTER" ] && echo same || echo rewritten)" "same" + "$(if [ -z "$PHYS_BEFORE" ] || [ -z "$PHYS_AFTER" ]; then echo "no reading: [$PHYS_BEFORE] -> [$PHYS_AFTER]" + elif [ "$PHYS_BEFORE" = "$PHYS_AFTER" ]; then echo same; else echo rewritten; fi)" "same" THIRD="$(q "SELECT pgcolumnar.recluster('rg_t','a');")" check_num "a third identical recluster is also a no-op" "$THIRD" "0" @@ -72,9 +76,14 @@ check_num "a repeat recluster by the new key is a no-op" \ "$(q "SELECT pgcolumnar.recluster('rg_t','b');")" "0" # ---- correctness: the data is unchanged by the no-op (oracle vs heap) -------- -psql_run "CREATE TABLE rg_h (a int, b int, pad text);" -psql_run "INSERT INTO rg_h SELECT a,b,pad FROM rg_t;" -check "no-op recluster preserved the exact row set (columnar == heap mirror)" \ +# The mirror is built from the generator, NOT copied from rg_t: a copy of rg_t agrees +# with rg_t whatever the reclusters did to it (with a row corrupted before the copy, +# this passed). rg_t holds g = 1..20000, then g = 1..5000 again from the append. +psql_run "CREATE TABLE rg_h AS + SELECT ((g*2654435761)::bigint % 1000)::int AS a, g AS b, md5(g::text) AS pad FROM generate_series(1,20000) g + UNION ALL + SELECT ((g*2654435761)::bigint % 1000)::int, g, md5(g::text) FROM generate_series(1,5000) g;" +check_text "no-op recluster preserved the exact row set (columnar == heap mirror)" \ "$(q "SELECT count(*)||'/'||sum(a)||'/'||sum(b)||'/'||md5(string_agg(pad,',' ORDER BY b)) FROM rg_t;")" \ "$(q "SELECT count(*)||'/'||sum(a)||'/'||sum(b)||'/'||md5(string_agg(pad,',' ORDER BY b)) FROM rg_h;")"