From 0c96c611f4fd7a5b5e8dd3e2446c54b6b6f01e64 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Sat, 17 Oct 2026 16:39:53 +0000 Subject: [PATCH] test: each covering-count mode names the path it took (#1339) test_differential.py:895 [inaccurate]: "count meta=on" and "count meta=off" compared the count with the heap. A fallback returns the same count, so neither arm could say which path answered. native_agg proves the metadata node only on a table with no deletes; this table has deletes and updates. Both twins now check the plan in each mode: "count meta=on takes the path it names" wants the metadata node ("Columnar Vectorized Aggregates"), and "count meta=off ..." wants Custom Scan (PgColumnarScan) without it. An empty plan, or one that lost the columnar scan, is named rather than read as the scan. One check per test (#1528 review): the twin's covering module fixture takes each mode's path and count pair as values, so an inverted enable_vectorization test reddens both path checks in the twin, as it does in the shell, instead of stopping at the first. Co-Authored-By: Claude Opus 5.5 --- test/check_ledger.tsv | 2 + test/check_ledger_budget.txt | 12 ++++ test/differential.sh | 17 ++++- test/pytest/TESTS.md | 13 +++- test/pytest/expected_tests.txt | 4 +- test/pytest/test_differential.py | 110 +++++++++++++++++++++++++------ 6 files changed, 131 insertions(+), 27 deletions(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index c9f66ac96..701a0f278 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -723,7 +723,9 @@ differential differential cg boundary N=99 scan 15;16;17;18;19 never - differential differential compound 15;16;17;18;19 never - differential differential control: the set oracle is order-blind by design 15;16;17;18;19 never - differential differential count meta=off 15;16;17;18;19 never - +differential differential count meta=off takes the path it names 15;16;17;18;19 2026-10-17 columnar_vector.c:1404 the hook ignores pgcolumnar.enable_vectorization, so count(*) takes the metadata node with it off differential differential count meta=on 15;16;17;18;19 never - +differential differential count meta=on takes the path it names 15;16;17;18;19 2026-10-17 columnar_vector.c:1404 the upper-path hook always returns, so count(*) never takes the metadata node differential differential dict text agg 15;16;17;18;19 never - differential differential dict text eq 15;16;17;18;19 never - differential differential dict text group 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 1107206cf..3d9216346 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -3690,4 +3690,16 @@ suites_not_covered 184 # never 3077 + 4 = 3081 # dated 1378 + 7 = 1385 # 3081 + 1385 == 4466 +# +# #1339, differential's covering count names the path it took (jdatcmd): +# 3081 -> 3081. "count meta=on" and "count meta=off" compared the count to the +# heap, and a fallback returns the same count, so neither said which path ran. +# Each mode now checks the plan, one check per test. Both checks arrive dated: +# the hook returning early reddens the "on" check alone, and the hook ignoring +# enable_vectorization reddens the "off" check alone, on each of 15-19. +# +# ledger rows 4466 + 2 = 4468 +# never 3081 + 0 = 3081 +# dated 1385 + 2 = 1387 +# 3081 + 1387 == 4468 checks_never_observed_red 3081 diff --git a/test/differential.sh b/test/differential.sh index c55e50828..a6cc6506e 100755 --- a/test/differential.sh +++ b/test/differential.sh @@ -387,12 +387,25 @@ psql_run "DELETE FROM t_col WHERE id % 13 = 0;" psql_run "UPDATE t_heap SET v = v + 1 WHERE id % 17 = 0;" psql_run "UPDATE t_col SET v = v + 1 WHERE id % 17 = 0;" +# A fallback returns the same count, so the oracle check alone cannot say which +# path answered it. native_agg proves the metadata node on a table with no +# deletes; here the table has deletes and updates, which is what makes the +# per-group arithmetic matter. So each mode first names the path it took: the +# metadata node reports "Columnar Vectorized Aggregates", and the scan shows +# "Custom Scan (PgColumnarScan)" without it. An empty plan is "no plan" and any +# other plan is "other plan", so neither an error nor a plan that lost the +# columnar scan can pass as the scan. for mode in on off; do q "ALTER DATABASE $PGC_DB SET pgcolumnar.enable_vectorization=$mode;" >/dev/null + plan="$(q "EXPLAIN (COSTS OFF) SELECT count(*) FROM t_col;")" + path="no plan" + [ -n "$plan" ] && path="other plan" + grep -qF 'Custom Scan (PgColumnarScan)' <<<"$plan" && path=scan + grep -q 'Columnar Vectorized Aggregates' <<<"$plan" && path=metadata + want=metadata; [ "$mode" = off ] && want=scan + check "count meta=$mode takes the path it names" "$path" "$want" diff_query "count meta=$mode" "SELECT count(*) FROM %T" done q "ALTER DATABASE $PGC_DB RESET pgcolumnar.enable_vectorization;" >/dev/null -# The metadata count fast path is proven by the native_agg suite; the oracle -# checks above prove the count is correct with the path on and off. pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 3f8a6f7ed..c214abd0f 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -3704,13 +3704,20 @@ Four selectivities from one row to most. `wide nomatch` is the second arm in the cannot fail alone, and gets the same treatment as `empty scan`: a stated reason and a positive control at a value that is present. -### `test_a_covering_count_agrees_with_the_path_on_and_off` +### `test_the_delete_and_update_leave_a_count_neither_gives_alone`, `test_each_count_mode_takes_the_path_it_names`, `test_the_covering_count_agrees_with_the_heap` An UPDATE appends the new row and masks the old, so the stored per-group count and the visible count diverge -- which is the arithmetic the metadata path has to get right and a plain scan gets right for free. Run both ways through `enable_vectorization`, which is what distinguishes -"the fast path is correct" from "the fast path was not taken". `SET` on the connection rather -than the bash suite's `ALTER DATABASE`, which exists because each psql there is a new session. +"the fast path is correct" from "the fast path was not taken". A fallback returns the same +count, so each mode also checks the plan: `on` must show the metadata node ("Columnar +Vectorized Aggregates"), and `off` must show `Custom Scan (PgColumnarScan)` without it. + +One check per test: the `covering` module fixture takes each mode's path and count pair as +values, so an inverted switch reddens both path checks, as it does in the shell, rather than +stopping at the first. `SET` on the connection rather than the bash suite's `ALTER DATABASE`, +which exists because each psql there is a new session. + ## 32. test_stats_privilege.py: stats is readable only by a caller who may read the table Port of `test/stats_privilege.sh` (#560, ported for #432). diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 0a77aff30..12d6f7105 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -844,4 +844,6 @@ guard_tests 442 # test (2 -> 34 tests), by collection over the cluster files. # cluster_tests 932 -> 943 for #1341's test_native_varlena_bound.py, the twin of native_varlena_bound.sh: # eleven tests, one per check, by collection over the cluster files on this tree. -cluster_tests 943 +# cluster_tests 943 -> 947 for #1339, one check per test in test_differential.py's covering +# count (1 -> 5 tests), by collection over the cluster files. +cluster_tests 947 diff --git a/test/pytest/test_differential.py b/test/pytest/test_differential.py index e56088e64..3836029e7 100644 --- a/test/pytest/test_differential.py +++ b/test/pytest/test_differential.py @@ -902,31 +902,99 @@ def test_a_selective_filter_with_a_wide_projection_agrees(pgc_conn, expect): # --------------------------------------------------------------------------- -def test_a_covering_count_agrees_with_the_path_on_and_off(pgc_conn, expect): - """The same count, both ways through `enable_vectorization`, after a delete and an - update. +COUNT_MODES = ["on", "off"] - An UPDATE on a columnar table appends the new row and masks the old, so the stored - per-group count and the visible count diverge -- which is exactly the arithmetic the - metadata path has to get right, and exactly what a plain scan gets right for free. - Running it both ways is what distinguishes "the fast path is correct" from "the fast - path was not taken". + +def _count_path(plan): + """The path a covering count took: "metadata" when the plan shows the metadata + node, "scan" when it shows the columnar scan without it. Anything else is named, + so a plan that lost the columnar scan, or a failed EXPLAIN, cannot pass as either.""" + if not plan: + return "no plan" + if "Columnar Vectorized Aggregates" in plan: + return "metadata" + if "Custom Scan (PgColumnarScan)" in plan: + return "scan" + return "other plan: " + plan.replace("\n", " | ") + + +@pytest.fixture(scope="module") +def covering(pgc_cluster): + """The covering-count pair after a delete and an update, and each mode's readings. + + ONE CHECK PER TEST (#1339). Each mode's path and count pair are taken here as + values, so a red in one cannot hide another: the #1528 review measured an + inverted switch reddening both path checks in the shell and only the first in a + loop that asserted as it went. A failed query is a query_error(), which every + comparison refuses. `SET` on this connection rather than the bash suite's `ALTER DATABASE`: that form exists because each psql invocation there is a new session, and this one is not. """ - p = _pair(pgc_conn, "id int, v int", - load="SELECT g, g%10 FROM generate_series(1,20000) g", - options="chunk_group_row_limit => 1000, stripe_row_limit => 3000") - pgc_conn.execute("DELETE FROM t_heap WHERE id % 13 = 0") - pgc_conn.execute("DELETE FROM t_col WHERE id % 13 = 0") - pgc_conn.execute("UPDATE t_heap SET v = v + 1 WHERE id % 17 = 0") - pgc_conn.execute("UPDATE t_col SET v = v + 1 WHERE id % 17 = 0") - expect.num(p.one("SELECT count(*) FROM t_heap"), 18462, + import psycopg + from pgc_vacuity import query_error + + conn = psycopg.connect(pgc_cluster.dsn(), autocommit=True) + schema = "pgc_differential_count" + + def rows(sql): + try: + return [tuple(r) for r in conn.execute(sql).fetchall()] + except psycopg.Error as exc: + return query_error(exc.diag.message_primary) + + got = {} + try: + conn.execute(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE') + conn.execute(f'CREATE SCHEMA "{schema}"') + conn.execute(f'SET search_path TO "{schema}", public') + try: + _pair(conn, "id int, v int", + load="SELECT g, g%10 FROM generate_series(1,20000) g", + options="chunk_group_row_limit => 1000, stripe_row_limit => 3000") + conn.execute("DELETE FROM t_heap WHERE id % 13 = 0") + conn.execute("DELETE FROM t_col WHERE id % 13 = 0") + conn.execute("UPDATE t_heap SET v = v + 1 WHERE id % 17 = 0") + conn.execute("UPDATE t_col SET v = v + 1 WHERE id % 17 = 0") + except psycopg.Error as exc: + print(f"-- building the covering-count pair failed: {exc.diag.message_primary}") + heap = rows("SELECT count(*) FROM t_heap") + got["count"] = heap[0][0] if isinstance(heap, list) and heap else heap + for mode in COUNT_MODES: + conn.execute(f"SET pgcolumnar.enable_vectorization = {mode}") + plan = rows("EXPLAIN (COSTS OFF) SELECT count(*) FROM t_col") + got[mode] = { + "path": _count_path("\n".join(r[0] for r in plan)) + if isinstance(plan, list) else f"no plan: {plan}", + "col": rows("SELECT count(*) FROM t_col"), + "heap": rows("SELECT count(*) FROM t_heap"), + } + conn.execute("RESET pgcolumnar.enable_vectorization") + yield got + finally: + try: + conn.execute(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE') + finally: + conn.close() + + +def test_the_delete_and_update_leave_a_count_neither_gives_alone(covering, expect): + """An UPDATE on a columnar table appends the new row and masks the old, so the + stored per-group count and the visible count diverge: the arithmetic the + metadata path has to get right, and a plain scan gets right for free.""" + expect.num(covering["count"], 18462, "premise: the delete and update leave a count neither one would give alone") - for mode in ("on", "off"): - pgc_conn.execute(f"SET pgcolumnar.enable_vectorization = {mode}") - c, h = p.both("SELECT count(*) FROM %T") - expect.row_set(c, h, f"count meta={mode}") - pgc_conn.execute("RESET pgcolumnar.enable_vectorization") + +@pytest.mark.parametrize("mode", COUNT_MODES) +def test_each_count_mode_takes_the_path_it_names(covering, expect, mode): + """A fallback returns the same count, so the count arm alone cannot say which path + answered it. native_agg proves the metadata node on a table with no deletes, not + on this one.""" + expect.text(covering[mode]["path"], "metadata" if mode == "on" else "scan", + f"count meta={mode} takes the path it names") + + +@pytest.mark.parametrize("mode", COUNT_MODES) +def test_the_covering_count_agrees_with_the_heap(covering, expect, mode): + expect.row_set(covering[mode]["col"], covering[mode]["heap"], f"count meta={mode}")