commandprompt · ChronicallyJD · Aug 7, 2026 · Aug 7, 2026
diff --git a/bench/cb_guards.sh b/bench/cb_guards.sh
@@ -0,0 +1,58 @@
+#!/usr/bin/env bash
+#
+# Guards for the ClickBench harness (#465).
+#
+# Separate from run_clickbench.sh so they can be sourced and tested without the
+# 15 GB download and the tuned cluster the benchmark needs. test/bench_guards.sh
+# runs them in the ordinary matrix; the benchmark itself cannot.
+#
+# Sourced, not executed. Nothing here touches a database or the filesystem.
+# Written fresh for pgColumnar.
+
+# cb_prepared_xacts_ok <current> <workers>
+#
+# pgcolumnar.parallel_copy prepares one transaction per worker, so the cluster
+# needs at least that many slots. The stock max_prepared_transactions is 0, which
+# means every parallel arm errors out on its first worker.
+#
+# This must be asked BEFORE any arm is loaded, because the setting is
+# PGC_POSTMASTER: raising it needs a restart, and discovering it mid-run wastes
+# the whole run.
+#
+# A serial arm asks for 0 workers and needs no slots, so it is never blocked by a
+# setting it does not use.
+cb_prepared_xacts_ok() {
+	local current="${1:-}" workers="${2:-}"
+	case "$current" in '' | *[!0-9]*) return 1 ;; esac
+	case "$workers" in '' | *[!0-9]*) return 1 ;; esac
+	[ "$current" -ge "$workers" ]
+}
+
+# cb_prepared_xacts_message <current> <workers>
+#
+# The message is the deliverable. Without it the operator meets a per-worker
+# error in a load log and has to work back to the cause; with it they are told
+# the setting, the value, and that it costs a restart.
+cb_prepared_xacts_message() {
+	local current="${1:-}" workers="${2:-}"
+	printf '%s\n' \
+		"max_prepared_transactions is ${current:-unset}, and a ${workers}-worker parallel arm needs ${workers}." \
+		"pgcolumnar.parallel_copy prepares one transaction per worker, so every arm would fail at once." \
+		"Set max_prepared_transactions = ${workers} (or more) and restart the postmaster; it cannot be changed in a session."
+}
+
+# cb_rows_ok <got> <want>
+#
+# A load that lost rows is a failure and not a fast result. An errored parallel
+# arm leaves an EMPTY table and returns in about no time, which reads as perfect
+# scaling; #465 records a harness that published exactly that.
+#
+# Both sides must be numbers. A psql that failed yields an empty string, and
+# comparing "" with "" passes while measuring nothing, which is the trap #418
+# exists to forbid.
+cb_rows_ok() {
+	local got="${1:-}" want="${2:-}"
+	case "$got" in '' | *[!0-9]*) return 1 ;; esac
+	case "$want" in '' | *[!0-9]*) return 1 ;; esac
+	[ "$got" = "$want" ]
+}
diff --git a/bench/run_clickbench.sh b/bench/run_clickbench.sh
@@ -115,6 +115,15 @@ CB_PORT="${PGC_CB_PORT:-58900}"
 CB_PGDATA="${PGC_CB_PGDATA:-$CB_DATA/pgdata}"
 CB_KEEP="${PGC_CB_KEEP:-0}"
 CB_MAXGROUPS="${PGC_CB_MAXGROUPS:-200000000}"
+# Workers for the parallel-load arm. 0 disables the arm entirely; the serial arm
+# is never affected. Reported ALONGSIDE the serial number, not instead of it:
+# serial COPY is the single-connection ingest path and parallel_copy is the bulk
+# path, and both are worth publishing (#465).
+CB_PCOPY_WORKERS="${PGC_CB_PCOPY_WORKERS:-16}"
+
+# The load guards live in their own file so the matrix can test them without the
+# 15 GB download this script needs. See test/bench_guards.sh.
+. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/cb_guards.sh"
 
 CB_URL_BASE="https://raw.githubusercontent.com/ClickHouse/ClickBench/main/postgresql"
 CB_TSV_URL="https://datasets.clickhouse.com/hits_compatible/hits.tsv.gz"
@@ -304,6 +313,10 @@ max_worker_processes = $(( NCPU + 15 ))
 max_parallel_workers = $NCPU
 max_parallel_workers_per_gather = $(( NCPU / 2 ))
 max_parallel_maintenance_workers = $(( NCPU / 2 ))
