From 442fe2656021bae2e1c6d4d65794da4408a931f5 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 12 Oct 2026 20:14:32 +0000 Subject: [PATCH 1/2] test: an alpha4 binary refuses what this build wrote; min/max(ts) read no data Two promises made today had no test of their own. #1137 (format_downgrade.sh, test_format_downgrade.py). The stamp's claim is about two binaries, and native_format.sh stands in for alpha4 with a catalog UPDATE. These build v1.0-alpha4, write with it, upgrade to this tree, read one table, append to another, create a third, then put the alpha4 library back under the upgraded catalog: alpha4 reads the table this build only read count, sum, index fetch alpha4 refuses the appended and the new table 0A000, scan and fetch stamp set back to 1 stamps "1 1 1", 5 arms red; alpha4 answers "ok" on an index fetch of a chunk it cannot decode The shell suite is opt-in under PGC_RUN_UPGRADE=1 and invoked there, not only exempted (#396); not_a_suite lists it in selftest 040 and run_coverage.sh. The pytest twin runs in CI: it fetches the tag on a shallow checkout and restores every pgcolumnar* file in the prefix by bytes and mtime, by atomic rename. #1293 (native_agg.sh, test_native_agg.py). The suite proved the metadata node is chosen and the answer matches heap; a build that kept the node and decoded every group anyway passed it. With every group forced dirty, max(ts) read 414 blocks against a decode's 403 and every existing arm stayed green; the new arms go red. They also check parity after deleting the newest and oldest rows. native_agg 35 passed (PG 16, 17) format_downgrade 14 passed (PG 16, 17) pytest twins passed; each red under its mutation harness_selftest 1213 passed; guard half 404; parity 14/14 and 10 + INCOMPLETE Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 24 ++ docs/testing.md | 26 ++ test/format_downgrade.sh | 248 ++++++++++++++++++ test/native_agg.sh | 66 +++++ test/pytest/TESTS.md | 38 +++ test/pytest/expected_tests.txt | 2 +- test/pytest/test_compare_to_bash.py | 7 + test/pytest/test_format_downgrade.py | 202 ++++++++++++++ test/pytest/test_native_agg.py | 114 ++++++++ test/run_all_versions.sh | 29 ++ test/run_coverage.sh | 2 +- .../040-the-detection-primitive-itself.sh | 2 +- 12 files changed, 757 insertions(+), 3 deletions(-) create mode 100755 test/format_downgrade.sh create mode 100644 test/pytest/test_format_downgrade.py create mode 100644 test/pytest/test_native_agg.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bb469509f..073aab09a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,30 @@ true until the next version shipped. ## [Unreleased] +### Added + +- `test/format_downgrade.sh` and `test_format_downgrade.py` test #1137's promise + with both real libraries. They build `v1.0-alpha4`, write with it, upgrade to the + tree under test, write again, and then put the alpha4 library back under the + upgraded catalog. alpha4 must still read a table this build only read, and must + refuse with `0A000` a table this build appended to or created, on a scan and on + an index fetch. With the stamp set back to 1, the stamps read `1 1 1` and five + arms go red; alpha4 then answers `ok` on an index fetch it cannot decode. The + shell suite runs under `PGC_RUN_UPGRADE=1`, like `extension_upgrade.sh`. The + pytest twin runs in CI on every major. It fetches the tag when the checkout is + shallow, and it puts the install prefix back exactly: same files, same bytes, + same mtimes. + +- `native_agg.sh` and a new `test_native_agg.py` test that `max(ts)` and `min(ts)` + over `timestamptz` read no data (#1293). The suite proved that the metadata node + is chosen and that the answer matches heap, but a build that kept the node and + decoded every group anyway passed it. With every group forced dirty, `max(ts)` + read 414 blocks against a decode's 403, and every existing arm stayed green. The + new arms compare the table's own blocks with a decode of the column, and check + parity again after the newest and oldest rows are deleted. The first draft of + the fixture overflowed `int`, its `INSERT` failed without a word, and every arm + measured an empty table. A row-count premise now comes first. + ### Changed - Five selftest parts and `rewrite_storage_oid` no longer hold a check that has diff --git a/docs/testing.md b/docs/testing.md index 2c95f8594..32d9f1fea 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -423,6 +423,32 @@ PGC_RUN_UPGRADE=1 PGC_UPGRADE_OLD_SRC=/root/pgcolumnar-1.0-alpha \ test/run_all_versions.sh ``` +## Format downgrade + +`PGC_RUN_UPGRADE=1` also runs `test/format_downgrade.sh`, once, against the first +major in the list. It checks the other direction: what an older build does with a +table that this build has written to. + +From 1.0-alpha5, a table that this build writes carries format version 2, and +1.0-alpha4 refuses any version except 1. A table that this build only reads keeps +version 1. The suite tests that promise with both real libraries. It installs +`v1.0-alpha4` and writes two tables with it. It upgrades to the tree under test, +reads one table, appends to the other and creates a third. Then it puts the +alpha4 library back under the upgraded catalog. The old library must still read +the table that was only read. It must refuse the other two with SQLSTATE `0A000`, +on a scan and on an index fetch. + +It finds the old source the same way as the extension upgrade suite, from a ref or a +directory. Pass the directory by environment through the runner: + +```sh +PGC_RUN_UPGRADE=1 PGC_DOWNGRADE_OLD_SRC=/root/pgcolumnar-1.0-alpha4 \ + test/run_all_versions.sh +``` + +The suite puts the tree's own library back when it exits, so a suite that runs after +it never finds the old one installed. + Without it, in a tree that has no `.git`, the suite reports **SKIP** and the matrix records `SKIP`. It does not report a pass, because a gate that goes green having run nothing is the defect this suite exists to catch. It does not report a failure either. Being unable to diff --git a/test/format_downgrade.sh b/test/format_downgrade.sh new file mode 100755 index 000000000..767b0aedb --- /dev/null +++ b/test/format_downgrade.sh @@ -0,0 +1,248 @@ +#!/usr/bin/env bash +# +# pgColumnar format-downgrade gate (#1137). +# +# 1.0-alpha5 stops writing a validity bitmap for a chunk with no nulls (#1130). An +# alpha4 binary misreads such a chunk without refusing it: measured with the real +# v1.0-alpha4 library, 9 of 40 single-row fetches returned NULL for a row that holds +# a value. #1137 answered that with the stamp: a table this build WRITES carries +# pgcolumnar.storage.format_version 2, alpha4 refuses any stamp but 1, and a table +# this build only READS keeps 1 and stays readable by alpha4. +# +# THAT CLAIM IS ABOUT TWO BINARIES, so only a test with both can hold it. The arms in +# native_format.sh stand in for alpha4 with a catalog UPDATE; this suite installs the +# real old library, writes with it, upgrades, and then puts the OLD library back +# under the upgraded catalog -- which is exactly what a user who rolls back the +# package does. +# +# t_read written by alpha4, only read by this build alpha4 still reads it +# t_append written by alpha4, appended to by this build alpha4 refuses it +# t_new written by this build alpha4 refuses it +# +# Refusal is asserted by SQLSTATE (0A000), on a sequential scan AND on an index fetch +# with the custom scan off, which is the path that returned the silent NULLs. +# +# Not in run_all_versions.sh, for extension_upgrade.sh's reason: it builds an old ref. +# +# test/format_downgrade.sh [PG_CONFIG] [OLD_REF_OR_DIR] default old ref: v1.0-alpha4 +# +# The old source is resolved exactly as extension_upgrade.sh resolves it. + +set -uo pipefail +PG_CONFIG=${1:-pg_config} +OLD_SRC=${2:-${PGC_DOWNGRADE_OLD_SRC:-v1.0-alpha4}} +SRCDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +. "$(dirname "${BASH_SOURCE[0]}")/portlib.sh" +# lib.sh for its recorders and accounting only, the way capability_sweep.sh uses +# it: this suite stands up its own cluster (it must swap the library under it), so +# pgc_setup and its EXIT trap are not used. +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +command -v "$PG_CONFIG" >/dev/null 2>&1 || { echo "FATAL: $PG_CONFIG not found"; exit 1; } +BINDIR=$($PG_CONFIG --bindir) +PKGLIB=$($PG_CONFIG --pkglibdir) +PGMAJ=$($PG_CONFIG --version | grep -oE '[0-9]+' | head -1) +PGC_MAJOR="$(pgc_major_of "$PG_CONFIG")" +TMP=$(mktemp -d /tmp/pgc-fmtdown.XXXXXX) +DATA=$TMP/data +PORT=$(pgc_pick_free_port "$PGC_AUX_PORT_LO" "$PGC_AUX_PORT_HI" "$$") +LOG=$TMP/server.log +fail=0 +cleanup () { + runuser -u postgres -- "$BINDIR/pg_ctl" -D "$DATA" -w stop >/dev/null 2>&1 + # LEAVE THE PREFIX WITH THE TREE'S OWN LIBRARY. This suite puts the old one in + # place on purpose; a suite run after it must not find it there. + [ -f "$TMP/new.so" ] && cp "$TMP/new.so" "$PKGLIB/pgcolumnar.so" + rm -rf "$TMP" +} +trap cleanup EXIT +runpg () { runuser -u postgres -- "$@"; } +q () { runpg "$BINDIR/psql" -h /tmp -p "$PORT" -d fmtdown -X -Atc "$1" 2>&1; } +# sqlstate SQL -> the SQLSTATE it fails with, or ok +sqlstate () { + local out + out=$(runpg "$BINDIR/psql" -h /tmp -p "$PORT" -d fmtdown -X -At \ + -v ON_ERROR_STOP=1 -v VERBOSITY=sqlstate -c "$1" 2>&1 >/dev/null) + [ -z "$out" ] && { echo ok; return; } + echo "$out" | sed -n 's/^ERROR: *\([0-9A-Z]\{5\}\).*/\1/p' | head -1 +} +echo "== format_downgrade: PG$PGMAJ, old $OLD_SRC" +EXPLICIT=0 +{ [ "$#" -ge 2 ] || [ -n "${PGC_DOWNGRADE_OLD_SRC:-}" ]; } && EXPLICIT=1 + +# ASK GIT WHETHER IT CAN ANSWER, do not stat .git (#1224). A linked worktree's +# .git is a regular FILE holding `gitdir: `, and so is the one-line gitfile +# devloop.sh writes into its build dir and that an agent staging across a +# container boundary has to write. git works in all three; `[ -d .git ]` is false +# in all three, so this suite skipped its default path in environments selftest +# 310 lists as supported -- "the audit container, worktrees of a clone" among +# them. devloop.sh already had the right form and the reason beside it. +# +# `git clone --shared` from a linked worktree is measured to work, and old refs +# are reachable through it, so this is the whole fix rather than a step of it. +# AND IT MUST BE THIS TREE'S REPOSITORY, NOT AN ENCLOSING ONE (#1228). +# rev-parse searches UPWARD, so a copy with no git presence of its own that +# happens to sit inside any repository answers yes. `git clone --shared` then +# takes the path literally rather than walking up and dies with +# `fatal: repository does not exist`, so the cost was a SKIP carrying +# instructions becoming a FATAL about a missing repository -- a diagnosis +# regression rather than a wrong result, measured by @OffgridwithJD. +# +# --show-toplevel separates the three, measured: a linked worktree and a +# gitfile-staged copy both report THEMSELVES, and a directory nested in a +# repository reports the enclosing root. +pgc_eu_own_repo() { # pgc_eu_own_repo DIR -> 0 when DIR is its own work tree + local _top + _top="$(git -C "$1" rev-parse --show-toplevel 2>/dev/null)" || return 1 + [ -n "$_top" ] && [ "$_top" = "$(cd "$1" 2>/dev/null && pwd -P)" ] +} + +if [ "$EXPLICIT" = 0 ] && ! pgc_eu_own_repo "$SRCDIR"; then + # WHICH of the two it is, rather than a disjunction the reader has to + # resolve themselves. They are different situations with different repairs, + # and the enclosing root is the fact that makes the second one legible. + _eu_encl="$(git -C "$SRCDIR" rev-parse --show-toplevel 2>/dev/null || true)" + echo " SKIP $SRCDIR is not its own git work tree, so the default ref" \ + "$OLD_SRC cannot be built." + if [ -n "$_eu_encl" ]; then + echo " It sits inside the repository at $_eu_encl, which is a" + echo " different tree and does not carry this one's history." + else + echo " It has no git presence of its own and none above it." + fi + echo " Supply the old source:" + echo " PGC_DOWNGRADE_OLD_SRC=/path/to/old/source, or pass it as the second argument." + echo "== extension_upgrade: SKIP" + exit 2 +fi + +# A value that looks like a path but is not there is a wrong path, not a ref. Falling +# through to the ref branch would answer a question about directories with an error about +# git checkouts, which is the kind of message that costs someone an afternoon. +case "$OLD_SRC" in + */*) + if [ ! -d "$OLD_SRC" ]; then + echo " FAIL $OLD_SRC looks like a path and is not a directory." + echo " Give a checked-out source tree of the previous release." + exit 1 + fi + ;; +esac + +# Directory form: a path to an already-checked-out old tree needs no git at all. +if [ -d "$OLD_SRC" ]; then + [ -f "$OLD_SRC/Makefile" ] || { echo " FAIL $OLD_SRC has no Makefile"; exit 1; } + cp -a "$OLD_SRC" "$TMP/old" || { echo "FATAL: could not copy $OLD_SRC"; exit 1; } + echo " old source: directory $OLD_SRC" +else + # Only reachable when an old source was named explicitly, so this is a failure and + # not a skip: the caller asked for something this tree cannot provide. + if ! pgc_eu_own_repo "$SRCDIR"; then + _eu_encl="$(git -C "$SRCDIR" rev-parse --show-toplevel 2>/dev/null || true)" + echo " FAIL $SRCDIR is not its own git work tree, so the ref $OLD_SRC" \ + "cannot be built." + if [ -n "$_eu_encl" ]; then + echo " It sits inside the repository at $_eu_encl, which is a" + echo " different tree and does not carry this one's history." + else + echo " It has no git presence of its own and none above it." + fi + echo " Pass a directory:" + echo " test/extension_upgrade.sh $PG_CONFIG /path/to/old/source" + exit 1 + fi + if ! git -C "$SRCDIR" rev-parse --verify -q "$OLD_SRC^{commit}" >/dev/null 2>&1; then + echo " FAIL $OLD_SRC is not present. Fetch tags, or pass an explicit ref or dir:" + echo " git fetch --tags && test/extension_upgrade.sh $PG_CONFIG " + exit 1 + fi + git clone -q --shared "$SRCDIR" "$TMP/old" || { echo "FATAL: clone failed"; exit 1; } + git -C "$TMP/old" checkout -q --detach "$OLD_SRC" \ + || { echo "FATAL: checkout $OLD_SRC failed"; exit 1; } + echo " old source: ref $OLD_SRC" +fi +make -C "$TMP/old" PG_CONFIG="$PG_CONFIG" clean >/dev/null 2>&1 +make -C "$TMP/old" PG_CONFIG="$PG_CONFIG" -j"$(nproc)" >"$TMP/build_old.log" 2>&1 +old_rc=$? +check_num "premise: the old build compiles" "$old_rc" "0" +[ "$old_rc" = 0 ] || { tail -20 "$TMP/build_old.log"; pgc_summary; } +make -C "$TMP/old" PG_CONFIG="$PG_CONFIG" install >/dev/null 2>&1 \ + || { echo "FAIL old install"; exit 1; } +cp "$PKGLIB/pgcolumnar.so" "$TMP/old.so" + +chown -R postgres "$TMP" 2>/dev/null +runpg "$BINDIR/initdb" -D "$DATA" --locale=C -U postgres >/dev/null 2>&1 \ + || { echo "FAIL initdb"; exit 1; } +{ + echo "shared_preload_libraries='pgcolumnar'" + echo "port=$PORT" + echo "unix_socket_directories='/tmp'" # extension_upgrade.sh says why +} >> "$DATA/postgresql.conf" +runpg "$BINDIR/pg_ctl" -D "$DATA" -l "$LOG" -w start >/dev/null 2>&1 \ + || { echo "FAIL start"; tail -10 "$LOG"; exit 1; } +runpg "$BINDIR/createdb" -h /tmp -p "$PORT" fmtdown >/dev/null 2>&1 + +# ---- 1. the old build writes two tables ------------------------------------------- +# Wide text with no nulls, compression off: the shape whose bitmap #1130 elides, and +# the shape #1137 measured the silent NULLs on. +q "CREATE EXTENSION pgcolumnar" >/dev/null +for t in t_read t_append; do + q "CREATE TABLE $t (id int, v text) USING pgcolumnar" >/dev/null + q "SELECT pgcolumnar.set_options('$t', compression => 'none')" >/dev/null + q "INSERT INTO $t SELECT g, repeat(md5(g::text), 6) FROM generate_series(1, 3000) g" >/dev/null + q "CREATE INDEX ON $t (id)" >/dev/null +done +check "premise: the old build stored both tables" \ + "$(q "SELECT count(*) FROM t_read")|$(q "SELECT count(*) FROM t_append")" "3000|3000" + +# ---- 2. this build: upgrade, read one, append to one, create one ------------------ +make -C "$SRCDIR" PG_CONFIG="$PG_CONFIG" clean >/dev/null 2>&1 +make -C "$SRCDIR" PG_CONFIG="$PG_CONFIG" -j"$(nproc)" >"$TMP/build_new.log" 2>&1 \ + || { echo "FAIL new build"; tail -20 "$TMP/build_new.log"; exit 1; } +make -C "$SRCDIR" PG_CONFIG="$PG_CONFIG" install >/dev/null 2>&1 \ + || { echo "FAIL new install"; exit 1; } +cp "$PKGLIB/pgcolumnar.so" "$TMP/new.so" +# What the cleanup trap puts back. Counted here because a restore of nothing is +# the silent way this suite could leave the prefix holding the old library. +check_num "premise: the prefix holds the tree's own install to put back" \ + "$(n=$(ls "$PKGLIB"/pgcolumnar*.so 2>/dev/null | grep -c .); [ "$n" -lt 2 ] && echo "$n" || echo 2)" "2" +check "premise: the two libraries differ" \ + "$(cmp -s "$TMP/old.so" "$TMP/new.so" && echo same || echo differ)" "differ" +runpg "$BINDIR/pg_ctl" -D "$DATA" -l "$LOG" -w restart >/dev/null 2>&1 \ + || { echo "FAIL restart on the new build"; tail -10 "$LOG"; exit 1; } +upd=$(q "ALTER EXTENSION pgcolumnar UPDATE") +check "ALTER EXTENSION UPDATE succeeds" "$(case "$upd" in *ERROR*) echo "$upd";; *) echo ok;; esac)" "ok" + +stamp () { q "SELECT format_version FROM pgcolumnar.storage WHERE storage_id = pgcolumnar.get_storage_id('$1')"; } +check "this build reads the table the old build wrote" \ + "$(q "SELECT count(*), sum(id) FROM t_read")" "3000|4501500" +q "INSERT INTO t_append SELECT g, repeat(md5(g::text), 6) FROM generate_series(3001, 3100) g" >/dev/null +q "CREATE TABLE t_new (id int, v text) USING pgcolumnar" >/dev/null +q "SELECT pgcolumnar.set_options('t_new', compression => 'none')" >/dev/null +q "INSERT INTO t_new SELECT g, repeat(md5(g::text), 6) FROM generate_series(1, 3000) g" >/dev/null +q "CREATE INDEX ON t_new (id)" >/dev/null +check "stamps after this build: read, appended, new" \ + "$(stamp t_read) $(stamp t_append) $(stamp t_new)" "1 2 2" + +# ---- 3. roll the library back, under the upgraded catalog -------------------------- +runpg "$BINDIR/pg_ctl" -D "$DATA" -w stop >/dev/null 2>&1 +cp "$TMP/old.so" "$PKGLIB/pgcolumnar.so" +runpg "$BINDIR/pg_ctl" -D "$DATA" -l "$LOG" -w start >/dev/null 2>&1 \ + || { echo "FAIL start on the old library"; tail -10 "$LOG"; exit 1; } +check "premise: the server now runs the old library" \ + "$(cmp -s "$PKGLIB/pgcolumnar.so" "$TMP/old.so" && echo old || echo other)" "old" + +fetch () { # fetch TABLE -> SQLSTATE of an index fetch of a non-key column + sqlstate "SET pgcolumnar.enable_custom_scan = off; SET enable_seqscan = off; + SET enable_bitmapscan = off; SELECT v FROM $1 WHERE id = 1234" +} +check "the old build still reads a table this build only read" \ + "$(q "SELECT count(*), sum(id) FROM t_read")" "3000|4501500" +check "and fetches from it by index" "$(fetch t_read)" "ok" +check "the old build refuses a table this build appended to, on a scan" \ + "$(sqlstate "SELECT count(v) FROM t_append")" "0A000" +check "the old build refuses a table this build appended to, on an index fetch" "$(fetch t_append)" "0A000" +check "the old build refuses a table this build wrote, on a scan" \ + "$(sqlstate "SELECT count(v) FROM t_new")" "0A000" +check "the old build refuses a table this build wrote, on an index fetch" "$(fetch t_new)" "0A000" + +pgc_summary diff --git a/test/native_agg.sh b/test/native_agg.sh index c68ad227a..4aff4bca7 100755 --- a/test/native_agg.sh +++ b/test/native_agg.sh @@ -39,6 +39,10 @@ plan() { # no separate Aggregate node above a scan; a fallback plan has an Aggregate node # and no such line. has_aggnode() { grep -q 'Columnar Vectorized Aggregates' <<<"$1" && echo yes || echo no; } +# at_least GOT FLOOR -- GOT when it falls short of FLOOR, else FLOOR, so a failing +# arm reports the number it measured (selftest part 540). An empty GOT stays +# empty, which check_num refuses as "not a measurement" rather than reading as 0. +at_least() { [ -z "$1" ] && return; [ "$1" -lt "$2" ] && echo "$1" || echo "$2"; } has_gather() { grep -q 'Gather' <<<"$1" && echo yes || echo no; } # Same, with a parallel plan available: workers allowed, no setup charge, and no @@ -134,4 +138,66 @@ check "empty count(*)" "$(q 'SELECT count(*) FROM e;')" "0" check "empty sum NULL" "$(q 'SELECT sum(id) IS NULL FROM e;')" "t" check "empty min NULL" "$(q 'SELECT min(label) IS NULL FROM e;')" "t" +# ---- min/max over timestamptz read no data, and stay right after deletes (#1293) +# +# "What is the latest reading?" -- max(ts) -- is the time-series question, and +# the arms above cover min/max only on int4 and text. They also prove the +# metadata node is CHOSEN and that the answer matches heap; neither shows the +# data was not read. A build that kept the node and decoded every group anyway +# would return the same answers and pass them all. +# +# So these arms read the TABLE's own blocks, from pg_statio, flushed by the +# querying session, against a control that must decode the same column. +psql_run "CREATE TABLE tz (dev int, ts timestamptz) USING pgcolumnar;" +psql_run "SELECT pgcolumnar.set_options('tz', stripe_row_limit => 1000);" +psql_run "INSERT INTO tz SELECT g % 50, '2026-03-01'::timestamptz + ((g::bigint * 7919) % 400000) * interval '1 min' + FROM generate_series(1, 400000) g;" +psql_run "CREATE TABLE tzh AS SELECT * FROM tz;" +# THE ROW COUNT FIRST. The first draft computed g * 7919 in int, which overflows +# past g = 271,000; the INSERT failed, psql_run reported nothing, and every arm +# below measured an empty table -- the decode "control" read 3 blocks. +check_num "premise: the timestamptz table holds every row inserted" \ + "$(q 'SELECT count(*) FROM tz;')" "400000" + +tz_blocks() { # tz_blocks QUERY -> blocks of tz read while QUERY ran, in its own session + q "SELECT pg_stat_reset();" >/dev/null + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -At -c "$1" -c "SELECT pg_stat_force_next_flush()" >/dev/null 2>&1 + q "SELECT coalesce(heap_blks_read, 0) + coalesce(heap_blks_hit, 0) + FROM pg_statio_user_tables WHERE relname = 'tz';" +} +tz_decode="$(tz_blocks "SELECT max(ts + interval '0 s') FROM tz")" +tz_max="$(tz_blocks "SELECT max(ts) FROM tz")" +tz_min="$(tz_blocks "SELECT min(ts) FROM tz")" +echo "-- tz blocks read: decode=$tz_decode max=$tz_max min=$tz_min" + +check "premise: max(ts) over timestamptz plans the metadata agg node" \ + "$(has_aggnode "$(plan 'SELECT max(ts) FROM tz')")" "yes" +# THE CONTROL. A decode of ts has to show up, or every comparison below is +# against an instrument that cannot see a read. +check_num "control: decoding ts reads the table's blocks" \ + "$(at_least "$tz_decode" 10)" "10" +# A tenth, not zero: the metapage and its neighbours are read either way (15 +# blocks against 6,603 on a 10M-row table, measured on #1293). The margin is +# reported, not collapsed to a yes or no. +# The margin is decode - 10 * reading: positive passes, and a failure reports by +# how much it fell short. +check_num "max(ts) reads under a tenth of the blocks a decode of ts reads" \ + "$( [ -n "$tz_max" ] && at_least $((tz_decode - 10 * tz_max)) 1 )" "1" +check_num "min(ts) reads under a tenth of the blocks a decode of ts reads" \ + "$( [ -n "$tz_min" ] && at_least $((tz_decode - 10 * tz_min)) 1 )" "1" + +check "max(timestamptz) parity" "$(q 'SELECT max(ts) FROM tz;')" "$(q 'SELECT max(ts) FROM tzh;')" +check "min(timestamptz) parity" "$(q 'SELECT min(ts) FROM tz;')" "$(q 'SELECT min(ts) FROM tzh;')" + +# The newest and oldest readings deleted: a zone map still records them, so a +# group with deletes must not be folded from it (#149). +psql_run "DELETE FROM tz WHERE ts > (SELECT max(ts) FROM tzh) - interval '30 min' OR ts < (SELECT min(ts) FROM tzh) + interval '30 min';" +psql_run "DELETE FROM tzh WHERE ts > (SELECT max(ts) FROM tzh) - interval '30 min' OR ts < (SELECT min(ts) FROM tzh) + interval '30 min';" +check "premise: the deletes removed rows at both ends, the same rows from each" \ + "$(q 'SELECT count(*) < 400000 FROM tz;')|$(q 'SELECT count(*) FROM tz;')" \ + "t|$(q 'SELECT count(*) FROM tzh;')" +check "max(timestamptz) parity after deleting the newest rows" "$(q 'SELECT max(ts) FROM tz;')" "$(q 'SELECT max(ts) FROM tzh;')" +check "min(timestamptz) parity after deleting the oldest rows" "$(q 'SELECT min(ts) FROM tz;')" "$(q 'SELECT min(ts) FROM tzh;')" + pgc_summary diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 8eb2e1602..adf3baadc 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -130,6 +130,8 @@ behaviour, the source of that number is named. - [82. test_rewrite_storage_oid.py: a type rewrite keeps the storage row on the live table](#82-test_rewrite_storage_oidpy-a-type-rewrite-keeps-the-storage-row-on-the-live-table) - [83. test_catalog_scan_index.py: twelve metadata scans do not read their catalog whole](#83-test_catalog_scan_indexpy-twelve-metadata-scans-do-not-read-their-catalog-whole) - [84. test_native_format.py: the format stamp is a range, and an append raises it](#84-test_native_formatpy-the-format-stamp-is-a-range-and-an-append-raises-it) +- [85. test_native_agg.py: min/max over timestamptz read no data](#85-test_native_aggpy-minmax-over-timestamptz-read-no-data) +- [86. test_format_downgrade.py: an alpha4 binary refuses what this build wrote](#86-test_format_downgradepy-an-alpha4-binary-refuses-what-this-build-wrote) ## 1. How to read a test in here @@ -6344,3 +6346,39 @@ Not ported, and declared INCOMPLETE in `test_compare_to_bash.py`: the metapage-v | test | what it holds | | --- | --- | | `test_the_format_stamp_is_a_range_and_an_append_raises_it` | this build stamps 2; a table stamped 1 is read on all four entry points and returns its rows; reading leaves its stamp at 1 and appending raises it to 2; stamps 3 and 0 are refused with `0A000` on all four; the backend survives | + +## 85. test_native_agg.py: min/max over timestamptz read no data + +A partial port of `native_agg.sh`, covering the arms #1293 added. "What is the latest reading?" is `max(ts)`, and an ungrouped, unfiltered `min`/`max` is answered from the row groups' zone maps. The suite already proved the metadata node is chosen and that the answer matches heap, on `int4` and `text`. Neither shows the data went unread: with every group forced dirty, the node is still chosen, every parity arm stays green, and `max(ts)` reads 414 blocks against a decode's 403 (shell twin). + +THE ARM READS THE TABLE'S OWN BLOCKS from `pg_statio`, against a control that must decode the column (`max(at + interval '0 s')`). A tenth rather than zero, because the metapage and its neighbours are read either way: 13 blocks here, and 15 on a 10M-row table measured on #1293. The margin `decode - 10 * reading` is what is compared, so a failure reports by how much it fell short. + +THE ROW COUNT IS A PREMISE because the shell twin's first draft overflowed `int` computing its timestamps. The `INSERT` failed without a word, and every arm measured an empty table: the decode "control" read 3 blocks. Parity is then checked again after deleting the newest and oldest rows, because a zone map still records deleted values (#149). + +Removal proof, on this twin: forcing every group dirty reddens `max(ts) reads under a tenth of the blocks a decode of ts reads` (got -1487, wanted at least 1). + +Not ported, and declared INCOMPLETE in `test_compare_to_bash.py`: the `int4`/`text` parity, `sum`/`avg`, parallel-plan and fallback arms. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_min_max_over_timestamptz_read_no_data` | the table holds every row; `max(ts)` plans the metadata node; a decode reads at least 10 blocks; `max` and `min` each read under a tenth of that; parity with heap before and after deleting rows at both ends | + +## 86. test_format_downgrade.py: an alpha4 binary refuses what this build wrote + +Port of `format_downgrade.sh` (#1137). 1.0-alpha5 stops writing a validity bitmap for a chunk with no nulls, and an alpha4 binary misreads such a chunk without refusing it: with the real `v1.0-alpha4` library, 9 of 40 single-row fetches returned NULL for a row that holds a value. The fix is the format stamp. A table this build writes carries `format_version` 2, alpha4 refuses any stamp but 1, and a table this build only reads keeps 1. + +THE CLAIM IS ABOUT TWO BINARIES, so this test uses both. It builds `v1.0-alpha4` from the tag, which it fetches when the checkout is shallow, as CI's is. It writes two tables with the old build, upgrades to this tree, reads one, appends to the other and creates a third. Then it puts the old library back under the upgraded catalog, which is what a package rollback does. `native_format.sh`'s arms stand in for alpha4 with a catalog `UPDATE`; this one does not. + +ON A CLUSTER OF ITS OWN, AND THE PREFIX IS PUT BACK EXACTLY. The session cluster shares the install prefix. So before anything is installed, every `pgcolumnar*` file in the library and extension directories is saved, bytes and mtime, and restored in a `finally` by atomic rename. Anything the old install added is removed. The mtime matters because the harness compares the library's mtime with the postmaster's start. Nothing is written into a library in place, because a copy over a library a running postmaster has mapped corrupts it. Measured: the prefix's library afterwards is byte-identical to the one this tree built. + +Refusal is asserted by SQLSTATE `0A000`, on a scan and on an index fetch with the custom scan off. The index fetch is the path that returned the silent NULLs. + +Removal proof: with `COLUMNAR_NATIVE_FORMAT_VERSION` set back to 1, the stamps read `1 1 1` and the test fails there. On the shell twin the same mutation reddens five arms, and alpha4 answers `ok` on the appended table's index fetch. That `ok` is the silent read this test exists to refuse. + +### Every test + +| test | what it holds | +| --- | --- | +| `test_an_alpha4_binary_refuses_what_this_build_wrote` | the old build stores both tables; the libraries differ; the upgrade succeeds and this build reads the old table; the stamps are `1 2 2` for read, appended and new; on the old library again, the table only read is still read and fetched by index, and the appended and new tables are refused with `0A000` on a scan and on an index fetch | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 896235699..6fbf68788 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -543,4 +543,4 @@ guard_tests 404 # own test is not in it. Git conflicted again -- loudly, because the key moved # on both sides -- and the answer is neither branch's number. Collected on the # rebased tree, not added up. -cluster_tests 491 +cluster_tests 493 diff --git a/test/pytest/test_compare_to_bash.py b/test/pytest/test_compare_to_bash.py index 3203c3918..a3fe4bc65 100644 --- a/test/pytest/test_compare_to_bash.py +++ b/test/pytest/test_compare_to_bash.py @@ -95,6 +95,7 @@ "catalog_scan_index", "differential", "encode_post_codec", + "format_downgrade", "hilbert_cluster", "hilbert_locality", "iceberg_fdw", @@ -135,6 +136,12 @@ # about the PORT rather than a licence: the standing arm does not grade it, so the # reason is the only thing standing between a declared gap and a forgotten one. INCOMPLETE = { + # #1293 ported the arms it added: min/max over timestamptz read no data, and + # stay right after deletes. The int4/text parity, sum/avg, parallel-plan and + # fallback arms of the shell suite predate it and are #432's to port. + "native_agg": "the timestamptz min/max arms (blocks read against a decode " + "control, parity before and after deletes) are ported; the " + "int4/text parity, sum/avg, parallel-plan and fallback arms are not", # The port asserts the engine counters, the heap oracle, the scattered zero # and the declared-collation property. The shell suite additionally pins the # three states of max_upper and its own fixture geometry by name, and those diff --git a/test/pytest/test_format_downgrade.py b/test/pytest/test_format_downgrade.py new file mode 100644 index 000000000..82b79029a --- /dev/null +++ b/test/pytest/test_format_downgrade.py @@ -0,0 +1,202 @@ +"""An alpha4 binary refuses a table this build wrote, and still reads one it only read (#1137). + +1.0-alpha5 stops writing a validity bitmap for a chunk with no nulls (#1130), and an +alpha4 binary misreads such a chunk without refusing it: with the real v1.0-alpha4 +library, 9 of 40 single-row fetches returned NULL for a row that holds a value. The +fix is the stamp. A table this build WRITES carries format_version 2, alpha4 refuses +any stamp but 1, and a table this build only READS keeps 1. + +THE CLAIM IS ABOUT TWO BINARIES, so this test uses both: it builds v1.0-alpha4, +writes with it, upgrades to this tree, writes again, then puts the old library back +under the upgraded catalog -- which is what a package rollback does. Refusal is +asserted by SQLSTATE (0A000) on a scan and on an index fetch with the custom scan +off, the path that returned the silent NULLs. + +ON A CLUSTER OF ITS OWN, and the prefix is put back exactly. The session cluster +shares the install prefix, so every `pgcolumnar*` file in the library and extension +directories is saved first -- bytes and mtime -- and restored in a `finally` by +atomic rename, with anything the old install added removed. The mtime matters: the +harness compares the library's mtime against the postmaster's start, and a newer +library would make later files refuse to run. A copy written over a library a +running postmaster has mapped corrupts it, so nothing is written in place. + +Independent of `test/format_downgrade.sh`: its own tables, row counts and keys, and +it restores the prefix by snapshot where the shell suite reinstalls. +""" + +import os +import pathlib +import shutil +import subprocess +import tempfile + +import psycopg + +OLD_REF = "v1.0-alpha4" +ROWS = 2500 +# sum(1..2500) +FULL_SUM = ROWS * (ROWS + 1) // 2 +SRCDIR = pathlib.Path(__file__).resolve().parent.parent.parent + + +def _run(argv, **kw): + return subprocess.run(argv, check=True, capture_output=True, text=True, **kw) + + +def _snapshot(dirs): + """{path: (bytes, (atime, mtime))} for every pgcolumnar* file in `dirs`.""" + snap = {} + for d in dirs: + for p in pathlib.Path(d).glob("pgcolumnar*"): + if p.is_file(): + st = p.stat() + snap[p] = (p.read_bytes(), (st.st_atime, st.st_mtime)) + return snap + + +def _restore(snap, dirs): + """Put the prefix back exactly: same files, same bytes, same mtimes.""" + for d in dirs: + for p in pathlib.Path(d).glob("pgcolumnar*"): + if p.is_file() and p not in snap: + p.unlink() + for p, (data, times) in snap.items(): + tmp = p.with_name(p.name + ".pgc-restore") + tmp.write_bytes(data) + os.utime(tmp, times) + os.replace(tmp, p) + + +def _place(src, dst): + """Install one file by atomic rename, never by writing into a mapped one.""" + tmp = pathlib.Path(str(dst) + ".pgc-place") + shutil.copyfile(src, tmp) + os.replace(tmp, dst) + + +def _sqlstate(dsn, statements): + try: + with psycopg.connect(dsn, autocommit=True) as c: + for s in statements: + c.execute(s) + return "ok" + except psycopg.Error as e: + return e.sqlstate or "?" + + +def test_an_alpha4_binary_refuses_what_this_build_wrote(pgc_cluster, expect): + from pgc_cluster import Cluster, pick_port + + pg_config = pgc_cluster.pg_config + libdir, sharedir = pgc_cluster.libdir, pgc_cluster.sharedir + extdir = os.path.join(sharedir, "extension") + + # The old source. CI checks out without tags, so fetch the one this needs. + have = subprocess.run(["git", "-C", str(SRCDIR), "rev-parse", "-q", "--verify", + f"{OLD_REF}^{{commit}}"], capture_output=True) + if have.returncode != 0: + subprocess.run(["git", "-C", str(SRCDIR), "fetch", "-q", "--depth", "1", "origin", + f"refs/tags/{OLD_REF}:refs/tags/{OLD_REF}"], capture_output=True) + have = subprocess.run(["git", "-C", str(SRCDIR), "rev-parse", "-q", "--verify", + f"{OLD_REF}^{{commit}}"], capture_output=True) + if have.returncode != 0: + expect.cannot_run("ABSENT_FIXTURE", f"{OLD_REF} is not in this clone and could " + "not be fetched") + return + + work = pathlib.Path(tempfile.mkdtemp(prefix="pgc-downgrade-")) + os.chmod(work, 0o777) + snap = _snapshot([libdir, extdir]) + expect.at_least(len(snap), 2, "premise: the prefix holds the tree's own install to put back") + cluster = None + try: + old = work / "old" + _run(["git", "clone", "-q", "--shared", str(SRCDIR), str(old)]) + _run(["git", "-C", str(old), "checkout", "-q", "--detach", OLD_REF]) + built = subprocess.run(["make", "-C", str(old), f"PG_CONFIG={pg_config}", + f"-j{os.cpu_count() or 2}"], capture_output=True, text=True) + expect.num(built.returncode, 0, "premise: the old build compiles") + old_so = old / "pgcolumnar.so" + new_so = work / "new.so" + shutil.copyfile(os.path.join(libdir, "pgcolumnar.so"), new_so) + expect.text("differ" if old_so.read_bytes() != new_so.read_bytes() else "same", + "differ", "premise: the two libraries differ") + + # Install the old build: library AND its SQL and control files. + _run(["make", "-C", str(old), f"PG_CONFIG={pg_config}", "install"]) + datadir = work / "data" + datadir.mkdir() + cluster = Cluster(pg_config, "downgrade", datadir, pick_port(7)) + cluster.initdb() + cluster.start() + dsn = cluster.dsn() + + with psycopg.connect(dsn, autocommit=True) as c: + c.execute("CREATE EXTENSION pgcolumnar") + for t in ("kept", "grown"): + c.execute(f"CREATE TABLE {t} (k int, body text) USING pgcolumnar") + c.execute(f"SELECT pgcolumnar.set_options('{t}', compression => 'none')") + c.execute(f"INSERT INTO {t} SELECT g, repeat(md5((g*3)::text), 5)" + f" FROM generate_series(1,{ROWS}) g") + c.execute(f"CREATE INDEX ON {t} (k)") + stored = [c.execute(f"SELECT count(*) FROM {t}").fetchone()[0] + for t in ("kept", "grown")] + expect.text(f"{stored[0]}|{stored[1]}", f"{ROWS}|{ROWS}", + "premise: the old build stored both tables") + + # This tree: put the session's own install back, upgrade, and write. + cluster.stop() + _restore(snap, [libdir, extdir]) + cluster.start() + with psycopg.connect(dsn, autocommit=True) as c: + try: + c.execute("ALTER EXTENSION pgcolumnar UPDATE") + upd = "ok" + except psycopg.Error as e: + upd = str(e).splitlines()[0] + expect.text(upd, "ok", "ALTER EXTENSION UPDATE succeeds") + got = c.execute("SELECT count(*), sum(k) FROM kept").fetchone() + expect.text(f"{got[0]}|{got[1]}", f"{ROWS}|{FULL_SUM}", + "this build reads the table the old build wrote") + c.execute(f"INSERT INTO grown SELECT g, repeat(md5((g*3)::text), 5)" + f" FROM generate_series({ROWS + 1},{ROWS + 80}) g") + c.execute("CREATE TABLE fresh (k int, body text) USING pgcolumnar") + c.execute("SELECT pgcolumnar.set_options('fresh', compression => 'none')") + c.execute(f"INSERT INTO fresh SELECT g, repeat(md5((g*3)::text), 5)" + f" FROM generate_series(1,{ROWS}) g") + c.execute("CREATE INDEX ON fresh (k)") + stamps = " ".join(str(c.execute( + "SELECT format_version FROM pgcolumnar.storage" + f" WHERE storage_id = pgcolumnar.get_storage_id('{t}')").fetchone()[0]) + for t in ("kept", "grown", "fresh")) + expect.text(stamps, "1 2 2", "stamps after this build: read, appended, new") + + # Roll the library back under the upgraded catalog. + cluster.stop() + _place(old_so, os.path.join(libdir, "pgcolumnar.so")) + cluster.start() + expect.text("old" if pathlib.Path(libdir, "pgcolumnar.so").read_bytes() + == old_so.read_bytes() else "other", "old", + "premise: the server now runs the old library") + + def fetch(t): + return _sqlstate(dsn, ["SET pgcolumnar.enable_custom_scan = off", + "SET enable_seqscan = off", "SET enable_bitmapscan = off", + f"SELECT body FROM {t} WHERE k = 777"]) + + with psycopg.connect(dsn, autocommit=True) as c: + got = c.execute("SELECT count(*), sum(k) FROM kept").fetchone() + expect.text(f"{got[0]}|{got[1]}", f"{ROWS}|{FULL_SUM}", + "the old build still reads a table this build only read") + expect.text(fetch("kept"), "ok", "and fetches from it by index") + expect.text(_sqlstate(dsn, ["SELECT count(body) FROM grown"]), "0A000", + "the old build refuses a table this build appended to, on a scan") + expect.text(fetch("grown"), "0A000", "the old build refuses a table this build appended to, on an index fetch") + expect.text(_sqlstate(dsn, ["SELECT count(body) FROM fresh"]), "0A000", + "the old build refuses a table this build wrote, on a scan") + expect.text(fetch("fresh"), "0A000", "the old build refuses a table this build wrote, on an index fetch") + finally: + if cluster is not None: + cluster.stop() + _restore(snap, [libdir, extdir]) + shutil.rmtree(work, ignore_errors=True) diff --git a/test/pytest/test_native_agg.py b/test/pytest/test_native_agg.py new file mode 100644 index 000000000..ea5fd5648 --- /dev/null +++ b/test/pytest/test_native_agg.py @@ -0,0 +1,114 @@ +"""min/max over timestamptz are answered from zone maps and read no data (#1293). + +"What is the latest reading?" -- `max(ts)` -- is the time-series question. An +ungrouped, unfiltered `min`/`max` is answered from the row groups' zone maps +(Phase D5b), and a group with deletes is decoded rather than folded (#149). + +THE ARM THIS FILE EXISTS FOR IS THE BLOCK COUNT. Proving the metadata node is +chosen and that the answer matches heap does not show the data went unread: a +build that kept the node and decoded every group anyway returns the same answer. +Measured on the shell twin with every group forced dirty: max(ts) read 414 blocks +against a decode's 403, and every parity arm stayed green. So each arm reads the +table's own blocks from pg_statio against a control that must decode the column. + +A partial port of `test/native_agg.sh`: the timestamptz arms only. The int4, +text, sum/avg, parallel and fallback arms are not ported, and +`test_compare_to_bash.py` declares the pair INCOMPLETE with that reason. +Independent of the shell twin: its own table, row count, key spacing and +delete windows. + +`pg_stat_reset()` is database-wide; this file must not run concurrently with +another that reads statistics. +""" + +ROWS = 300000 + + +def _one(conn, sql): + with conn.cursor() as cur: + cur.execute(sql) + row = cur.fetchone() + return row[0] if row else None + + +def _blocks(conn, table, sql): + """Blocks of `table` read while `sql` ran. + + FLUSH BEFORE THE RESET: this connection's own pending statistics would + otherwise land on top of the reading (test_catalog_delete_index.py). + """ + with conn.cursor() as cur: + cur.execute("SELECT pg_stat_force_next_flush()") + cur.execute("SELECT pg_stat_reset()") + cur.execute(sql) + cur.fetchall() + cur.execute("SELECT pg_stat_force_next_flush()") + cur.execute( + "SELECT coalesce(heap_blks_read,0) + coalesce(heap_blks_hit,0)" + " FROM pg_statio_user_tables WHERE relname = %s" + " AND schemaname = current_schema()", + (table,), + ) + row = cur.fetchone() + return int(row[0]) if row else None + + +def test_min_max_over_timestamptz_read_no_data(pgc_conn, expect): + with pgc_conn.cursor() as cur: + cur.execute("CREATE TABLE readings (sensor bigint, at timestamptz) USING pgcolumnar") + cur.execute("SELECT pgcolumnar.set_options('readings', stripe_row_limit => 2000)") + # bigint arithmetic: the shell twin's first draft overflowed int here, the + # INSERT failed silently, and every arm measured an empty table. + cur.execute( + "INSERT INTO readings SELECT g % 37, '2025-11-15'::timestamptz" + f" + ((g::bigint * 104729) % {ROWS}) * interval '2 min'" + f" FROM generate_series(1, {ROWS}) g" + ) + cur.execute("CREATE TABLE readings_heap AS SELECT * FROM readings") + + expect.num(_one(pgc_conn, "SELECT count(*) FROM readings"), ROWS, + "premise: the timestamptz table holds every row inserted") + + with pgc_conn.cursor() as cur: + cur.execute("EXPLAIN SELECT max(at) FROM readings") + plan = "\n".join(r[0] for r in cur.fetchall()) + expect.text("yes" if "Columnar Vectorized Aggregates" in plan else "no", "yes", + "premise: max(ts) over timestamptz plans the metadata agg node") + + decode = _blocks(pgc_conn, "readings", "SELECT max(at + interval '0 s') FROM readings") + mx = _blocks(pgc_conn, "readings", "SELECT max(at) FROM readings") + mn = _blocks(pgc_conn, "readings", "SELECT min(at) FROM readings") + print(f"-- readings blocks: decode={decode} max={mx} min={mn}") + + expect.at_least(decode, 10, "control: decoding ts reads the table's blocks") + # The margin is decode - 10 * reading: positive passes, and a failure + # reports by how much it fell short. + expect.at_least(decode - 10 * mx, 1, + "max(ts) reads under a tenth of the blocks a decode of ts reads") + expect.at_least(decode - 10 * mn, 1, + "min(ts) reads under a tenth of the blocks a decode of ts reads") + + def both(sql): + return (str(_one(pgc_conn, sql.format(t="readings"))), + str(_one(pgc_conn, sql.format(t="readings_heap")))) + + got, want = both("SELECT max(at) FROM {t}") + expect.text(got, want, "max(timestamptz) parity") + got, want = both("SELECT min(at) FROM {t}") + expect.text(got, want, "min(timestamptz) parity") + + # The newest and oldest readings deleted: a zone map still records them. + with pgc_conn.cursor() as cur: + for t in ("readings", "readings_heap"): + cur.execute( + f"DELETE FROM {t} WHERE at > (SELECT max(at) FROM readings_heap) - interval '1 hour'" + f" OR at < (SELECT min(at) FROM readings_heap) + interval '45 min'" + ) + n, nh = (_one(pgc_conn, "SELECT count(*) FROM readings"), + _one(pgc_conn, "SELECT count(*) FROM readings_heap")) + expect.text(f"{n < ROWS}|{n}", f"True|{nh}", + "premise: the deletes removed rows at both ends, the same rows from each") + got, want = both("SELECT max(at) FROM {t}") + expect.text(got, want, "max(timestamptz) parity after deleting the newest rows") + got, want = both("SELECT min(at) FROM {t}") + expect.text(got, want, "min(timestamptz) parity after deleting the oldest rows") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 7ca80c94f..f592d4931 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -2043,6 +2043,35 @@ if [ "${PGC_RUN_UPGRADE:-0}" = 1 ]; then SUMMARY+=("FAIL extension_upgrade (no runnable pg_config)") overall=1 fi + + # Format downgrade (#1137), on the same switch, invoked rather than only + # exempted for extension_upgrade's reason (#396): a guard nobody runs is the + # failure. It rolls the library back to the previous release under an upgraded + # catalog and requires the old binary to refuse what this build wrote. It + # builds that release from a throwaway clone, so it takes the same old source. + if [ -x "$_ex" ]; then + _fdlog="$(mktemp "/tmp/pgcolumnar-fmtdown-${_exmaj}.XXXXXX.log")" + bash "$SRCDIR/test/format_downgrade.sh" "$_ex" \ + ${PGC_DOWNGRADE_OLD_SRC:+"$PGC_DOWNGRADE_OLD_SRC"} >"$_fdlog" 2>&1 + _fdrc=$? + if [ "$_fdrc" = 0 ]; then + echo " PASS format_downgrade PG$_exmaj" + SUMMARY+=("PASS format_downgrade PG$_exmaj") + rm -f "$_fdlog" + elif [ "$_fdrc" = 2 ]; then + echo " SKIP format_downgrade PG$_exmaj" + grep -E '^\s*(SKIP| )' "$_fdlog" | sed 's/^/ /' + SUMMARY+=("SKIP format_downgrade PG$_exmaj") + rm -f "$_fdlog" + else + echo " FAIL format_downgrade PG$_exmaj" + grep -E '^\s*FAIL' "$_fdlog" | sed 's/^/ >> /' + tail -30 "$_fdlog" | sed 's/^/ /' + echo " full log: $_fdlog" + SUMMARY+=("FAIL format_downgrade PG$_exmaj") + overall=1 + fi + fi fi echo diff --git a/test/run_coverage.sh b/test/run_coverage.sh index 0379fc08a..f3e873d5c 100755 --- a/test/run_coverage.sh +++ b/test/run_coverage.sh @@ -111,7 +111,7 @@ not_a_suite() { case "$1" in lib|portlib|run_all_versions|build_all_versions|devloop|rebuild) return 0 ;; native_scale|build_san|run_san|run_coverage) return 0 ;; - pg_upgrade|extension_upgrade|capability_sweep) return 0 ;; + pg_upgrade|extension_upgrade|format_downgrade|capability_sweep) return 0 ;; *) return 1 ;; esac } diff --git a/test/selftest/040-the-detection-primitive-itself.sh b/test/selftest/040-the-detection-primitive-itself.sh index 7547904ee..04ffe2fd6 100644 --- a/test/selftest/040-the-detection-primitive-itself.sh +++ b/test/selftest/040-the-detection-primitive-itself.sh @@ -60,7 +60,7 @@ RUNNER="$TESTDIR/run_all_versions.sh" # not a suite the ordinary five-major matrix can carry. not_a_suite() { case "$1" in - lib|portlib|run_all_versions|build_all_versions|devloop|rebuild|native_scale|build_san|run_san|run_coverage|pg_upgrade|extension_upgrade|capability_sweep) return 0 ;; + lib|portlib|run_all_versions|build_all_versions|devloop|rebuild|native_scale|build_san|run_san|run_coverage|pg_upgrade|extension_upgrade|format_downgrade|capability_sweep) return 0 ;; *) return 1 ;; esac } From 5d649ab417d43d3a8a92f330d3572cd01ce2725c Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 12 Oct 2026 20:30:48 +0000 Subject: [PATCH 2/2] test: the library swap declines beside other xdist workers (#1301 review) A backend another worker starts while alpha4's library sits in the shared prefix would load it against this tree's catalogs. The swap test now declines, named (UNMET_PRECONDITION), whenever it is not alone; an arm drives the decision through the environments pytest-xdist sets. Red first: NameError before the helper existed. Serial: 2 passed. Under -n 2: declines with its reason. Co-Authored-By: Claude Opus 5.5 --- test/pytest/TESTS.md | 3 +- test/pytest/expected_tests.txt | 2 +- test/pytest/test_format_downgrade.py | 47 ++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index adf3baadc..7f63e77f4 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -6369,7 +6369,7 @@ Not ported, and declared INCOMPLETE in `test_compare_to_bash.py`: the `int4`/`te Port of `format_downgrade.sh` (#1137). 1.0-alpha5 stops writing a validity bitmap for a chunk with no nulls, and an alpha4 binary misreads such a chunk without refusing it: with the real `v1.0-alpha4` library, 9 of 40 single-row fetches returned NULL for a row that holds a value. The fix is the format stamp. A table this build writes carries `format_version` 2, alpha4 refuses any stamp but 1, and a table this build only reads keeps 1. -THE CLAIM IS ABOUT TWO BINARIES, so this test uses both. It builds `v1.0-alpha4` from the tag, which it fetches when the checkout is shallow, as CI's is. It writes two tables with the old build, upgrades to this tree, reads one, appends to the other and creates a third. Then it puts the old library back under the upgraded catalog, which is what a package rollback does. `native_format.sh`'s arms stand in for alpha4 with a catalog `UPDATE`; this one does not. +THE CLAIM IS ABOUT TWO BINARIES, so this test uses both, and it runs only when it is alone: see the second test below. It builds `v1.0-alpha4` from the tag, which it fetches when the checkout is shallow, as CI's is. It writes two tables with the old build, upgrades to this tree, reads one, appends to the other and creates a third. Then it puts the old library back under the upgraded catalog, which is what a package rollback does. `native_format.sh`'s arms stand in for alpha4 with a catalog `UPDATE`; this one does not. ON A CLUSTER OF ITS OWN, AND THE PREFIX IS PUT BACK EXACTLY. The session cluster shares the install prefix. So before anything is installed, every `pgcolumnar*` file in the library and extension directories is saved, bytes and mtime, and restored in a `finally` by atomic rename. Anything the old install added is removed. The mtime matters because the harness compares the library's mtime with the postmaster's start. Nothing is written into a library in place, because a copy over a library a running postmaster has mapped corrupts it. Measured: the prefix's library afterwards is byte-identical to the one this tree built. @@ -6382,3 +6382,4 @@ Removal proof: with `COLUMNAR_NATIVE_FORMAT_VERSION` set back to 1, the stamps r | test | what it holds | | --- | --- | | `test_an_alpha4_binary_refuses_what_this_build_wrote` | the old build stores both tables; the libraries differ; the upgrade succeeds and this build reads the old table; the stamps are `1 2 2` for read, appended and new; on the old library again, the table only read is still read and fetched by index, and the appended and new tables are refused with `0A000` on a scan and on an index fetch | +| `test_the_library_swap_declines_beside_other_workers` | the decision that makes the swap decline beside other xdist workers, driven with the environments pytest-xdist sets: a serial run and a one-worker run proceed, a run beside three other workers declines, and a worker with no stated count is treated as not alone. A backend another worker started during the swap would load alpha4 against this tree's catalogs and fail in an unrelated test; atomic renames protect only processes that had already mapped the library (found by @jdatcmd). Under `-n`, the swap test declines as `UNMET_PRECONDITION` with that reason, so a parallel run ends INCOMPLETE naming it rather than risking someone else's test | diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 6fbf68788..3d4aa087b 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -543,4 +543,4 @@ guard_tests 404 # own test is not in it. Git conflicted again -- loudly, because the key moved # on both sides -- and the answer is neither branch's number. Collected on the # rebased tree, not added up. -cluster_tests 493 +cluster_tests 494 diff --git a/test/pytest/test_format_downgrade.py b/test/pytest/test_format_downgrade.py index 82b79029a..8043c60f1 100644 --- a/test/pytest/test_format_downgrade.py +++ b/test/pytest/test_format_downgrade.py @@ -39,6 +39,23 @@ SRCDIR = pathlib.Path(__file__).resolve().parent.parent.parent +def _beside_other_workers(env): + """True when this process may share the install prefix with other workers. + + pytest-xdist sets PYTEST_XDIST_WORKER in every worker and + PYTEST_XDIST_WORKER_COUNT to the number of workers. A serial run sets + neither. A worker with no stated count is treated as not alone: guessing + "alone" is the direction that breaks someone else's test. + """ + count = env.get("PYTEST_XDIST_WORKER_COUNT") + if count is not None: + try: + return int(count) > 1 + except ValueError: + return True + return "PYTEST_XDIST_WORKER" in env + + def _run(argv, **kw): return subprocess.run(argv, check=True, capture_output=True, text=True, **kw) @@ -87,6 +104,12 @@ def _sqlstate(dsn, statements): def test_an_alpha4_binary_refuses_what_this_build_wrote(pgc_cluster, expect): from pgc_cluster import Cluster, pick_port + if _beside_other_workers(os.environ): + expect.cannot_run("UNMET_PRECONDITION", + "swaps the library in the shared install prefix, which a backend " + "another xdist worker starts would load; run this file without -n") + return + pg_config = pgc_cluster.pg_config libdir, sharedir = pgc_cluster.libdir, pgc_cluster.sharedir extdir = os.path.join(sharedir, "extension") @@ -200,3 +223,27 @@ def fetch(t): cluster.stop() _restore(snap, [libdir, extdir]) shutil.rmtree(work, ignore_errors=True) + + +def test_the_library_swap_declines_beside_other_workers(expect): + """The swap puts alpha4's library in the SHARED prefix for a few seconds. + + A backend that another xdist worker starts during that window loads alpha4 + against this tree's catalogs, and fails in an unrelated test, at random, in a + way a serial run cannot reproduce. Atomic renames protect processes that had + already mapped the library, not new ones. So the test declines -- named, + UNMET_PRECONDITION, never a pass -- whenever it is not alone. Found by + @jdatcmd reviewing #1301. + + Driven through the decision itself, with the environments pytest-xdist sets, + so both directions are asserted without starting workers. + """ + expect.text(str(_beside_other_workers({})), "False", + "a serial run is alone, so the swap runs") + expect.text(str(_beside_other_workers({"PYTEST_XDIST_WORKER_COUNT": "1"})), "False", + "one xdist worker is alone, so the swap runs") + expect.text(str(_beside_other_workers({"PYTEST_XDIST_WORKER_COUNT": "4", + "PYTEST_XDIST_WORKER": "gw2"})), "True", + "beside three other workers the swap declines") + expect.text(str(_beside_other_workers({"PYTEST_XDIST_WORKER": "gw0"})), "True", + "a worker whose count is not stated is assumed not to be alone")