From 493a8deedc3e3e9e5b47809cfdbcb7c2dc6ba5f1 Mon Sep 17 00:00:00 2001 From: "Joshua (D) Drake" <136637981+ChronicallyJD@users.noreply.github.com> Date: Wed, 5 Aug 2026 18:54:14 -0600 Subject: [PATCH 1/6] test: assert the planner's chosen plan is not far worse than one it declined (#434) Every suite we have asks whether a plan is CORRECT. None asks whether it is the one a reasonable cost model would pick. That gap is how #434 survived: the planner prices a columnar index scan below the custom scan and then runs an order of magnitude slower, and both plans return the right answer, so nothing failed. For each query this runs the planner's own choice, then forces each alternative, and fails when the choice is more than 3x slower than the best one declined. It is a ratio between two plans in the same run on the same box, so it does not depend on the machine. Against main: range scan with aggregate chose Index Scan: TIMEOUT at 120s alternative Custom Scan: 23.7 ms wide range, narrow proj. chose Index Only Scan: 354.8 ms alternative Custom Scan: 15.3 ms 23.23x 3x on purpose. The point is not to police the cost model, which is allowed to be wrong. It is to catch wrong by orders of magnitude. A tight bound would flake on a shared runner and teach people to ignore it. Four premises, because each is a way this could pass while measuring nothing: the payload must not have compressed, the index key must be correlated, forcing must produce a different plan, and both the chosen plan and at least one alternative must have been identified and timed. The last of those came from a real failure while writing this. "Index Only Scan" does not match a grep for "Index Scan", so the chosen plan read as "unknown" and was compared against another unknown. Before that, node and time were passed as a space-separated pair and read back with a plain read, which splits "Custom Scan (PgColumnarScan) 23.380" into a node of "Custom". Both produced output that looked like a result. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UqprqkCXuH8SegiZejE1Tw --- test/planner_choice_quality.sh | 134 +++++++++++++++++++++++++++++++++ test/run_all_versions.sh | 2 +- 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100755 test/planner_choice_quality.sh diff --git a/test/planner_choice_quality.sh b/test/planner_choice_quality.sh new file mode 100755 index 00000000..5e68c0f7 --- /dev/null +++ b/test/planner_choice_quality.sh @@ -0,0 +1,134 @@ +#!/usr/bin/env bash +# +# The planner's chosen plan must not be catastrophically worse than one it +# declined (issues #433, #434). +# +# Every other suite asks whether a plan is CORRECT. None asks whether it is the +# one a reasonable cost model would pick. That gap is how #434 survived: the +# planner prices a columnar index scan at 31,502 and the custom scan at 69,204, +# then chooses the index scan, which is 12.4x slower. Both plans return the right +# answer, so every existing check passes. +# +# WHAT THIS ASSERTS +# +# For each query: run the planner's own choice, then force each alternative, and +# fail when the choice is more than PLAN_BOUND times slower than the best +# alternative. It is a ratio between two plans in the same run on the same box, +# so it does not depend on how fast the machine is. +# +# WHY THE BOUND IS LOOSE +# +# 3x. The point is not to police the cost model, which is allowed to be wrong. +# It is to catch the case where it is wrong by orders of magnitude, which is what +# #434 is. A tight bound here would flake on a shared runner and teach people to +# ignore it. +# +# WHY TIMING RATHER THAN BUFFERS +# +# Buffers would be exact, and they are the right tool when the question is "did +# this read less". Here the question is "is the chosen plan much slower", and +# slower is what the user experiences. The bound is loose enough to survive +# timing noise, and PGC_SKIP_TIMING drops the ratio checks while leaving the +# premises, which is what check_timing is for. +# +# Usage: test/planner_choice_quality.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}" + +PLAN_BOUND=${PGC_PLAN_BOUND:-3} +ROWS=${PGC_PLANQ_ROWS:-200000} + +# The shape #433 and #434 are about: wide incompressible rows, an index on a +# correlated key. The payload must not compress, or the row group stays under the +# fetch cache cap and the effect disappears. Correlating on both g and the inner +# series is what makes every row and every block differ. +psql_run "CREATE TABLE pq (k bigint, tag text, payload bytea) USING pgcolumnar; + INSERT INTO pq + SELECT g, 'tag' || (g % 5), + decode((SELECT string_agg(md5(g::text || s::text), '') + FROM generate_series(1,64) s), 'hex') + FROM generate_series(1,$ROWS) g; + CREATE INDEX pq_k ON pq (k); + ANALYZE pq;" >/dev/null + +check "fixture rows" "$(q 'SELECT count(*) FROM pq')" "$ROWS" +SZ=$(q "SELECT pg_total_relation_size('pq')") +check "premise: the payload did not compress, so the fetch path is exercised" \ + "$([ "$SZ" -gt $(( ROWS * 700 )) ] && echo yes || echo "no ($(( SZ / ROWS )) bytes per row)")" "yes" +check "premise: the index key is correlated, which is the case that misprices" \ + "$(q "SELECT CASE WHEN correlation > 0.9 THEN 'yes' ELSE 'no (' || correlation || ')' END + FROM pg_stats WHERE tablename='pq' AND attname='k'")" "yes" + +# Time a query under a given setting, and report which scan node ran. +run_plan() { # run_plan -> " " + local node ms out + node=$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -Atq -c "$1" -c "EXPLAIN (COSTS OFF) $2" 2>&1 | + grep -oE 'Index Only Scan|Index Scan|Bitmap Heap Scan|Custom Scan \([A-Za-z]+\)|Seq Scan' | head -1) + out=$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -Atq -c "SET statement_timeout='120s';" -c "$1" -c '\timing on' -c "$2" 2>&1) + if grep -qiE 'timeout|canceling' <<<"$out"; then + printf '%s\tTIMEOUT\n' "${node:-unknown}"; return + fi + ms=$(grep -oE 'Time: [0-9.]+ ms' <<<"$out" | tail -1 | grep -oE '[0-9.]+') + # Tab-delimited, because a node name contains spaces: "Custom Scan + # (PgColumnarScan)" read back through a space-split gives node="Custom" and + # ms="Scan (PgColumnarScan) 23.380". The first version of this file did that + # and compared two garbage strings while reporting a pass. + printf '%s\t%s\n' "${node:-unknown}" "${ms:-}" +} + +# The planner's choice against the best alternative it declined. +choice_vs_best() { # choice_vs_best