+# One prepared transaction per parallel_copy worker. The stock value is 0, which
+# makes every parallel arm fail on its first worker; the preflight below refuses
+# to start rather than let that be reported as a fast load (#465).
+max_prepared_transactions = $CB_PCOPY_WORKERS
 listen_addresses = ''
 unix_socket_directories = '/tmp'
 CONF
@@ -328,6 +341,22 @@ EXTVER=$($PSQL -At -c "SELECT extversion FROM pg_extension WHERE extname='pgcolu
 require "the extension is installed" "$([ -n "$EXTVER" ] && echo yes || echo no)" "yes" || exit 1
 note "   pgcolumnar $EXTVER on port $CB_PORT"
 
+# ---- preflight the parallel arm, before any row is loaded (#465) ------------
+#
+# max_prepared_transactions is PGC_POSTMASTER. Discovering it is too low during
+# the load means the run is already wasted, and the failure arrives as a
+# per-worker error in a load log rather than as its cause. So ask the RUNNING
+# cluster, not the file this script wrote, because an operator may be pointing it
+# at a cluster they tuned themselves.
+if [ "$CB_PCOPY_WORKERS" -gt 0 ]; then
+	CB_PREPARED=$($PSQL -At -c "SHOW max_prepared_transactions")
+	if ! cb_prepared_xacts_ok "$CB_PREPARED" "$CB_PCOPY_WORKERS"; then
+		cb_prepared_xacts_message "$CB_PREPARED" "$CB_PCOPY_WORKERS" >&2
+		die "the parallel arm cannot run; set PGC_CB_PCOPY_WORKERS=0 to skip it"
+	fi
+	note "   parallel arm: $CB_PCOPY_WORKERS workers, max_prepared_transactions=$CB_PREPARED"
+fi
+
 # ---------------------------------------------------------------------------
 # 4. One table per arm, from the same fetched DDL
 # ---------------------------------------------------------------------------
@@ -425,6 +454,54 @@ if printf '%s\n' "${ARMS[@]}" | grep -qx duckdb; then
 	note "   duckdb: ${LOAD_S[duckdb]}s, ${ROWS[duckdb]} rows, ${SIZE_B[duckdb]} bytes"
 fi
 
+# ---- the parallel load arm, reported beside the serial one (#465) ----------
+#
+# Serial COPY is the single-connection ingest path; pgcolumnar.parallel_copy is
+# the bulk path. They measure different capabilities and both are published, so
+# this is an EXTRA table rather than a replacement for the serial number.
+#
+# It is deliberately not in ARMS: the 43 queries have nothing to say about a
+# table that exists only to be loaded, and running them would double the run for
+# no reading. That also means it needs its own row assertion, below, rather than
+# inheriting the one over ARMS.
+#
+# The load must fail LOUDLY. With max_prepared_transactions too low every worker
+# errors at once and the load returns in about no time, which reads as perfect
+# scaling: #465 records a harness that published 0.0s / 0.8s / 1.1s / 1.3s and
+# meant "every arm failed". The preflight above stops that before it starts, and
+# ON_ERROR_STOP plus the row check below stop it if it happens anyway.
+if [ "$CB_PCOPY_WORKERS" -gt 0 ] && [ -n "${ROWS[hits_col]:-}" ]; then
+	pc_tbl=hits_col_pcopy
+	$PSQL -c "DROP TABLE IF EXISTS $pc_tbl;" >/dev/null 2>&1
+	ddl_for "$pc_tbl" "USING pgcolumnar" | $PSQL -v ON_ERROR_STOP=1 >/dev/null 2>"$CB_DATA/ddl.pcopy.err"
+	if [ -s "$CB_DATA/ddl.pcopy.err" ]; then
+		head -5 "$CB_DATA/ddl.pcopy.err"; die "parallel arm DDL failed"
+	fi
+	t0=$(date +%s.%N)
+	$PSQL -v ON_ERROR_STOP=1 \
+		-c "SELECT pgcolumnar.parallel_copy('$pc_tbl'::regclass, '$TSV', $CB_PCOPY_WORKERS)" \
+		> "$CB_DATA/load.pcopy.log" 2>&1 \
+		|| { tail -10 "$CB_DATA/load.pcopy.log"; die "the parallel arm failed; it is not reported as a fast load"; }
+	$PSQL -c "VACUUM ANALYZE $pc_tbl;" >/dev/null 2>&1
+	t1=$(date +%s.%N)
+	LOAD_S[columnar_pcopy]=$(awk -v a="$t0" -v b="$t1" 'BEGIN { printf "%.1f", b - a }')
+	ROWS[columnar_pcopy]=$($PSQL -At -c "SELECT count(*) FROM $pc_tbl")
+	SIZE_B[columnar_pcopy]=$($PSQL -At -c "SELECT pg_total_relation_size('$pc_tbl')")
+	note "   columnar_pcopy (${CB_PCOPY_WORKERS}w): ${LOAD_S[columnar_pcopy]}s, ${ROWS[columnar_pcopy]} rows, ${SIZE_B[columnar_pcopy]} bytes"
+
+	# The assertion that stops a failed parallel load being published as a fast
+	# one. cb_rows_ok refuses a missing count rather than comparing it, so a psql
+	# that died does not compare "" with "" and pass (#418).
+	if cb_rows_ok "${ROWS[columnar_pcopy]}" "$TSV_ROWS"; then
+		printf 'ok     premise: %s\n' "the parallel arm loaded every row of the file"
+	else
+		printf 'FAIL   premise: %s (got [%s] want [%s])\n' \
+			"the parallel arm loaded every row of the file" \
+			"${ROWS[columnar_pcopy]}" "$TSV_ROWS" >&2
+		fail=1
+	fi
+fi
+
 # Every arm must hold the same rows as the file. A load that silently dropped
 # rows makes every query below faster and wrong.
 for arm in "${ARMS[@]}"; do
@@ -616,6 +693,12 @@ printf '%-16s %12s %16s %10s\n' arm 'load (s)' 'size (bytes)' 'errors'
 for arm in "${ARMS[@]}"; do
 	printf '%-16s %12s %16s %10s\n' "$arm" "${LOAD_S[$arm]}" "${SIZE_B[$arm]}" "${ERRS[$arm]:-0}"
 done
+if [ -n "${LOAD_S[columnar_pcopy]:-}" ]; then
+	printf '%-16s %12s %16s %10s\n' "columnar_pcopy" \
+		"${LOAD_S[columnar_pcopy]}" "${SIZE_B[columnar_pcopy]}" "-"
+	printf '   parallel_copy with %s workers, beside the serial number above; both are the point\n' \
+		"$CB_PCOPY_WORKERS"
+fi
 echo
 echo "-- hot times, milliseconds. 'x' is columnar over heap; above 1.00 means we lose."
 printf '%-6s' query

diff --git a/test/bench_guards.sh b/test/bench_guards.sh
@@ -0,0 +1,112 @@
+#!/usr/bin/env bash
+#
+# The benchmark's own guards (#465).
+#
+# bench/run_clickbench.sh publishes numbers. A benchmark that reports a failed
+# load as a fast one is worse than a benchmark that does not run, because the
+# number reaches documentation and nobody re-derives it.
+#
+# That is not hypothetical. pgcolumnar.parallel_copy prepares one transaction per
+# worker, and the stock max_prepared_transactions is 0, so EVERY parallel arm
+# errors out instantly. The first harness written against it printed those
+# failures as
+#
+#     0.0s / 0.8s / 1.1s / 1.3s
+#
+# which is indistinguishable from perfect scaling, and is the shape a reader
+# would publish. #465 records it.
+#
+# So the guards get a suite of their own, and it runs in the matrix even though
+# the benchmark it guards does not: the benchmark needs a 15 GB download and a
+# tuned cluster, while its arithmetic needs neither. Testing the decision without
+# the dataset is the whole point of putting it in a file that can be sourced.
+#
+# Usage:  test/bench_guards.sh [PG_CONFIG]
+# The argument is accepted and ignored; this suite needs no cluster.
+# Written fresh for pgColumnar.
+set -uo pipefail
+SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+
+PGC_CHECKS=0
+PGC_FAIL=0
+check() {
+	local name="$1" got="$2" want="$3"
+	PGC_CHECKS=$((PGC_CHECKS + 1))
+	if [ "$got" = "$want" ]; then
+		echo "PASS  $name"
+	else
+		echo "FAIL  $name: got [$got] want [$want]"
+		PGC_FAIL=1
+	fi
+}
+
+GUARDS="$SRCDIR/bench/cb_guards.sh"
+if [ ! -f "$GUARDS" ]; then
+	echo "FAIL  bench/cb_guards.sh is missing, so the benchmark's guards are untestable"
+	PGC_CHECKS=$((PGC_CHECKS + 1))
+	PGC_FAIL=1
+	echo; echo "checks run: $PGC_CHECKS"; echo "$(basename "$0"): FAILED"; exit 1
+fi
+# shellcheck source=/dev/null
+. "$GUARDS"
+
+echo "== pgColumnar test: $(basename "$0") =="
+
+# ---- max_prepared_transactions must be preflighted, not discovered ----------
+#
+# Raising it needs a postmaster restart, so finding out during the load means the
+# whole run is wasted. The guard has to answer before any arm is loaded.
+check "the stock 0 is refused for an 8-worker parallel arm" \
+	"$(cb_prepared_xacts_ok 0 8 && echo ok || echo refused)" "refused"
+check "an exact match is accepted" \
+	"$(cb_prepared_xacts_ok 8 8 && echo ok || echo refused)" "ok"
+check "more than enough is accepted" \
+	"$(cb_prepared_xacts_ok 16 8 && echo ok || echo refused)" "ok"
+check "one short is refused, which is the off-by-one that would run 7 workers" \
+	"$(cb_prepared_xacts_ok 7 8 && echo ok || echo refused)" "refused"
+
+# A serial arm needs none, and must not be blocked by a setting it does not use.
+check "a serial arm needs no prepared transactions" \
+	"$(cb_prepared_xacts_ok 0 0 && echo ok || echo refused)" "ok"
+
+# The message is the deliverable here: the operator has to know WHAT to set and
+# that it costs a restart. A bare "failed" sends them to the load log, which
+# reports a per-worker error and not the cause.
+msg="$(cb_prepared_xacts_message 0 8)"
+check "the message names the setting to change" \
+	"$([ "$(grep -c 'max_prepared_transactions' <<<"$msg")" -ge 1 ] && echo yes || echo no)" "yes"
+check "and the number it must reach" \
+	"$([ "$(grep -c '8' <<<"$msg")" -ge 1 ] && echo yes || echo no)" "yes"
+check "and says it needs a restart, which is why this runs first" \
+	"$([ "$(grep -ci 'restart' <<<"$msg")" -ge 1 ] && echo yes || echo no)" "yes"
+
+# ---- a load that lost rows is a failure, not a fast result ------------------
+check "a short load is refused" \
+	"$(cb_rows_ok 1999999 2000000 && echo ok || echo refused)" "refused"
+check "an empty load is refused, which is what an errored parallel arm produces" \
+	"$(cb_rows_ok 0 2000000 && echo ok || echo refused)" "refused"
+check "an exact load is accepted" \
+	"$(cb_rows_ok 2000000 2000000 && echo ok || echo refused)" "ok"
+# Empty is not zero. A psql that failed produces neither.
+#
+# BOTH sides empty is the case that matters, and it is the only one of the three
+# that a plain `[ "$got" = "$want" ]` gets wrong: one empty side is unequal to a
+# number and is refused either way. The first version of this suite asserted only
+# the one-sided cases, and a removal proof showed they passed with the numeric
+# check deleted, which means they were testing nothing. That is the trap #418
+# exists to forbid, met while writing the test for it.
+check "two missing measurements are refused, not called equal" \
+	"$(cb_rows_ok '' '' && echo ok || echo refused)" "refused"
+check "a missing count is refused rather than compared with the expectation" \
+	"$(cb_rows_ok '' 2000000 && echo ok || echo refused)" "refused"
+check "and a missing expectation is refused too" \
+	"$(cb_rows_ok 2000000 '' && echo ok || echo refused)" "refused"
+
+echo
+echo "checks run: $PGC_CHECKS"
+if [ "$PGC_FAIL" != 0 ]; then
+	echo "$(basename "$0"): FAILED"
+	exit 1
+fi
+echo "$(basename "$0"): PASSED"
+exit 0
diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh
@@ -48,6 +48,7 @@ SUITES=(
 	arrow_nested
 	arrow_nested_import
 	audit
+	bench_guards
 	bloom_lazy
 	bloom_setting
 	bloom_sizing