From ee06908cba2222983ae75e805ca7e3f52293b061 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Tue, 1 Sep 2026 18:45:51 +0000 Subject: [PATCH 1/2] fix: refuse TABLESAMPLE rather than returning every row Co-authored-by: Cursor --- src/columnar_customscan.c | 8 ++++ src/columnar_vector.c | 4 ++ test/run_all_versions.sh | 1 + test/tablesample.sh | 77 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+) create mode 100755 test/tablesample.sh diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index c295b89d..90070e53 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -2527,6 +2527,14 @@ PgColumnarSetRelPathlist(PlannerInfo *root, RelOptInfo *rel, Index rti, return; if (rte->rtekind != RTE_RELATION || rte->relkind != RELKIND_RELATION) return; + /* + * TABLESAMPLE must reach the AM sample callbacks, which raise 0A000. + * Without this, the custom scan replaces Sample Scan and returns every + * row. docs/limitations.md states the error; the AM already implements + * it. The hook was the hole. + */ + if (rte->tablesample != NULL) + return; /* * A partition is RELOPT_OTHER_MEMBER_REL, not RELOPT_BASEREL, and excluding * it cost the custom scan entirely: no column projection, no zone-map diff --git a/src/columnar_vector.c b/src/columnar_vector.c index 6f2ed530..f646d75f 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -959,6 +959,8 @@ PgColumnarCreateUpperPaths(PlannerInfo *root, UpperRelationKind stage, if (rte == NULL || rte->rtekind != RTE_RELATION || rte->relkind != RELKIND_RELATION) return; + if (rte->tablesample != NULL) + return; if (!OidIsValid(rte->relid) || !PgColumnarIsColumnarRelation(rte->relid)) return; relid = rte->relid; @@ -1781,6 +1783,8 @@ PgColumnarTryGroupAggPath(PlannerInfo *root, RelOptInfo *input_rel, if (rte == NULL || rte->rtekind != RTE_RELATION || rte->relkind != RELKIND_RELATION) return; + if (rte->tablesample != NULL) + return; /* * A legacy inheritance parent is a plain RELKIND_RELATION with rte->inh set; * its children hold rows this single-relation scan would never see. Leave the diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 28d85fa0..d687ae62 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -260,6 +260,7 @@ SUITES=( sorted_pathkeys sorted_projection stats_privilege + tablesample temporal ttl_expire ungrouped_vector_agg diff --git a/test/tablesample.sh b/test/tablesample.sh new file mode 100755 index 00000000..3d416fdc --- /dev/null +++ b/test/tablesample.sh @@ -0,0 +1,77 @@ +#!/usr/bin/env bash +# +# TABLESAMPLE must raise, not return the whole table. +# +# docs/limitations.md: "TABLESAMPLE is unsupported and says so: it raises an +# error rather than returning no rows." The AM sample callbacks already raise +# 0A000. The planner hooks did not look at rte->tablesample, so a custom scan +# or vectorized aggregate replaced Sample Scan and answered every row. +# +# Measured on 10,000 rows at shipped defaults: +# +# SELECT count(*) FROM n TABLESAMPLE BERNOULLI(10) REPEATABLE (7) +# heap: ~1000 +# columnar: 10000, plan: Custom Scan (Columnar Vector Agg) +# +# and on a wide table the same silent full scan, plan: Custom Scan (Columnar Scan). +# A shape that still reached Sample Scan did raise. The contract is the error, +# not a sample, so this pins 0A000 on the shapes that used to succeed. +# +# The arms assert SQLSTATE rather than message text. A grep for "not supported" +# is also satisfied by a dozen unrelated errors, and by a connection failure. +# +# Usage: test/tablesample.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}" + +sqlstate() { + local out code + out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -Atq -v VERBOSITY=verbose \ + -c "\\set VERBOSITY verbose" \ + -c "$1" 2>&1)" + code="$(printf '%s\n' "$out" | sed -n 's/.*ERROR:[[:space:]]*\([0-9A-Z]\{5\}\):.*/\1/p' | head -1)" + if [ -n "$code" ]; then printf '%s\n' "$code"; else printf '00000\n'; fi +} + +psql_run "CREATE TABLE n (id int, v text) USING pgcolumnar;" +psql_run "INSERT INTO n SELECT g, 'x' || g FROM generate_series(1, 10000) g;" +check_num "premise: the table holds 10000 rows" \ + "$(q 'SELECT count(*) FROM n')" "10000" + +# The defect was that this succeeded with 10000. After the fix it must refuse. +st_count="$(sqlstate 'SELECT count(*) FROM n TABLESAMPLE BERNOULLI(10) REPEATABLE (7);')" +echo "-- count(*) TABLESAMPLE SQLSTATE: ${st_count:-}" +check "count(*) TABLESAMPLE raises 0A000 rather than answering every row" \ + "$st_count" "0A000" + +st_sum="$(sqlstate 'SELECT sum(id) FROM n TABLESAMPLE BERNOULLI(10) REPEATABLE (7);')" +check "sum() TABLESAMPLE raises 0A000 rather than folding the whole table" \ + "$st_sum" "0A000" + +st_sys="$(sqlstate 'SELECT count(*) FROM n TABLESAMPLE SYSTEM(10) REPEATABLE (7);')" +check "SYSTEM TABLESAMPLE raises 0A000 as well" "$st_sys" "0A000" + +st_proj="$(sqlstate 'SELECT id FROM n TABLESAMPLE BERNOULLI(10) REPEATABLE (7);')" +check "a projected TABLESAMPLE scan raises 0A000 rather than returning every id" \ + "$st_proj" "0A000" + +st_group="$(sqlstate 'SET pgcolumnar.enable_group_vectorization=on; SELECT id % 10, count(*) FROM n TABLESAMPLE BERNOULLI(10) REPEATABLE (7) GROUP BY 1;')" +check "grouped TABLESAMPLE raises 0A000 rather than grouping every row" \ + "$st_group" "0A000" + +# control: the probe distinguishes success from 0A000, and heap still samples +st_ok="$(sqlstate 'SELECT count(*) FROM n;')" +check "control: an unsampled count reports success, so the probe is not stuck on 0A000" \ + "$st_ok" "00000" + +psql_run "CREATE TABLE h (id int, v text);" +psql_run "INSERT INTO h SELECT g, 'x' || g FROM generate_series(1, 10000) g;" +st_heap="$(sqlstate 'SELECT count(*) FROM h TABLESAMPLE BERNOULLI(10) REPEATABLE (7);')" +check "control: the same TABLESAMPLE on a heap table raises nothing" \ + "$st_heap" "00000" + +pgc_summary From 64cb0c9fdf1b8308584e17a8663fed8e94108b79 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 2 Sep 2026 02:29:41 +0000 Subject: [PATCH 2/2] docs: record the TABLESAMPLE refusal in the changelog (#866) The change is user-visible and shipped without its entry. Every claim in the entry was measured on this branch rather than taken from the pull request summary. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011QP9UpEMdAj814XAPmftAH --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 333c91c8..9eed10b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,23 @@ true until the next version shipped. the column type. A file whose type is not temporal is unaffected, so importing a plain `int64` file into a `timestamp` column still works. +- `TABLESAMPLE` on a columnar table is refused rather than silently ignored + (#866). + + The AM's sample callbacks have always raised `0A000`, and + `docs/limitations.md` has always said so. The planner hooks never let a query + reach them: `set_rel_pathlist_hook` added a custom scan in place of the Sample + Scan, and the vectorized aggregate paths did the same for an aggregate. The + sample was then not applied at all. Measured on 10,000 rows with a heap mirror + as the oracle: `TABLESAMPLE BERNOULLI (10)` returned 1030 rows from the heap + table and **all 10,000** from the columnar one, and `sum(id)` came back as the + whole table's sum rather than a sample's. `SYSTEM` behaved the same way. + + A wrong answer is worse than a refusal, and the refusal is the documented + behaviour. All three hooks that read the range-table entry now decline when + `rte->tablesample` is set, so the query reaches the callbacks and raises. An + unsampled query on the same table still takes the custom scan. + - A shebang and the execute bit go together, and every directory that documents a command is swept (#856). Two things were left over from #852.