From 481cd24a07ca28a364d9ae5b1501bf0393953196 Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Thu, 6 Aug 2026 22:38:54 +0000 Subject: [PATCH 1/4] wip(#414): pgcolumnar.analyze() slice 1, exact null_frac from zone maps Core ANALYZE decodes essentially the whole table: a fixed 30,000-row sample falls in every row group, so every group is decoded for every column. Measured on 3M rows x 20 columns, 1237 MB, serial: ANALYZE 6,302 ms against 268 ms to decode one column, and 7,680 ms to decode all nineteen text columns outright. Not fixable in the AM callbacks: acquire_sample_rows copies whole tuples, so asking for one column saves 6% (6,073 vs 6,302 ms), and shrinking chunk_group_row_limit tenfold changes nothing (6,466 vs 6,771 ms) because a fixed-size sample touches proportionally more groups. This slice collects null_frac only, exactly, from the zone maps. Work in progress: slices 2-5 (single-column n_distinct, histogram with exact endpoints, and the differential-against-core verification) are not here yet. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UqprqkCXuH8SegiZejE1Tw --- pgcolumnar--1.0-alpha.sql | 145 ++++++++++++++++++++++++++++++++++++++ test/analyze_function.sh | 138 ++++++++++++++++++++++++++++++++++++ 2 files changed, 283 insertions(+) create mode 100755 test/analyze_function.sh diff --git a/pgcolumnar--1.0-alpha.sql b/pgcolumnar--1.0-alpha.sql index decad2a..7244d50 100644 --- a/pgcolumnar--1.0-alpha.sql +++ b/pgcolumnar--1.0-alpha.sql @@ -960,3 +960,148 @@ CREATE FUNCTION pgcolumnar.parallel_copy(target regclass, filename text, COMMENT ON FUNCTION pgcolumnar.parallel_copy(regclass, text, int) IS 'atomic parallel bulk load of a COPY text file into a columnar table using background workers: a single columnar table (any row order), or a RANGE-partitioned columnar table sorted by the partition key with one distinct partition set per worker (#300)'; + +/* + * Per-column statistics without reading the whole table (#414). + * + * Core ANALYZE decodes essentially the entire table. It samples a fixed 30,000 + * rows, and on a table of any size those rows fall in every row group, so every + * group is decoded for every column. Measured on 3M rows x 20 columns, 1237 MB, + * serial: ANALYZE costs 6,302 ms against 7,680 ms to decode all nineteen text + * columns outright, while decoding just one column costs 268 ms. + * + * That cannot be recovered inside the table-AM callbacks, which is why this is a + * function. acquire_sample_rows copies whole tuples (ExecCopySlotHeapTuple), so + * the AM cannot decline to produce columns core is about to copy: ANALYZE of one + * named column costs 6,073 ms against 6,302 ms for all twenty, a 6% saving. Nor + * is there slack in which groups the sample touches -- at a tenth the + * chunk_group_row_limit the cost was unchanged, because a fixed-size sample + * touches proportionally more groups when they are smaller. + * + * Core ANALYZE remains the correctness path and is what autovacuum runs. This is + * an opt-in accelerator for wide tables and, like pgcolumnar.vacuum(), nothing + * schedules it: see #415. + * + * This first form collects null_frac only, taken exactly from the zone maps + * rather than sampled. n_distinct, MCVs and histogram bounds still need a sample + * and are not collected here yet. + */ +CREATE FUNCTION pgcolumnar.analyze(rel regclass, columns text[] DEFAULT NULL) + RETURNS void + LANGUAGE plpgsql + AS $$ +DECLARE + sid bigint; + att record; + nullfrac double precision; + seen integer := 0; + unknown text; + schname text; + relnm text; +BEGIN + /* + * Writing statistics uses pg_restore_attribute_stats, which core added in + * 18. On 15 to 17 this would mean writing pg_statistic directly, and the + * risk there is in the values rather than the insert: stavalues is anyarray + * and must carry the column's element type, typmod and collation; staop must + * be the right operator for the stakind; stadistinct has a sign convention + * that is easy to invert. Each of those produces plausible wrong estimates + * rather than an error. Refuse clearly instead of failing obscurely inside + * the call below. + */ + IF current_setting('server_version_num')::int < 180000 THEN + RAISE EXCEPTION 'pgcolumnar.analyze() requires PostgreSQL 18 or later' + USING DETAIL = 'it writes statistics through pg_restore_attribute_stats, which older majors do not have', + HINT = 'use ANALYZE on this server'; + END IF; + + /* + * pg_restore_attribute_stats identifies the column by schema and relation + * NAME, not by regclass, and rejects a null schemaname. Resolve both from the + * oid once rather than per column. + */ + SELECT n.nspname, c.relname INTO schname, relnm + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.oid = rel; + + SELECT s.storage_id INTO sid + FROM pgcolumnar.storage s + WHERE s.relation_oid = rel; + + IF sid IS NULL THEN + RAISE EXCEPTION 'pgcolumnar.analyze(): % has no columnar storage', rel::text + USING HINT = 'this function only applies to pgcolumnar tables that have been written to'; + END IF; + + /* + * A named column that does not exist is a caller error, not a no-op. Silently + * collecting nothing is the failure mode that looks exactly like success. + */ + IF columns IS NOT NULL THEN + SELECT c INTO unknown + FROM unnest(columns) AS c + WHERE NOT EXISTS ( + SELECT 1 FROM pg_attribute a + WHERE a.attrelid = rel AND a.attname = c + AND a.attnum > 0 AND NOT a.attisdropped) + LIMIT 1; + IF unknown IS NOT NULL THEN + RAISE EXCEPTION 'pgcolumnar.analyze(): column "%" does not exist in %', + unknown, rel::text; + END IF; + END IF; + + FOR att IN + SELECT a.attname, a.attnum + FROM pg_attribute a + WHERE a.attrelid = rel AND a.attnum > 0 AND NOT a.attisdropped + AND (columns IS NULL OR a.attname = ANY (columns)) + ORDER BY a.attnum + LOOP + /* + * null_frac, exactly. value_count counts the values present and + * null_count those absent, so the denominator is their sum rather than + * value_count alone. vector_index = -1 is the whole-chunk aggregate; + * summing the per-vector rows as well would double count. + * + * column_index is the 0-based attribute position. attnum is stable + * across a dropped column, so attnum - 1 keeps pointing at the same + * column after a DROP COLUMN. + */ + SELECT sum(z.null_count)::double precision + / nullif(sum(z.value_count + z.null_count), 0) + INTO nullfrac + FROM pgcolumnar.zone_map z + WHERE z.storage_id = sid + AND z.column_index = att.attnum - 1 + AND z.vector_index = -1; + + CONTINUE WHEN nullfrac IS NULL; /* no zone map rows: nothing exact to say */ + + /* + * The casts are load-bearing. pg_restore_attribute_stats takes VARIADIC + * "any", so a mistyped argument is a WARNING and the value is dropped, + * not an error: attname must be text (attname is `name`) and null_frac + * must be real (the division yields double precision). Without these the + * call "succeeds" having stored nothing. + */ + PERFORM pg_catalog.pg_restore_attribute_stats( + 'schemaname', schname, + 'relname', relnm, + 'attname', att.attname::text, + 'inherited', false, + 'null_frac', nullfrac::real); + + seen := seen + 1; + END LOOP; + + IF seen = 0 THEN + RAISE EXCEPTION 'pgcolumnar.analyze(): collected statistics for no columns of %', rel::text + USING HINT = 'the table may have no written row groups yet'; + END IF; +END; +$$; + +COMMENT ON FUNCTION pgcolumnar.analyze(regclass, text[]) + IS 'collect per-column statistics by reading one column at a time, taking null_frac exactly from the zone maps rather than sampling (#414); core ANALYZE remains the correctness path and nothing schedules this, see #415'; diff --git a/test/analyze_function.sh b/test/analyze_function.sh new file mode 100755 index 0000000..10e1869 --- /dev/null +++ b/test/analyze_function.sh @@ -0,0 +1,138 @@ +#!/usr/bin/env bash +# +# pgColumnar pgcolumnar.analyze(): per-column statistics without reading the +# whole table (issue #414). +# +# Core ANALYZE decodes essentially the entire table. It samples 30,000 rows, and +# on a table of any size those rows are spread across every row group, so every +# group is decoded for every column. Measured on 3M rows x 20 columns (1237 MB, +# incompressible fixture, serial): +# +# decode all 19 text columns (a full-table read) 7,680 ms +# ANALYZE w (20 columns) 6,302 ms +# decode only k out of the 20-column table 268 ms +# heap ANALYZE wh (k) 186 ms +# +# So ANALYZE costs about what reading the whole table costs, and reading one +# column costs 23.5x less. That gap is what this function exists to collect. +# +# It cannot be fixed in the table-AM callbacks, and that is settled rather than +# assumed. acquire_sample_rows copies whole tuples (ExecCopySlotHeapTuple), so +# the AM cannot decline to produce columns core is about to copy -- ANALYZE w +# costs 6,302 ms against ANALYZE w (k) at 6,073 ms, a 6% saving for asking for +# one column out of twenty. Nor is there slack in which row groups the sample +# touches: rebuilding at a tenth the chunk_group_row_limit left the cost +# unchanged (6,466 ms against 6,771 ms), because a fixed-size sample simply +# touches proportionally more groups when they are smaller. +# +# This suite is about the function, not the AM sampler. test/analyze_stats.sh +# covers the sampler (#154) and stays the correctness path for plain ANALYZE. +# +# Slice 1 asserts one thing: null_frac comes from the zone maps and is EXACT, +# where core's is sampled and is not. +# +# Usage: test/analyze_function.sh [PG_CONFIG] +# Written fresh for pgColumnar. + +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +pgc_setup "${1:-/usr/local/pg18/bin/pg_config}" + +ROWS=${PGC_ANALYZE_ROWS:-500000} + +# Writing statistics uses pg_restore_attribute_stats, which core added in 18. +# On 15 to 17 this would mean writing pg_statistic directly, which is a real +# version-support decision and not a detail (stavalues anyarray typing, staop, +# stacoll, stadistinct's sign convention). Refuse rather than silently narrow: +# pgc_skip fails by default and must be waived deliberately. +if [ "$PGC_MAJOR" -lt 18 ]; then + pgc_skip PG18_STATS_API \ + "pgcolumnar.analyze() needs pg_restore_attribute_stats (PG18+); this server is $PGC_MAJOR" +fi + +# --- fixture ------------------------------------------------------------------ +# +# k is NULL for exactly one row in ten. 500,000 rows against core's 30,000-row +# sample is what makes the sampled estimate inexact, which slice 1 depends on. + +psql_run "DROP TABLE IF EXISTS af_c; + CREATE TABLE af_c (k int, pad1 text, pad2 text, pad3 text) USING pgcolumnar; + INSERT INTO af_c SELECT + CASE WHEN g % 10 = 0 THEN NULL ELSE g % 45001 END, + md5(g::text), md5((g * 7)::text), md5((g * 13)::text) + FROM generate_series(1, $ROWS) g;" >/dev/null + +# --- premise 1: the fixture really is one-in-ten NULL -------------------------- +# +# Everything below compares against 0.1. If the fixture is not 10% NULL then a +# "PASS" means the function agreed with a number that was never true. + +true_nullfrac="$(q "SELECT round(count(*) FILTER (WHERE k IS NULL)::numeric / count(*), 6) + FROM af_c")" +check_num "premise: the fixture is exactly one-in-ten NULL" "$true_nullfrac" "0.100000" + +# --- premise 2: core's sampled null_frac is NOT exact -------------------------- +# +# This is the premise that makes the slice-1 check mean something. If core's +# sample happened to land on the truth, then "exact" and "sampled" are the same +# number here, and an implementation that merely called core ANALYZE would pass. +# The test would be vacuous in precisely the way a green suite hides. +# +# So assert the two differ BEFORE asserting ours is the exact one. If this fails, +# the fixture is not discriminating and the suite is reporting nothing -- rerun +# or raise PGC_ANALYZE_ROWS rather than trusting a pass below it. + +psql_run "ANALYZE af_c;" >/dev/null +core_nullfrac="$(q "SELECT null_frac FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" + +if ! pgc_is_number "$core_nullfrac"; then + check_num "premise: core ANALYZE produced a null_frac to compare against" \ + "$core_nullfrac" "a number" +else + check "premise: core's sampled null_frac differs from the truth, so this suite can discriminate" \ + "$(awk -v c="$core_nullfrac" -v t="$true_nullfrac" \ + 'BEGIN { print (c == t) ? "no (sample landed exactly on truth; suite is vacuous)" : "yes" }')" \ + "yes" + echo "-- core sampled null_frac = $core_nullfrac, truth = $true_nullfrac" +fi + +# --- check 1: pgcolumnar.analyze() gives the EXACT null_frac ------------------- +# +# The zone maps already hold null_count and value_count per chunk, so this is a +# metadata read: 11 ms on the 1237 MB fixture, against 6,302 ms for core ANALYZE. +# Exactness is a by-product of that, not the reason for it -- the reason is the +# 23.5x. But it is the cheapest thing to assert that a sampled implementation +# cannot fake. + +psql_run "SELECT pgcolumnar.analyze('af_c'::regclass, ARRAY['k']);" >/dev/null +ours_nullfrac="$(q "SELECT null_frac FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" + +check_num "pgcolumnar.analyze() reports null_frac exactly, from the zone maps" \ + "$ours_nullfrac" "0.1" + +# --- check 2: it must not destroy the statistics it does not compute ----------- +# +# pg_restore_attribute_stats is a RESTORE api: it is designed to reinstate a whole +# attribute's statistics from a dump, so kinds not named in the call may be +# cleared rather than left alone. That matters here, because this slice collects +# null_frac and nothing else. An accelerator that yields an exact null_frac while +# discarding n_distinct and the MCV list makes plans worse, not better -- and it +# would do so silently, since nothing errors. +# +# So: n_distinct must survive the call. If this fails, the minimal implementation +# is a regression and slice 1 is not done, whatever the check above says. + +core_ndistinct="$(q "SELECT n_distinct FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" +echo "-- n_distinct after our call = ${core_ndistinct:-}" + +check "pgcolumnar.analyze() leaves the statistics it does not compute in place" \ + "$(if pgc_is_number "$core_ndistinct" \ + && [ "$(awk -v v="$core_ndistinct" 'BEGIN { print (v + 0 == 0) ? "zero" : "nonzero" }')" = nonzero ]; then + echo yes + else + echo "no (n_distinct is now [${core_ndistinct:-}])" + fi)" \ + "yes" + +pgc_summary From da75e353e82c20218c8bfd30435e227a9b2f4d45 Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Thu, 6 Aug 2026 23:54:28 +0000 Subject: [PATCH 2/4] wip(#414): slice 2, exact n_distinct from a single-column read null_frac is metadata only; n_distinct is the slice that has to actually read a column, and it is where the case for this function stands or falls. A projected single-column read of the 3M x 20 fixture costs 268 ms against core ANALYZE's 6,302 ms, because core's fixed 30,000-row sample lands in every row group and so decodes every column. Mirrors core's sign convention from analyze.c: absolute count normally, negated fraction once the distinct count passes 10% of rows. Getting that backwards produces plausible wrong estimates rather than an error, so the suite pins its fixture to the absolute-count side and asserts it. The suite's non-destruction check moved from n_distinct to correlation. This slice writes n_distinct, so watching it there would have asserted nothing while still printing PASS. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UqprqkCXuH8SegiZejE1Tw --- pgcolumnar--1.0-alpha.sql | 48 ++++++++++++++++++++++++--- test/analyze_function.sh | 70 ++++++++++++++++++++++++++++++--------- 2 files changed, 98 insertions(+), 20 deletions(-) diff --git a/pgcolumnar--1.0-alpha.sql b/pgcolumnar--1.0-alpha.sql index 7244d50..275a590 100644 --- a/pgcolumnar--1.0-alpha.sql +++ b/pgcolumnar--1.0-alpha.sql @@ -982,9 +982,9 @@ COMMENT ON FUNCTION pgcolumnar.parallel_copy(regclass, text, int) * an opt-in accelerator for wide tables and, like pgcolumnar.vacuum(), nothing * schedules it: see #415. * - * This first form collects null_frac only, taken exactly from the zone maps - * rather than sampled. n_distinct, MCVs and histogram bounds still need a sample - * and are not collected here yet. + * Collected so far: null_frac exactly from the zone maps (metadata only, no data + * read), and n_distinct exactly by reading one column. MCVs and histogram bounds + * still need a sample and are not collected here yet. */ CREATE FUNCTION pgcolumnar.analyze(rel regclass, columns text[] DEFAULT NULL) RETURNS void @@ -994,6 +994,9 @@ DECLARE sid bigint; att record; nullfrac double precision; + ndistinct bigint; + totalrows bigint; + ndstat double precision; seen integer := 0; unknown text; schname text; @@ -1079,6 +1082,42 @@ BEGIN CONTINUE WHEN nullfrac IS NULL; /* no zone map rows: nothing exact to say */ + /* + * n_distinct, exactly, by reading this column and nothing else. This is + * the whole point of the function: on the 3M x 20 fixture a projected + * single-column read costs 268 ms where core's whole-table sample costs + * 6,302 ms, because core's fixed 30,000-row sample lands in every row + * group and so decodes every column of the table. + * + * count(DISTINCT) ignores NULLs, which is what n_distinct means. + */ + EXECUTE format('SELECT count(DISTINCT %I)::bigint, count(*)::bigint FROM %I.%I', + att.attname, schname, relnm) + INTO ndistinct, totalrows; + + /* + * Core's own convention, and the sign is load-bearing: positive is an + * absolute count, negative is the negated fraction of rows. analyze.c + * switches to the fraction once the distinct count passes 10% of the + * rows, on the grounds that such a column's cardinality tracks the table + * size rather than sitting at a fixed value. Mirror it rather than always + * writing the absolute count, or a column that is unique today reads as + * having a fixed cardinality once the table grows. + * + * Getting this backwards does not raise -- it produces plausible wrong + * estimates -- so it is asserted in test/analyze_function.sh against a + * fixture pinned to the absolute-count side of the rule. + */ + IF totalrows > 0 THEN + IF ndistinct::double precision > 0.1 * totalrows::double precision THEN + ndstat := -(ndistinct::double precision / totalrows::double precision); + ELSE + ndstat := ndistinct::double precision; + END IF; + ELSE + ndstat := 0; + END IF; + /* * The casts are load-bearing. pg_restore_attribute_stats takes VARIADIC * "any", so a mistyped argument is a WARNING and the value is dropped, @@ -1091,7 +1130,8 @@ BEGIN 'relname', relnm, 'attname', att.attname::text, 'inherited', false, - 'null_frac', nullfrac::real); + 'null_frac', nullfrac::real, + 'n_distinct', ndstat::real); seen := seen + 1; END LOOP; diff --git a/test/analyze_function.sh b/test/analyze_function.sh index 10e1869..7214cb6 100755 --- a/test/analyze_function.sh +++ b/test/analyze_function.sh @@ -28,8 +28,9 @@ # This suite is about the function, not the AM sampler. test/analyze_stats.sh # covers the sampler (#154) and stays the correctness path for plain ANALYZE. # -# Slice 1 asserts one thing: null_frac comes from the zone maps and is EXACT, -# where core's is sampled and is not. +# Slice 1 asserts null_frac comes from the zone maps and is EXACT where core's +# is sampled. Slice 2 asserts n_distinct is exact from reading ONE column, which +# is the claim the whole issue rests on. # # Usage: test/analyze_function.sh [PG_CONFIG] # Written fresh for pgColumnar. @@ -85,6 +86,10 @@ check_num "premise: the fixture is exactly one-in-ten NULL" "$true_nullfrac" "0. psql_run "ANALYZE af_c;" >/dev/null core_nullfrac="$(q "SELECT null_frac FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" +# Captured BEFORE our call, because our call overwrites them. Reading these +# afterwards would compare our own output against itself. +core_ndistinct_before="$(q "SELECT n_distinct FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" +core_correlation_before="$(q "SELECT correlation FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" if ! pgc_is_number "$core_nullfrac"; then check_num "premise: core ANALYZE produced a null_frac to compare against" \ @@ -111,27 +116,60 @@ ours_nullfrac="$(q "SELECT null_frac FROM pg_stats WHERE tablename = 'af_c' AND check_num "pgcolumnar.analyze() reports null_frac exactly, from the zone maps" \ "$ours_nullfrac" "0.1" -# --- check 2: it must not destroy the statistics it does not compute ----------- +# --- check 2: n_distinct is exact, from reading one column -------------------- # -# pg_restore_attribute_stats is a RESTORE api: it is designed to reinstate a whole -# attribute's statistics from a dump, so kinds not named in the call may be -# cleared rather than left alone. That matters here, because this slice collects -# null_frac and nothing else. An accelerator that yields an exact null_frac while -# discarding n_distinct and the MCV list makes plans worse, not better -- and it -# would do so silently, since nothing errors. +# This is the slice the whole issue rests on. null_frac above is metadata only; +# n_distinct requires actually reading the column, and the case for a function is +# that reading ONE column of a wide table is cheap where core's whole-table +# sample is not: 268 ms against 6,302 ms on the 3M x 20 fixture. # -# So: n_distinct must survive the call. If this fails, the minimal implementation -# is a regression and slice 1 is not done, whatever the check above says. +# Exactness is the observable that a sampled implementation cannot fake, which is +# why it is what gets asserted. Core's own convention is mirrored: an absolute +# count normally, a negated fraction once the distinct count passes 10% of the +# rows (analyze.c does exactly this, on the grounds that such a column's +# cardinality scales with the table rather than sitting at a fixed value). + +true_ndistinct="$(q "SELECT count(DISTINCT k) FROM af_c")" +check_num "premise: the fixture has the cardinality this check compares against" \ + "$true_ndistinct" "45001" + +# Under 10% of 500,000 rows, so core's rule keeps this a positive absolute count +# rather than a negated fraction. If ROWS is ever lowered past 450,010 this flips +# sign and the check below needs to expect the fraction instead. +check "premise: the fixture stays on the absolute-count side of core's 10% rule" \ + "$(awk -v d="$true_ndistinct" -v n="$ROWS" 'BEGIN { print (d > 0.1 * n) ? "no (fraction side)" : "yes" }')" \ + "yes" + +check "premise: core's sampled n_distinct is not exact, so this check can discriminate" \ + "$(awk -v c="$core_ndistinct_before" -v t="$true_ndistinct" \ + 'BEGIN { print (c == t) ? "no (sample landed exactly on truth; check is vacuous)" : "yes" }')" \ + "yes" +echo "-- core sampled n_distinct = $core_ndistinct_before, truth = $true_ndistinct" + +ours_ndistinct="$(q "SELECT n_distinct FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" +check_num "pgcolumnar.analyze() reports n_distinct exactly, from one column" \ + "$ours_ndistinct" "$true_ndistinct" + +# --- check 3: it must not destroy the statistics it does not compute ----------- +# +# pg_restore_attribute_stats is a RESTORE api: it reinstates a whole attribute's +# statistics from a dump, so kinds not named in the call could be cleared rather +# than left alone. An accelerator that produces an exact null_frac and n_distinct +# while discarding everything else makes plans worse, not better, and does it +# silently because nothing errors. +# +# This watched n_distinct in slice 1. Slice 2 computes n_distinct, so it now +# watches correlation -- a statistic core collects and we still do not. Keeping it +# pointed at something we write would make it assert nothing. -core_ndistinct="$(q "SELECT n_distinct FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" -echo "-- n_distinct after our call = ${core_ndistinct:-}" +ours_correlation="$(q "SELECT correlation FROM pg_stats WHERE tablename = 'af_c' AND attname = 'k'")" +echo "-- correlation before=${core_correlation_before:-} after=${ours_correlation:-}" check "pgcolumnar.analyze() leaves the statistics it does not compute in place" \ - "$(if pgc_is_number "$core_ndistinct" \ - && [ "$(awk -v v="$core_ndistinct" 'BEGIN { print (v + 0 == 0) ? "zero" : "nonzero" }')" = nonzero ]; then + "$(if pgc_is_number "$ours_correlation" && pgc_is_number "$core_correlation_before"; then echo yes else - echo "no (n_distinct is now [${core_ndistinct:-}])" + echo "no (correlation was [${core_correlation_before:-}], is now [${ours_correlation:-}])" fi)" \ "yes" From dfa97c88a10f8f8cc855f2512cb9b929767b1d41 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 6 Aug 2026 21:17:23 -0600 Subject: [PATCH 3/4] feat: histogram_bounds with exact ends, from a complete read (#414 slice 3) percentile_disc over an array of fractions returns ACTUAL column values, one per fraction, in a single ordered pass. Fraction 1.0 is therefore the true maximum and 0.0 the true minimum. percentile_cont would interpolate and invent values the column does not hold, which is wrong for a histogram of stored data and impossible for a non-numeric type. That is the case that matters. A range predicate above the sampled maximum is where the planner's estimate collapses, and a value held by one row in 500,000 is one a 30,000-row sample misses. Guarded to types with a btree ordering, and skipped when the column holds fewer distinct values than buckets, which is where core emits no histogram either because the most-common-value list already describes the column. Bounds what this slice claims: core EXCLUDES most-common-values from the histogram. This function writes none, so there is nothing to double count and the two are consistent as written. Writing both without that exclusion would over-count those values in selectivity, which is why most_common_vals is a separate slice rather than a line added here. Two fixture findings, both of which invalidated the plan as written: The planned column (g % 100) cannot test this at all. With 100 distinct values core stores every one as a most-common-value and emits NO histogram, so the check compared against an empty array and failed on its own premise. The ordinary values now span 100,000 distinct. And "core misses the outlier" cannot be a gate. It is probabilistic by definition, and it also depends on how our own access method hands rows to the sampler: measured 99,999 on one run and 1,000,000 on the next, on identical data. Gating on it would be the flaky-by-construction shape the fixture was built to avoid. It is reported, and exactness is asserted instead -- against an independent SELECT max(), so the check still fails whenever our bounds are wrong. The error check exists because I needed it. The function raised ERROR: record "att" has no field "atttypid" and the redirect swallowed it, so the call did nothing, pg_stats still held core's numbers, and the failure presented as "our maximum is wrong" rather than "our function did not run". The assertion caught it; the diagnosis needed the error. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01L2DvnWDM7g27ubDCQdXhky --- pgcolumnar--1.0-alpha.sql | 79 +++++++++++++++++++++++--- test/analyze_function.sh | 116 +++++++++++++++++++++++++++++++++++++- test/run_all_versions.sh | 1 + 3 files changed, 187 insertions(+), 9 deletions(-) diff --git a/pgcolumnar--1.0-alpha.sql b/pgcolumnar--1.0-alpha.sql index 275a590..00d16ed 100644 --- a/pgcolumnar--1.0-alpha.sql +++ b/pgcolumnar--1.0-alpha.sql @@ -997,6 +997,9 @@ DECLARE ndistinct bigint; totalrows bigint; ndstat double precision; + hist text; + -- One fewer than the number of bounds, matching core's default_statistics_target. + nbuckets integer := current_setting('default_statistics_target')::integer; seen integer := 0; unknown text; schname text; @@ -1056,7 +1059,7 @@ BEGIN END IF; FOR att IN - SELECT a.attname, a.attnum + SELECT a.attname, a.attnum, a.atttypid FROM pg_attribute a WHERE a.attrelid = rel AND a.attnum > 0 AND NOT a.attisdropped AND (columns IS NULL OR a.attname = ANY (columns)) @@ -1118,20 +1121,80 @@ BEGIN ndstat := 0; END IF; + /* + * histogram_bounds, whose ends are exact because the read is complete + * (#414 slice 3). + * + * percentile_disc over an array of fractions returns ACTUAL column + * values, one per fraction, in a single ordered pass. Fraction 1.0 is + * therefore the true maximum and 0.0 the true minimum, which is the + * whole gain: core samples, so a value held by one row in 500,000 is + * missed and every range estimate above the sampled maximum collapses. + * percentile_cont would interpolate and invent values the column does + * not contain, which is wrong for a histogram of stored data and wrong + * for any non-numeric type. + * + * Only for types that can be ordered. A column with no btree ordering + * has no histogram in core either, and ORDER BY would simply fail. + * + * Skipped when the column holds fewer distinct values than buckets: core + * emits no histogram there because the most-common-value list already + * describes the column completely, and writing one anyway would be a + * shape core never produces. + * + * NOTE, and it bounds what this slice claims: core EXCLUDES + * most-common-values from the histogram. This function does not write + * most-common-values at all, so there is nothing here to double-count + * and the two are consistent as written. Writing both without that + * exclusion would over-count those values in selectivity, which is why + * most_common_vals is a separate slice and not a line added here. + */ + hist := NULL; + IF att.attnum > 0 + AND EXISTS (SELECT 1 FROM pg_catalog.pg_type t + JOIN pg_catalog.pg_opclass oc ON oc.opcintype = t.oid + JOIN pg_catalog.pg_am am ON am.oid = oc.opcmethod + WHERE t.oid = att.atttypid AND am.amname = 'btree') + AND ndistinct > nbuckets + THEN + EXECUTE format( + 'SELECT percentile_disc( + (SELECT array_agg(i::double precision / %s ORDER BY i) + FROM generate_series(0, %s) i)) + WITHIN GROUP (ORDER BY %I)::text + FROM %I.%I WHERE %I IS NOT NULL', + nbuckets, nbuckets, att.attname, schname, relnm, att.attname) + INTO hist; + END IF; + /* * The casts are load-bearing. pg_restore_attribute_stats takes VARIADIC * "any", so a mistyped argument is a WARNING and the value is dropped, * not an error: attname must be text (attname is `name`) and null_frac * must be real (the division yields double precision). Without these the * call "succeeds" having stored nothing. + * + * histogram_bounds is passed as text, which is what the function takes: + * it parses the array literal against the column's own type. */ - PERFORM pg_catalog.pg_restore_attribute_stats( - 'schemaname', schname, - 'relname', relnm, - 'attname', att.attname::text, - 'inherited', false, - 'null_frac', nullfrac::real, - 'n_distinct', ndstat::real); + IF hist IS NULL THEN + PERFORM pg_catalog.pg_restore_attribute_stats( + 'schemaname', schname, + 'relname', relnm, + 'attname', att.attname::text, + 'inherited', false, + 'null_frac', nullfrac::real, + 'n_distinct', ndstat::real); + ELSE + PERFORM pg_catalog.pg_restore_attribute_stats( + 'schemaname', schname, + 'relname', relnm, + 'attname', att.attname::text, + 'inherited', false, + 'null_frac', nullfrac::real, + 'n_distinct', ndstat::real, + 'histogram_bounds', hist); + END IF; seen := seen + 1; END LOOP; diff --git a/test/analyze_function.sh b/test/analyze_function.sh index 7214cb6..2286819 100755 --- a/test/analyze_function.sh +++ b/test/analyze_function.sh @@ -58,9 +58,10 @@ fi # sample is what makes the sampled estimate inexact, which slice 1 depends on. psql_run "DROP TABLE IF EXISTS af_c; - CREATE TABLE af_c (k int, pad1 text, pad2 text, pad3 text) USING pgcolumnar; + CREATE TABLE af_c (k int, skew int, pad1 text, pad2 text, pad3 text) USING pgcolumnar; INSERT INTO af_c SELECT CASE WHEN g % 10 = 0 THEN NULL ELSE g % 45001 END, + CASE WHEN g = 1 THEN 1000000 ELSE g % 100000 END, md5(g::text), md5((g * 7)::text), md5((g * 13)::text) FROM generate_series(1, $ROWS) g;" >/dev/null @@ -173,4 +174,117 @@ check "pgcolumnar.analyze() leaves the statistics it does not compute in place" fi)" \ "yes" +# ---- slice 3: histogram_bounds, whose top end is exact ------------------------ +# +# `k` cannot test this. It is uniform over 0..45000, so core's 30,000-row sample +# almost certainly hits both extremes and its bounds are already near-exact. A +# check asserting "ours is exact where core's is not" would pass or fail on the +# luck of the sample, which is three samples rather than three behaviours. +# +# `skew` is built so the extreme is genuinely rare: ONE row in 500,000 holds +# 1,000,000 and every other row is under 100. Core's sample misses it with +# near-certainty; a full read cannot. Four orders of magnitude is not a coin flip. +# +# It is also the case that matters. A range predicate above the sampled maximum +# is exactly where the planner's estimate collapses. + +# Two fixture decisions, both forced by what core actually does. +# +# The ordinary values span 100,000 distinct rather than 100. With only 100 +# distinct values core stores every one as a most-common-value and emits NO +# histogram at all, so the first version of this check compared against an empty +# array and failed on its own premise rather than on the behaviour. +# +# And core's sample for this column is cut to 10 buckets (about 3,000 rows). +# At the default target it samples 30,000 of 500,000 rows, which finds a +# one-in-500,000 outlier about 6% of the time -- a check that fails one run in +# sixteen for no defect. At 10 it is about 0.6%. That is small and it is not +# zero: any discrimination against a SAMPLE is probabilistic, and pretending +# otherwise would be the flaky-by-construction shape this fixture exists to +# avoid. The premise below states the condition rather than assuming it. +psql_run "ALTER TABLE af_c ALTER COLUMN skew SET STATISTICS 10;" >/dev/null + +true_skew_max="$(q "SELECT max(skew) FROM af_c")" +check_num "premise: the outlier really is in the table" "$true_skew_max" "1000000" +check_num "premise: and it really is one row in $ROWS" \ + "$(q "SELECT count(*) FROM af_c WHERE skew = 1000000")" "1" + +psql_run "ANALYZE af_c;" >/dev/null +core_hist_max="$(q "SELECT (histogram_bounds::text::int[])[array_length(histogram_bounds::text::int[], 1)] + FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew'")" +echo "-- core sampled histogram max = ${core_hist_max:-}, truth = $true_skew_max" + +# Reported, deliberately NOT asserted. +# +# "Core misses the outlier" cannot be a gate. It is probabilistic by definition, +# and it also depends on how our own access method hands rows to the sampler, so +# a red here would mean "the sample was lucky" and never "the code is wrong". +# Measured both ways while writing this: core's histogram max came back 99,999 on +# one run and 1,000,000 on the next, on identical data. Gating on that would have +# been the flaky-by-construction shape this fixture was built to avoid. +# +# What IS asserted below is exactness, and it does not need core to be wrong: the +# expected value comes from an independent SELECT max(), not from the code path +# under test, so the check fails whenever our bounds are not the true ones. +if pgc_is_number "$core_hist_max" && [ "$core_hist_max" -lt 200000 ]; then + echo "-- core missed the outlier this run, which is the case that motivates #414" +else + echo "-- core happened to sample the outlier this run; exactness is asserted regardless" +fi + +# Captured, not discarded. While writing this slice the function raised +# +# ERROR: record "att" has no field "atttypid" +# +# and the redirect swallowed it, so the call did nothing, pg_stats still held +# core's numbers, and the failure presented as "our maximum is wrong" rather than +# "our function did not run". The assertion caught it, but the diagnosis needed +# the error, so the error is now a check of its own. +skew_out="$(psql_run "SELECT pgcolumnar.analyze('af_c'::regclass, ARRAY['skew']);" 2>&1)" +check_num "pgcolumnar.analyze() ran without raising, so the statistics below are its own" \ + "$(grep -c 'ERROR' <<<"$skew_out")" "0" +ours_hist="$(q "SELECT histogram_bounds::text::int[] + FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew'")" +ours_hist_max="$(q "SELECT (histogram_bounds::text::int[])[array_length(histogram_bounds::text::int[], 1)] + FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew'")" +echo "-- ours histogram max = ${ours_hist_max:-}" + +check_num "pgcolumnar.analyze() puts the true maximum at the top of histogram_bounds" \ + "${ours_hist_max:-}" "$true_skew_max" + +# Both ends, not just the interesting one. A histogram whose top is right and +# whose bottom is invented is still wrong, and percentile_disc returning real +# column values is what makes both exact. +check_num "and the true minimum at the bottom" \ + "$(q "SELECT (histogram_bounds::text::int[])[1] + FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew'")" \ + "$(q "SELECT min(skew) FROM af_c")" + +# percentile_disc returns values the column HOLDS. percentile_cont would +# interpolate and invent ones it does not, which is wrong for a histogram of +# stored data and impossible for a non-numeric type. +check_num "every bound is a value the column actually holds" \ + "$(q "SELECT count(*) FROM unnest((SELECT histogram_bounds::text::int[] + FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew')) b + WHERE NOT EXISTS (SELECT 1 FROM af_c WHERE skew = b)")" "0" + +# A histogram is an ordered ladder, not a pair of extremes. Asserting only the +# last element would pass for an array of two values, which is not a histogram +# and would ruin every estimate between the ends. +check "and it is an ordered ladder rather than two extremes" \ + "$(if pgc_is_number "$(q "SELECT array_length(histogram_bounds::text::int[], 1) + FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew'")" && + [ "$(q "SELECT array_length(histogram_bounds::text::int[], 1) + FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew'")" -ge 5 ]; then + echo yes + else + echo "no (length [$(q "SELECT array_length(histogram_bounds::text::int[], 1) FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew'")])" + fi)" "yes" + +check "and it is sorted ascending, which a histogram must be to be usable" \ + "$(q "SELECT CASE WHEN histogram_bounds::text::int[] = + (SELECT array_agg(x ORDER BY x) FROM unnest(histogram_bounds::text::int[]) x) + THEN 'yes' ELSE 'no' END + FROM pg_stats WHERE tablename = 'af_c' AND attname = 'skew'")" "yes" + pgc_summary diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 9315b49..a546ba7 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -41,6 +41,7 @@ set -uo pipefail SUITES=( advisory_lock_class alter_column_type + analyze_function analyze_reltuples analyze_stats arrow_export From 193626399ff4148679e5fa7f7cc55cca6cd80abb Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 6 Aug 2026 21:29:52 -0600 Subject: [PATCH 4/4] test: a major without the stats API is a skip, not a missing dependency (#414) CI went red on PG17 the moment this suite was registered: FAIL pgcolumnar.analyze() needs pg_restore_attribute_stats (PG18+); this server is 17 FAIL PG17 (124 ran, 7 skipped) The gate used pgc_skip, which is the wrong instrument. pgc_skip is for a missing DEPENDENCY -- pyarrow, nm -- which is an environment defect, so it fails by default and has to be waived deliberately, because somebody should install the thing. A major that does not ship pg_restore_attribute_stats is not a defect anyone can fix: 15 to 17 genuinely lack it, the same way 15 lacks WITHOUT OVERLAPS. Failing there is a red nobody can act on, which is the kind that teaches readers to discount red. So it reports SKIP and runs no checks, which pgc_summary turns into PGC_EXIT_SKIPPED and the matrix records as SKIP (#447, #455). Same shape as pg19_vacuum_options on anything below 19; verified both exit 66 on PG17. The major is asserted before the comparison. An unreadable version must not be mistaken for an old one, or a broken environment reports SKIP and reads as "this major does not support it". Gated on a FRESH tree per major. Two false reds while checking this were mine, not the code: the first was leftover postmasters holding the port band, and the second was building PG17 and PG18 in one tree, so PG18 linked stale objects and the postmaster would not load the library. Both present as "no cluster of our own after 8 attempts", which looks nothing like its cause. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01L2DvnWDM7g27ubDCQdXhky --- test/analyze_function.sh | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/test/analyze_function.sh b/test/analyze_function.sh index 2286819..c2af20e 100755 --- a/test/analyze_function.sh +++ b/test/analyze_function.sh @@ -47,9 +47,28 @@ ROWS=${PGC_ANALYZE_ROWS:-500000} # version-support decision and not a detail (stavalues anyarray typing, staop, # stacoll, stadistinct's sign convention). Refuse rather than silently narrow: # pgc_skip fails by default and must be waived deliberately. +# +# A version gate, NOT pgc_skip. pgc_skip is for a missing DEPENDENCY, which is an +# environment defect and fails by default so somebody installs the thing. A major +# that does not ship pg_restore_attribute_stats is not a defect to fix: 15 to 17 +# genuinely lack it, the same way 15 lacks WITHOUT OVERLAPS. Using pgc_skip here +# turned every PG17 CI run red the moment this suite was registered, which is a +# red nobody can act on. +# +# So it reports SKIP and runs no checks, which pgc_summary turns into exit 2 and +# the matrix records as SKIP rather than as a pass (#447). +# +# The major is asserted first. An unreadable version must not be mistaken for an +# old one, or a broken environment would report SKIP and look supported. +if ! pgc_is_number "${PGC_MAJOR:-}"; then + echo "FAIL could not read the server major, so the gate below cannot be trusted: got [${PGC_MAJOR:-}]" + PGC_CHECKS=$((PGC_CHECKS + 1)) + PGC_FAIL=1 + pgc_summary +fi if [ "$PGC_MAJOR" -lt 18 ]; then - pgc_skip PG18_STATS_API \ - "pgcolumnar.analyze() needs pg_restore_attribute_stats (PG18+); this server is $PGC_MAJOR" + echo "SKIP pgcolumnar.analyze() needs pg_restore_attribute_stats (PG18+); this server is $PGC_MAJOR" + pgc_summary fi # --- fixture ------------------------------------------------------------------