From 41f8e8779a33dbe56c8250cf1c17f6784bc2e6ae Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 13 Oct 2026 15:07:10 +0000 Subject: [PATCH 1/3] test: a NULL key counts as a group in the bucket bound (#1330 review) @OffgridwithJD's shape: three minute buckets fill the bucket bound exactly, and NULL rows add a fourth group. Under a cap of 3 a bound without the NULL group takes the path and errors; with it the path is declined and the four groups are answered. In both twins. Co-Authored-By: Claude Opus 5.5 --- test/native_groupagg.sh | 12 ++++++++++++ test/pytest/test_native_groupagg.py | 14 ++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/test/native_groupagg.sh b/test/native_groupagg.sh index 1dce56796..6c0134c76 100755 --- a/test/native_groupagg.sh +++ b/test/native_groupagg.sh @@ -608,4 +608,16 @@ rc_out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postg check "a cached plan run under a lower cap stops at the runtime cap, with its hint" \ "$(grep -c 'Columnar Vectorized Group Keys' <<<"$rc_out")|$(grep -c 'exceeded pgcolumnar.groupagg_max_groups (1000)' <<<"$rc_out")|$(grep -c 'HINT: Raise pgcolumnar.groupagg_max_groups' <<<"$rc_out")" "1|1|1" +# A NULL key is one more group, and the bucket bound must count it. @OffgridwithJD's +# shape on #1330: buckets at 00:00:59, 00:01:30 and 00:02:00 fill the bound of 3 +# (floor(61 s / 60 s) + 2), so the NULL rows make a fourth group. Under a cap of 3 a +# bound without the NULL group takes the path and errors. +psql_run "CREATE TABLE gate_nb (ts timestamptz, v int) USING pgcolumnar; + INSERT INTO gate_nb SELECT '2026-01-01 00:00:59+00', g FROM generate_series(1, 50) g; + INSERT INTO gate_nb SELECT '2026-01-01 00:01:30+00', g FROM generate_series(1, 50) g; + INSERT INTO gate_nb SELECT '2026-01-01 00:02:00+00', g FROM generate_series(1, 50) g; + INSERT INTO gate_nb SELECT NULL, g FROM generate_series(1, 50) g;" +check "a NULL key counts as a group in the bucket bound: four groups under a cap of 3 are declined and all answered" \ + "$(groups_or_error 3 "SELECT date_trunc('minute', ts) b, count(*) FROM gate_nb GROUP BY b")" "4" + pgc_summary diff --git a/test/pytest/test_native_groupagg.py b/test/pytest/test_native_groupagg.py index 3f908bcf8..0136f57b0 100644 --- a/test/pytest/test_native_groupagg.py +++ b/test/pytest/test_native_groupagg.py @@ -818,3 +818,17 @@ def test_on_by_default_and_gated_on_an_upper_bound(pgc_conn, expect): _exec(conn, "DEALLOCATE gk") expect.text(f"{1 if planned else 0}|{got}", "1|1|1", "a cached plan run under a lower cap stops at the runtime cap, with its hint") + + +def test_a_null_key_counts_as_a_group_in_the_bucket_bound(pgc_conn, expect): + """@OffgridwithJD's shape on #1330: three minute buckets fill the bound exactly + (lo 00:00:59, hi 00:02:00: floor(61 s / 60 s) + 2 = 3), and NULL rows add a + fourth group. Under a cap of 3, a bound without the NULL group takes the path + and errors. This file's own rows: other second offsets, 40 rows a group. + """ + conn = pgc_conn + _exec(conn, "CREATE TABLE nul_b (ts timestamptz, v int) USING pgcolumnar") + for ts in ("'2026-03-01 10:00:59+00'", "'2026-03-01 10:01:05+00'", "'2026-03-01 10:02:00+00'", "NULL"): + _exec(conn, f"INSERT INTO nul_b SELECT {ts}, g FROM generate_series(1, 40) g") + expect.text(_groups_or_error(conn, 3, "SELECT date_trunc('minute', ts) b, count(*) FROM nul_b GROUP BY b"), "4", + "a NULL key counts as a group in the bucket bound: four groups under a cap of 3 are declined and all answered") From 461d4ca9d0bea3c3661b12b4a3a0688aba4b0e01 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 13 Oct 2026 15:18:36 +0000 Subject: [PATCH 2/3] test: TESTS.md and expected_tests for the NULL-group arm (#1330 review) Co-Authored-By: Claude Opus 5.5 --- test/pytest/TESTS.md | 1 + test/pytest/expected_tests.txt | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 07981df9d..96f5d4b3f 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -5662,6 +5662,7 @@ every major for this pair, which is the designed third state and not a failure. | `test_the_bound_reads_the_zone_maps_in_one_catalog_scan` | planning a bucketed GROUP BY over 50 row groups scans zone_map and its index once in all, not once per group (#1304) | | `test_a_mixed_timestamp_predicate_never_narrows_the_bound` | a cross-type predicate under a non-UTC TimeZone never narrows the estimate below the true group count (#1292); the matching-type one still gets a tight bound | | `test_on_by_default_and_gated_on_an_upper_bound` | the grouped path is on by default and taken only when an upper bound on the groups (the row count, or the time buckets) is within the cap: a bucket query over more rows than the cap is taken, a plain key over more rows is declined (answering as heap does) until the cap is raised, and a skewed key that fools ANALYZE is declined and answers every group; a cached plan run under a lower cap stops at the runtime cap with its hint (#1292, #1330 review) | +| `test_a_null_key_counts_as_a_group_in_the_bucket_bound` | three minute buckets fill the bucket bound and NULL rows add a fourth group; under a cap of 3 the path is declined and all four groups answer, where a bound without the NULL group errors (#1330 review) | ## 72. test_analyze_function.py: statistics collected by reading, not by sampling diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index ec041c0a2..f2df05fdc 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -606,4 +606,8 @@ guard_tests 419 # # cluster_tests 527 -> 528 for #1297's test_partition_drop_plan, by collection on # this branch rebased onto b55cb23 (#1330). -cluster_tests 528 +# +# cluster_tests 528 -> 529 for the #1330 review's NULL-group arm in +# test_native_groupagg.py, one test, by collection on this branch rebased onto +# 614eb02 (#1331). +cluster_tests 529 From e4ca93b638fa7a1d10528caaaf0d624c18a051a9 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 13 Oct 2026 15:34:40 +0000 Subject: [PATCH 3/3] test: the gate arms measure the gate, not the cost model (#1330 review) The bound-without-row-count mutation stayed green in the pytest twin: its skewed table, once the mutated gate let it through, lost to the ordinary plan on cost, so nothing errored. Both twins' gate helpers now turn hash aggregation and sorting off, so the grouped path wins whenever the gate offers it; a declined query still runs, on the disabled plan. Co-Authored-By: Claude Opus 5.5 --- test/native_groupagg.sh | 8 +++++++- test/pytest/test_native_groupagg.py | 22 ++++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/test/native_groupagg.sh b/test/native_groupagg.sh index 6c0134c76..105e5ca8f 100755 --- a/test/native_groupagg.sh +++ b/test/native_groupagg.sh @@ -563,14 +563,20 @@ HIGHK="SELECT k, count(*), sum(i) FROM %T GROUP BY k" # or its error. The query runs itself: wrapped in SELECT count(*) FROM (...), the # planner drops the unused aggregates and the node refuses that shape, so the # wrapper would measure a different plan. +# Both helpers turn hash aggregation and sorting off, so the grouped path wins +# whenever the gate offers it: these arms test the gate, not the cost model. A +# declined query still runs, on the disabled plan. Without this, a gate that let a +# skewed key through could still lose on cost, and the arm would stay green. groups_or_error() { local out out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At -q \ + -c "SET enable_hashagg = off" -c "SET enable_sort = off" \ -c "SET pgcolumnar.groupagg_max_groups = $1" -c "$2" 2>&1)" if grep -q '^ERROR' <<<"$out"; then grep -m1 '^ERROR' <<<"$out"; else grep -c '' <<<"$out"; fi } under_cap() { # under_cap CAP QUERY -> 1 when the grouped node is planned under that cap - env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At \ + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" -At -q \ + -c "SET enable_hashagg = off" -c "SET enable_sort = off" \ -c "SET pgcolumnar.groupagg_max_groups = $1" -c "EXPLAIN (COSTS OFF) $2" 2>&1 \ | grep -c 'Columnar Vectorized Group Keys' } diff --git a/test/pytest/test_native_groupagg.py b/test/pytest/test_native_groupagg.py index 0136f57b0..016774da4 100644 --- a/test/pytest/test_native_groupagg.py +++ b/test/pytest/test_native_groupagg.py @@ -730,25 +730,39 @@ def test_a_mixed_timestamp_predicate_never_narrows_the_bound(pgc_conn, expect): # ---- on by default, gated on an upper bound (#1292 phase 2) ----------------- -def _under_cap(conn, cap, sql): +def _gate_only(conn, cap): + """Hash aggregation and sorting off, so the grouped path wins whenever the gate + offers it: the arms below test the gate, not the cost model. A declined query + still runs, on the disabled plan.""" + _exec(conn, "SET enable_hashagg = off") + _exec(conn, "SET enable_sort = off") _exec(conn, f"SET pgcolumnar.groupagg_max_groups = {cap}") + + +def _gate_reset(conn): + for name in ("enable_hashagg", "enable_sort", "pgcolumnar.groupagg_max_groups"): + _exec(conn, f"RESET {name}") + + +def _under_cap(conn, cap, sql): + _gate_only(conn, cap) try: return _plan(conn, sql) finally: - _exec(conn, "RESET pgcolumnar.groupagg_max_groups") + _gate_reset(conn) def _groups_or_error(conn, cap, sql): """The number of groups the query returns under that cap, or its error. The query runs itself: wrapped in count(*), the planner drops the unused aggregates and the node refuses that shape, so a wrapper would measure a different plan.""" - _exec(conn, f"SET pgcolumnar.groupagg_max_groups = {cap}") + _gate_only(conn, cap) try: return str(len(_rows(conn, sql))) except psycopg.Error as exc: return f"ERROR: {exc.diag.message_primary}" finally: - _exec(conn, "RESET pgcolumnar.groupagg_max_groups") + _gate_reset(conn) def test_on_by_default_and_gated_on_an_upper_bound(pgc_conn, expect):