From d8b43f95c48fb5038eb0adf1263087f7ea2b9fe6 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 9 Sep 2026 10:27:10 -0600 Subject: [PATCH 1/8] test: pin what the Hilbert curve buys in range locality (#889) test/hilbert_locality.sh measures the one thing #889 was added for and that neither hilbert_curve.sh nor hilbert_cluster.sh can see: whether laying a table on the Hilbert curve puts two-dimensionally near rows in the same row group. One fixture, 200,000 rows over a [0,100000) square in two int columns, materialised once into a heap table and loaded into both arms from there. stripe_row_limit 1500 is deliberately non-dyadic; 134 groups on both arms. One arm gets cluster() (Z-order), the other cluster_hilbert(). The suite then sums the engine's own "Columnar Chunk Groups Read" over 60 deterministic window placements at each of four window sizes. The result is pinned as EXACT INTEGERS, not as a threshold: box z_total h_total z/h 2000 241 118 2.0424 5000 351 209 1.6794 12000 588 402 1.4627 30000 1624 1313 1.2369 A threshold is the thing someone lowers when it reddens. h < z is asserted separately at every box, so a reader can tell "the layout moved" from "Hilbert stopped winning". The controls are what make the ratio mean anything. The partition digest is order-INDEPENDENT (per group, one string from both columns' min/max; those strings sorted, then hashed), because a digest ordered by group_number reports the NUMBERING and calls two identical partitions different. Two tables on the same curve hash equal, and a dense 256x256 dyadic grid hashes equal across the two curves -- the degenerate case the design predicted. If the two partitions are not different the suite refuses to report a ratio at all: measured, that mutation gives 2 failed + 12 unrunnable and exits INCOMPLETE. test/pytest/test_hilbert_locality.py is the same properties through the pytest harness of #897, which is not merged; the file says so in its header and cannot run on main. Run against #897 assembled beside it, it reproduces all eight integers: 16 passed. Neither file is registered in test/run_all_versions.sh yet. Verified on PostgreSQL 18.4, prefix /usr/local/pg18_loc889: 61 passed + 0 failed + 0 unrunnable = 61, hilbert_locality.sh: PASSED. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK --- test/hilbert_locality.sh | 526 +++++++++++++++++++++++ test/pytest/test_hilbert_locality.py | 621 +++++++++++++++++++++++++++ 2 files changed, 1147 insertions(+) create mode 100755 test/hilbert_locality.sh create mode 100644 test/pytest/test_hilbert_locality.py diff --git a/test/hilbert_locality.sh b/test/hilbert_locality.sh new file mode 100755 index 00000000..7484497d --- /dev/null +++ b/test/hilbert_locality.sh @@ -0,0 +1,526 @@ +#!/usr/bin/env bash +# +# pgColumnar Hilbert clustering: does the curve actually buy range locality? +# (issue #889, the measurement half.) +# +# WHAT THIS SUITE IS FOR +# +# test/hilbert_curve.sh pins the CURVE in C, by its mathematical properties and +# by frozen bytes. test/hilbert_cluster.sh pins the SQL SURFACE: the two verbs, +# the recorded kind, the gates and the daemon. Neither of them can tell whether +# laying a table on the Hilbert curve does the one thing the feature was added +# for -- put two-dimensionally near rows in the same row group, so that a +# two-dimensional range predicate has fewer groups to open. +# +# This file is that question and only that question. It builds one fixture, +# lays it out twice -- once with pgcolumnar.cluster() (Z-order) and once with +# pgcolumnar.cluster_hilbert() -- and counts the engine's own +# "Columnar Chunk Groups Read" over 60 window placements at each of four window +# sizes. The counts are PINNED AS EXACT INTEGERS. +# +# WHY EXACT INTEGERS AND NOT A THRESHOLD +# +# A threshold is the thing someone lowers when it reddens. "Hilbert reads at +# most 0.6x the groups Z-order does" survives a regression that costs half the +# benefit, and it survives it silently. An exact pin cannot: any change to the +# layout, to the curve, to the group sizing or to the skip logic moves one of +# these eight numbers and the suite says which. The pins are the measurement, +# not a target -- if one moves, the correct response is to find out what moved +# it and then, if the new layout is better, re-pin with the new numbers beside +# the reason. +# +# Alongside the pins, and separately, h < z is asserted AT EVERY BOX SIZE. That +# split is deliberate. If the pins move but h < z still holds, the layout +# changed. If h < z fails, HILBERT STOPPED WINNING, which is a different and +# much worse fact, and a reader must be able to tell the two apart from the +# output without re-deriving anything. +# +# WHAT THIS SUITE DOES NOT CLAIM +# +# - NOT that Hilbert is faster. Nothing here is timed. The unit is groups +# read, which is a count the engine reports about its own work; wall clock +# on this hardware is not a fair instrument and is not used. +# - NOT that Hilbert wins on every workload. It measures square windows over +# two uniformly distributed int columns. A one-dimensional predicate, a +# skewed distribution, a non-square window or a different column count is a +# different measurement and this suite says nothing about any of them. +# - NOT that Hilbert wins on every fixture. Arm 4 below is a fixture where it +# provably does not, and it is in here as a control precisely so that the +# win reported by arms 8a-8d cannot be read as a universal one. +# - NOT anything about the curve's mathematics or the SQL surface. Those are +# the two suites named above and this one does not repeat them. +# +# THREE DEFECTS THIS SHAPE EXISTS TO AVOID. Each was found in the pilot that +# produced these numbers, and each produced a plausible ratio while measuring +# something else: +# +# 1. THE TWO ARMS MUST HOLD THE IDENTICAL ROWS. The pilot ran a random() +# INSERT once per table, so the arms held DIFFERENT DATA and the ratio was +# a fact about the data rather than about the curve. Hence the heap table +# `src`, materialised once, from which both arms load -- and hence arm 1, +# which asserts the row multisets are equal rather than assuming that two +# INSERTs from one source produced one. +# +# 2. THE PARTITION DIGEST MUST BE ORDER-INDEPENDENT. The pilot's first digest +# ordered each column's min/max BY group_number, so it reported "the +# partitions differ" when the two curves had merely NUMBERED the same +# groups differently. Under that digest the dense dyadic control (arm 4) +# PASSED its premise and went on to report a ratio, which is exactly the +# case the control exists to catch. The digest below groups by +# group_number, builds one string per group from BOTH columns' min and max, +# and then sorts THOSE STRINGS before hashing. Two identical partitions +# hash equal however they are numbered. +# +# 3. A SINGLE WINDOW ORIGIN IS NOT A MEASUREMENT. At one origin the pilot's +# four differences were 1, 1, 0 and 1 groups, and the reported 2.000 ratio +# came off a single group. Hence 60 deterministic placements per box, and +# hence the assertion that all 60 were actually measured -- a grep that +# matched 3 lines and summed them would otherwise report a smaller total +# and read as a better result. +# +# WHY THE CONTROLS ARE NOT OPTIONAL +# +# Arms 8a-8d divide one number by another and call the quotient a benefit. That +# is only meaningful if the two arms differ IN THE LAYOUT and in nothing else, +# so the suite refuses to report the measurement at all unless arm 2 has +# established that the two partitions really are two different partitions. Arm 3 +# proves that refusal can fire: two tables laid out by the SAME verb produce +# IDENTICAL digests, so the digest is reporting the curve rather than reporting +# that any two tables differ. Arm 4 is the degenerate fixture the design +# predicted -- a dense dyadic grid, where the two curves cut the same blocks -- +# and there the digests are identical too, on real data, from the same +# instrument. +# +# Usage: test/hilbert_locality.sh [PG_CONFIG] +# Written fresh for pgColumnar. +# +# DELIBERATELY NOT REGISTERED in test/run_all_versions.sh yet. Registering it is +# part of the PR that lands it in the matrix; harness_selftest.sh sweeps +# test/*.sh and asserts every suite is registered, so this file makes that arm +# red until then. + +set -uo pipefail + +# Every digest below is a claim about the PHYSICAL group boundaries, and every +# group count is read out of a plan. A parallel plan divides that work across +# workers, so parallelism off is not a tuning choice: it is what makes the +# counters a fact about the layout rather than about scheduling. +export PGC_EXTRA_CONF="max_parallel_workers_per_gather=0" + +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +# ---- the fixture's constants ------------------------------------------------ + +# 200,000 rows over a [0, 100000) square in two int columns. +ROWS=200000 +SPAN=100000 + +# NOT DYADIC, ON PURPOSE. 200000/1500 is 133.33, so the last group is a short +# one and no group boundary lines up with a power of two. A dyadic stripe over a +# dyadic domain is the case where Z-order and Hilbert cut the same blocks and +# the measured difference collapses -- which is arm 4, kept as a control rather +# than allowed to become the main fixture by accident. +SR=1500 + +# The window placements. Two coprime strides over the span, so the 60 origins +# neither repeat nor march in step with the group boundaries, and the same 60 +# are used for both arms and every box. +PLACEMENTS=60 +STRIDE_X=997 +STRIDE_Y=7919 + +# ---- the instruments -------------------------------------------------------- + +# WHAT COUNTS AS A MEASUREMENT. A digest helper that returns the empty string on +# a query that could not run lets two FAILED reads compare equal and pass an +# equality arm (#418); the same empty string satisfies an INEQUALITY arm +# outright, which is the shape that matters here because arm 2 is an inequality. +# So the helpers return sentinels and every comparison goes through differs(). +pgc_measured() { # pgc_measured VALUE -> 0 when VALUE is a real measurement + case "$1" in + '' | QUERY_ERROR.* | EMPTY | NO_PARTITION) return 1 ;; + esac + return 0 +} + +differs() { # differs A B -> different | IDENTICAL | UNMEASURED[...] + pgc_measured "$1" || { printf 'UNMEASURED[a=%s]\n' "$1"; return; } + pgc_measured "$2" || { printf 'UNMEASURED[b=%s]\n' "$2"; return; } + if [ "$1" != "$2" ]; then echo different; else echo IDENTICAL; fi +} + +# THE PARTITION DIGEST, ORDER-INDEPENDENT BY CONSTRUCTION. +# +# One string per row group, built from BOTH clustering columns' min and max +# (ordered by column_index, so the two columns always appear in the same order +# within a group), and then those strings SORTED before hashing. What is hashed +# is therefore the SET of group boxes, and two layouts that cut the same boxes +# hash equal however the curve numbered them. +# +# Ordering the groups by group_number instead -- the obvious first version -- +# makes the digest report the NUMBERING as well as the partition, so it says +# "different" for two identical partitions and the dense control below passes +# its premise and produces numbers. That is defect 2 in the header. +# +# vector_index = -1 is the row-group-level zone map: the box of the whole group, +# which is what decides whether the group can be skipped. The per-vector rows +# (vector_index >= 0) describe a finer unit and are not what a group count is +# about. +partition_digest() { # partition_digest TABLE -> 12 hex chars, or NO_PARTITION + local d + d="$(q "SELECT substr(md5(string_agg(g, ',' ORDER BY g)), 1, 12) FROM ( + SELECT string_agg(column_index::text || ':' || + encode(minimum, 'hex') || ':' || + encode(maximum, 'hex'), '/' ORDER BY column_index) AS g + FROM pgcolumnar.zone_map + WHERE storage_id = pgcolumnar.get_storage_id('$1') + AND vector_index = -1 AND column_index IN (0, 1) + GROUP BY group_number) t;")" + printf '%s\n' "${d:-NO_PARTITION}" +} + +# The number of row groups the partition digest was built over, from the +# catalog. Printed and asserted rather than assumed from ROWS/SR: set_options +# has a floor and a call below it RAISES, and a suite that discards the error +# then measures a fixture built on the defaults. +group_count() { # group_count TABLE + q "SELECT count(*) FROM pgcolumnar.row_group + WHERE storage_id = pgcolumnar.get_storage_id('$1');" +} + +# A plan, with the run's own counters in it. +plan_of() { # plan_of SQL + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -At -c "EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF) $1" 2>&1 +} + +# One counter out of a plan. Empty when the line is absent, which check_num +# rejects rather than compares. +field() { # field PLAN LABEL + sed -n "s/.*$2: \([0-9]*\).*/\1/p" <<<"$1" | head -1 +} + +# The query under measurement. One shape, one place, so the arms and the +# premises cannot drift into asking about two different queries. +window_sql() { # window_sql TABLE OX OY BOX + printf 'SELECT count(*) FROM %s WHERE a BETWEEN %d AND %d AND b BETWEEN %d AND %d' \ + "$1" "$2" "$(($2 + $4))" "$3" "$(($3 + $4))" +} + +# THE MEASUREMENT ITSELF: 60 placements, one psql session, two numbers out. +# +# Returns "LINES TOTAL": how many "Columnar Chunk Groups Read" lines the run +# actually produced, and their sum. Both are returned because the sum alone +# cannot distinguish a genuinely small number of groups from a grep that matched +# fewer statements than were sent -- a truncated run reads as a BETTER result, +# which is the direction a defect here would go undetected. +read_groups() { # read_groups TABLE BOX -> "LINES TOTAL" + local t="$1" box="$2" f i ox oy span out + span=$((SPAN - box)) + f="$PGC_SQLDIR/loc.$t.$box.sql" + : > "$f" + for i in $(seq 1 "$PLACEMENTS"); do + ox=$(( (i * STRIDE_X) % span )) + oy=$(( (i * STRIDE_Y) % span )) + printf 'EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF) %s;\n' \ + "$(window_sql "$t" "$ox" "$oy" "$box")" >> "$f" + done + out="$(psql_file "$f")" + printf '%s\n' "$out" | awk ' + /Columnar Chunk Groups Read:/ { n++; v = $NF; gsub(/[^0-9]/, "", v); s += v } + END { print n+0, s+0 }' +} + +# The SQLSTATE a statement raises, as a value. +# +# A CALL MADE WITH psql_run IS NOT AN ASSERTION. This suite runs under +# `set -uo pipefail` with no -e and psql_run's exit status is checked nowhere in +# this tree, so a cluster verb that RAISED would leave the table simply +# unclustered -- and an unclustered table has a partition digest, a group count +# and a plan, so it reddens the pins below as though the CURVE had changed. +# Every layout verb goes through crun(), which says which one raised and with +# what state. +# +# The sentinel statement behind the one under test is not decoration: without +# it, "no ERROR line was found" is also what a psql that never reached the +# server produces, so an unreachable probe reads as success. +sqlstate() { # sqlstate SQL -> a five-character SQLSTATE, noerror, or PROBE_UNREACHABLE + local out st + out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -At -v VERBOSITY=sqlstate -v ON_ERROR_STOP=0 \ + -c "$1" -c "SELECT 'PGC_PROBE_OK';" 2>&1)" + case "$out" in + *PGC_PROBE_OK*) ;; + *) echo PROBE_UNREACHABLE; return ;; + esac + st="$(printf '%s\n' "$out" | sed -n 's/^.*ERROR:[[:space:]]*\([0-9A-Z]\{5\}\).*$/\1/p' | head -1)" + if [ -n "$st" ]; then printf '%s\n' "$st"; else echo noerror; fi +} + +crun() { # crun WHAT SQL + check_text "premise: $1 ran without raising" "$(sqlstate "$2")" "noerror" +} + +# ---- the fixture ------------------------------------------------------------ +# +# THE DATA IS MATERIALISED ONCE, INTO A HEAP TABLE, AND BOTH ARMS LOAD FROM IT. +# Running the generator once per arm would be two independent evaluations, and +# hashint8 is deterministic but a generator that were not -- random(), which is +# what the pilot used -- gives the two arms different rows and turns the ratio +# into a fact about the data. Loading from one materialised source removes the +# question rather than arguing about it, and arm 1 then checks it anyway. +psql_run "CREATE TABLE src (a int, b int, pad text);" > /dev/null +psql_run "INSERT INTO src (a, b, pad) + SELECT (hashint8(g) % $SPAN + $SPAN) % $SPAN, + (hashint8(g * 2654435761) % $SPAN + $SPAN) % $SPAN, + repeat('x', 40) + FROM generate_series(1, $ROWS) g;" > /dev/null +check_num "premise: the source holds the rows both arms will load" \ + "$(q 'SELECT count(*) FROM src;')" "$ROWS" + +# The generator is only a fair two-dimensional fixture if it fills the square. +# A generator that collapsed one column to a handful of values would make both +# curves the identity in that dimension and the comparison meaningless, and it +# would do so silently. +check_num "premise: column a spans the square, so this is a two-dimensional fixture" \ + "$(q "SELECT (min(a) < $((SPAN / 100)) AND max(a) > $((SPAN - SPAN / 100)))::int FROM src;")" "1" +check_num "premise: column b spans the square too" \ + "$(q "SELECT (min(b) < $((SPAN / 100)) AND max(b) > $((SPAN - SPAN / 100)))::int FROM src;")" "1" + +# hz: Z-order. hh: Hilbert. Identical in every other respect, and built by the +# same three statements in the same order. +for t in hz hh; do + psql_run "CREATE TABLE $t (a int, b int, pad text) USING pgcolumnar;" > /dev/null + psql_run "SELECT pgcolumnar.set_options('$t', stripe_row_limit => $SR);" > /dev/null + psql_run "INSERT INTO $t SELECT * FROM src;" > /dev/null +done +crun "cluster() on the Z-order arm" "SELECT pgcolumnar.cluster('hz', 'a', 'b');" +crun "cluster_hilbert() on the Hilbert arm" "SELECT pgcolumnar.cluster_hilbert('hh', 'a', 'b');" + +HZ_GROUPS="$(group_count hz)" +HH_GROUPS="$(group_count hh)" +echo "-- groups: hz=$HZ_GROUPS hh=$HH_GROUPS (stripe_row_limit=$SR over $ROWS rows)" + +# ============================================================================= +# ARM 1 PREMISE: THE TWO ARMS HOLD THE IDENTICAL ROW MULTISET +# ============================================================================= +# +# Without this every number below is a fact about two different tables. The +# hash is order-blind by construction (pgc_set_hash sorts the rendered rows +# before hashing), which is exactly right here: the arms are supposed to differ +# in ORDER and in nothing else, so an order-SENSITIVE oracle would report a +# difference for the very property under test. +HZ_SET="$(pgc_set_hash 'SELECT * FROM hz')" +HH_SET="$(pgc_set_hash 'SELECT * FROM hh')" +SRC_SET="$(pgc_set_hash 'SELECT * FROM src')" +check_text "premise: the two arms hold the identical row multiset" "$HH_SET" "$HZ_SET" +# And that multiset is the source's. Comparing the arms only to each other +# passes if both loads went equally wrong. +check_text "premise: and it is the source's multiset, so neither load dropped rows" \ + "$HZ_SET" "$SRC_SET" +# A hash of nothing equals a hash of nothing. pgc_set_hash returns EMPTY for a +# genuinely empty relation and QUERY_ERROR.N for a read that failed, so this +# rejects both without having to trust that the two above compared anything. +check_num "premise: and it is a hash of rows, not of an empty or failed read" \ + "$(pgc_measured "$HZ_SET" && echo 1 || echo 0)" "1" +check_num "premise: both arms hold every source row" \ + "$(q 'SELECT (SELECT count(*) FROM hz) + (SELECT count(*) FROM hh);')" "$((ROWS * 2))" + +# The fixture is only the fixture that was measured if the groups are the size +# they were measured at. 200,000 rows at 1,500 to a group is 134 groups, the +# last one short. +check_num "premise: hz has the group count this measurement was taken at" "$HZ_GROUPS" "134" +check_num "premise: hh has the same group count, so a group is the same unit on both arms" \ + "$HH_GROUPS" "$HZ_GROUPS" + +# ============================================================================= +# ARM 2 PREMISE: THE TWO PARTITIONS ARE TWO DIFFERENT PARTITIONS +# ============================================================================= +# +# This is what licenses arms 8a-8d to attribute their difference to the curve. +# If it fails, the measurement is not reported: there is no curve difference to +# measure, and a ratio printed anyway would be a number about noise. +DZ="$(partition_digest hz)" +DH="$(partition_digest hh)" +echo "-- partition digests: zorder=$DZ hilbert=$DH" +check_text "premise: the Z-order and Hilbert partitions differ" "$(differs "$DZ" "$DH")" "different" + +# Pinned, not merely different. "Different" is satisfied by any change to +# either layout; these two values say the layouts are the ones the pinned +# integers below were measured over. A moved digest beside moved integers is a +# layout change; moved integers beside these digests would be a change in the +# reader. +check_text "the Z-order partition is the one these numbers were measured over" "$DZ" "2169ae4551d8" +check_text "the Hilbert partition is the one these numbers were measured over" "$DH" "1706e49ef5a2" + +# ============================================================================= +# ARM 3 CONTROL: SAME CURVE TWICE -> THE DIGESTS ARE IDENTICAL +# ============================================================================= +# +# Arm 2 is an inequality, and an inequality passes for any instrument that +# reports "different" too readily -- including one that is really reporting +# "these are two different tables". This is the removal proof for that: two +# tables built the same way from the same source and laid out by the SAME verb +# must hash EQUAL. If this fails, arm 2 has proved nothing and neither has +# anything below it. +for t in cz1 cz2; do + psql_run "CREATE TABLE $t (a int, b int, pad text) USING pgcolumnar;" > /dev/null + psql_run "SELECT pgcolumnar.set_options('$t', stripe_row_limit => $SR);" > /dev/null + psql_run "INSERT INTO $t SELECT * FROM src;" > /dev/null + crun "cluster() on control table $t" "SELECT pgcolumnar.cluster('$t', 'a', 'b');" +done +CZ1="$(partition_digest cz1)" +CZ2="$(partition_digest cz2)" +echo "-- control digests, both Z-order: $CZ1 $CZ2" +check_text "control: two tables on the same curve have the identical partition" \ + "$(differs "$CZ1" "$CZ2")" "IDENTICAL" +# And it is the same partition the measured Z-order arm has, so the control is +# a control ON THIS FIXTURE rather than on an unrelated one. +check_text "control: and that partition is the measured Z-order arm's" "$CZ1" "$DZ" + +# ============================================================================= +# ARM 4 CONTROL: A DENSE DYADIC GRID -> THE TWO CURVES AGREE +# ============================================================================= +# +# The design predicted the degenerate case: over a dense grid whose side is a +# power of two, cut into groups whose size is a power of two, Z-order and +# Hilbert visit the same blocks in a different ORDER and therefore produce the +# same set of group boxes. This is that case, on real data, through the same +# digest -- so the suite carries its own counterexample to "Hilbert always +# changes the layout", and a reader can see that arm 2's "different" is a +# measurement rather than a foregone conclusion. +# +# 256 x 256 = 65,536 rows at 1,024 to a group is 64 groups, all full. +DENSE_SIDE=256 +DENSE_SR=1024 +psql_run "CREATE TABLE dsrc (a int, b int);" > /dev/null +psql_run "INSERT INTO dsrc SELECT (g - 1) / $DENSE_SIDE, (g - 1) % $DENSE_SIDE + FROM generate_series(1, $((DENSE_SIDE * DENSE_SIDE))) g;" > /dev/null +for t in dz dh; do + psql_run "CREATE TABLE $t (a int, b int) USING pgcolumnar;" > /dev/null + psql_run "SELECT pgcolumnar.set_options('$t', stripe_row_limit => $DENSE_SR);" > /dev/null + psql_run "INSERT INTO $t SELECT * FROM dsrc;" > /dev/null +done +crun "cluster() on the dense control" "SELECT pgcolumnar.cluster('dz', 'a', 'b');" +crun "cluster_hilbert() on the dense control" "SELECT pgcolumnar.cluster_hilbert('dh', 'a', 'b');" +check_num "control premise: the dense grid is dense -- every cell present exactly once" \ + "$(q "SELECT count(*) FROM (SELECT a, b FROM dsrc GROUP BY a, b HAVING count(*) <> 1) x;")" "0" +check_num "control premise: the dense grid has the dyadic group count" \ + "$(group_count dz)" "$(( DENSE_SIDE * DENSE_SIDE / DENSE_SR ))" +DDZ="$(partition_digest dz)" +DDH="$(partition_digest dh)" +echo "-- dense grid digests: zorder=$DDZ hilbert=$DDH" +check_text "control: on a dense dyadic grid the two curves cut the identical partition" \ + "$(differs "$DDZ" "$DDH")" "IDENTICAL" + +# ============================================================================= +# ARMS 5-7 PREMISES ABOUT THE PLAN THE COUNTERS COME OUT OF +# ============================================================================= +# +# The counters below are the columnar scan's own instrumentation. Three things +# have to hold before a difference in them is a difference in LAYOUT: +# +# 5. the query planned as a columnar scan on BOTH arms -- a fallback would +# read no groups at all and report nothing to sum; +# 6. the predicates were pushed down AND USABLE on both arms. "Pushed-Down +# Filters" counts what the reader was handed; a key it could not build a +# skip predicate from excludes no group at all while still being reported +# as pushed down (#477). Usable is the number that says skipping is +# possible, so a zero here would mean both arms read everything and the +# ratio would be 1.0 for a reason that is not about the curve; +# 7. the two arms have the same number of groups TO read. A ratio between two +# different denominators is not a ratio. +BOXES="2000 5000 12000 30000" + +PROBE_SQL_HZ="$(window_sql hz $((STRIDE_X % (SPAN - 2000))) $((STRIDE_Y % (SPAN - 2000))) 2000)" +PROBE_SQL_HH="$(window_sql hh $((STRIDE_X % (SPAN - 2000))) $((STRIDE_Y % (SPAN - 2000))) 2000)" +check_text "premise: the Z-order arm plans as a columnar scan" \ + "$(pgc_is_columnar_scan "$PROBE_SQL_HZ")" "yes" +check_text "premise: the Hilbert arm plans as a columnar scan" \ + "$(pgc_is_columnar_scan "$PROBE_SQL_HH")" "yes" + +for box in $BOXES; do + ox=$(( STRIDE_X % (SPAN - box) )) + oy=$(( STRIDE_Y % (SPAN - box) )) + pz="$(plan_of "$(window_sql hz "$ox" "$oy" "$box")")" + ph="$(plan_of "$(window_sql hh "$ox" "$oy" "$box")")" + uz="$(field "$pz" 'Columnar Usable Skip Predicates')" + uh="$(field "$ph" 'Columnar Usable Skip Predicates')" + tz="$(field "$pz" 'Columnar Chunk Groups Total')" + th="$(field "$ph" 'Columnar Chunk Groups Total')" + echo "-- box $box: usable z=$uz h=$uh; total z=$tz h=$th" + # FOUR, NOT TWO, AND THE FOUR IS THE POINT. A BETWEEN is two scan keys, + # >= and <=, and the reader builds a skip predicate from each; two BETWEENs + # over two columns are therefore four usable predicates. This arm was + # written expecting 2, and the plan said 4 (measured on PG 18.4). A "> 0" + # bound would have accepted either and would also accept an arm that had + # lost one whole dimension -- which is the dimension the curve is about -- + # so the exact count is what is pinned. + check_num "premise: box $box, the Z-order arm can skip on all four predicates" "$uz" "4" + check_num "premise: box $box, the Hilbert arm can skip on all four predicates" "$uh" "4" + # Read from the plan on both arms and compared to each other. Not compared + # to a number typed in here: a literal would still be satisfied if BOTH + # arms drifted, and the property is that the denominators match. + check_num "premise: box $box, both arms have the same number of groups to read" "$th" "$tz" + # And the denominator is the fixture's, not a plan that saw a fraction of it. + check_num "premise: box $box, that denominator is the whole relation" "$tz" "$HZ_GROUPS" +done + +# ============================================================================= +# ARM 8 THE MEASUREMENT +# ============================================================================= +# +# Reported only if arm 2 established a curve difference. This is the refusal the +# header describes: with no difference in layout there is nothing for a ratio to +# be about, and printing one anyway is how the pilot's first version produced +# numbers for the dense grid. +CURVE_DIFFERS="$(differs "$DZ" "$DH")" + +# The pins. Measured on main f2af080, PG 18.4, and identical on two consecutive +# runs of the fixture above. box:zorder:hilbert. +PINS="2000:241:118 5000:351:209 12000:588:402 30000:1624:1313" + +for pin in $PINS; do + box="${pin%%:*}" + rest="${pin#*:}" + want_z="${rest%%:*}" + want_h="${rest##*:}" + + if [ "$CURVE_DIFFERS" != different ]; then + check_unrunnable "box $box: groups read, Z-order" UNMET_PRECONDITION \ + "the two partitions are not different ($CURVE_DIFFERS), so a ratio between them is not about the curve" + check_unrunnable "box $box: groups read, Hilbert" UNMET_PRECONDITION \ + "the two partitions are not different ($CURVE_DIFFERS), so a ratio between them is not about the curve" + check_unrunnable "box $box: Hilbert reads fewer groups than Z-order" UNMET_PRECONDITION \ + "the two partitions are not different ($CURVE_DIFFERS)" + continue + fi + + rz="$(read_groups hz "$box")" + rh="$(read_groups hh "$box")" + nz="${rz%% *}"; sz="${rz##* }" + nh="${rh%% *}"; sh="${rh##* }" + + # EVERY PLACEMENT WAS MEASURED. A run that lost statements would sum fewer + # groups and read as a better result on whichever arm lost them. + check_num "box $box: all $PLACEMENTS placements reported a Z-order group count" "$nz" "$PLACEMENTS" + check_num "box $box: all $PLACEMENTS placements reported a Hilbert group count" "$nh" "$PLACEMENTS" + + # The pins. + check_num "box $box: groups read over $PLACEMENTS placements, Z-order" "$sz" "$want_z" + check_num "box $box: groups read over $PLACEMENTS placements, Hilbert" "$sh" "$want_h" + + # And, separately from the pins: the curve still wins here. This is the arm + # that stays meaningful when the pins are re-taken. + check_text "box $box: Hilbert reads fewer groups than Z-order" \ + "$(awk -v a="$sh" -v b="$sz" 'BEGIN { print (a + 0 < b + 0) ? "fewer" : "NOT FEWER" }')" \ + "fewer" + + # The ratio is PRINTED, from the two numbers just measured, and asserted + # nowhere. A ratio is the readable form of the result; it is not the pin, + # because a ratio can be held constant by both arms getting worse together. + echo "-- box $box: z=$sz h=$sh z/h=$(awk -v a="$sz" -v b="$sh" 'BEGIN { printf "%.4f", (b + 0 == 0) ? 0 : a / b }')" +done + +pgc_summary diff --git a/test/pytest/test_hilbert_locality.py b/test/pytest/test_hilbert_locality.py new file mode 100644 index 00000000..f5596076 --- /dev/null +++ b/test/pytest/test_hilbert_locality.py @@ -0,0 +1,621 @@ +"""Port of test/hilbert_locality.sh: does the Hilbert curve buy range locality? + +BLOCKED ON #897. THIS FILE CANNOT RUN ON main. + +The harness it is written against -- `conftest.py`, `pgc_vacuity.py`, +`pgc_cluster.py`, `pytest.ini` -- lives only on the branch of pull request #897, +which is NOT MERGED and currently has changes requested. `test/pytest/` does not +exist on main, so on main this file has no `expect` fixture, no `pgc_cluster` +fixture and no vacuity plugin, and pytest will fail at collection. It is +committed here so that the port exists and can be reviewed beside the bash +suite; it becomes runnable when #897 lands, and not before. + +It was RUN, before being committed, against #897's harness assembled beside it +in a scratch tree (PostgreSQL 18.4, prefix /usr/local/pg18_loc889): 16 passed, +18.20s including the extension build and 5.13s on a warm one. The numbers below +are that run's, and they are the bash suite's numbers. + + pytest --pg-config .../bin/pg_config --pgc-expect-tests 16 \ + test_hilbert_locality.py + +AND THE BLOCKING DEPENDENCY IS WIDER THAN test/pytest/. #897 also adds +`pgc_build_and_install` to test/lib.sh, which `pgc_cluster.build_and_install` +drives. With this file dropped beside #897's test/pytest/ but main's lib.sh, the +session ends before any test runs: + + RuntimeError: pgcolumnar failed to build or install from ... + bash: line 1: pgc_build_and_install: command not found + +So "blocked on #897" means the whole of #897, not only the directory. + +ONE THING THIS FILE CANNOT DO THAT ITS BASH TWIN DOES, AND IT IS MEASURED + +The bash suite REFUSES to report a ratio when the two partitions are not +different: the twelve measurement arms print UNRUN, pgc_summary prints +"12 unrunnable" and the suite exits 67, INCOMPLETE. Here the same refusal goes +through `expect.cannot_run("UNMET_PRECONDITION", ...)`, and the layer records +the third state but nothing reports it. Measured, with the fixture mutated to +lay BOTH arms out with cluster(): "1 failed, 15 passed" -- the four +test_groups_read_over_sixty_placements cases PASSED while asserting nothing +about groups read. Only test_the_two_partitions_differ reddened, and it reddened +for its own reason. The suite is still red overall, so nothing ships silently; +but a reader counting greens counts four that never asked their question. See +the report accompanying this file. + +WHAT THIS IS A PORT OF, AND WHAT IT IS FOR + +test/hilbert_locality.sh builds one fixture, lays it out twice -- once with +pgcolumnar.cluster() (Z-order) and once with pgcolumnar.cluster_hilbert() -- +and counts the engine's own "Columnar Chunk Groups Read" over 60 deterministic +window placements at each of four window sizes. Read that file first: its +header carries the reasoning, the three defects the shape exists to avoid, and +the list of things the measurement does NOT claim. Every property asserted here +is asserted there, under a name that matches, so the two can be compared +mechanically. + +WHAT THIS PORT DOES NOT CLAIM + +It does not claim the two harnesses agree by construction. They agree because +both were run and both produced 241/118, 351/209, 588/402 and 1624/1313 -- and +if one day they disagree, that disagreement is the finding, not a merge +conflict to resolve by editing one of them. + +THREE MECHANISMS DIFFER FROM THE BASH ARM, ON PURPOSE + + 1. THE COUNTERS ARE READ AS TYPED JSON, NOT GREPPED. The bash suite runs + EXPLAIN in text and pulls integers out with sed. Here it is + `EXPLAIN (..., FORMAT JSON)`, which psycopg hands back as parsed Python, so + "Columnar Chunk Groups Read" is a key holding an int. A regex that matched + the wrong line, or that matched nothing and yielded the empty string, is + not a failure mode this arm has. + + 2. THE COLUMNAR-SCAN PREMISE GOES THROUGH expect.plan_marker, not a grep for + a node name. `Custom Plan Provider == "PgColumnarScan"` is NOT the same + question: the vectorized aggregate node reuses the scan's registered + methods and reports the same provider, so that predicate says yes for a + plan with no columnar scan in it. `plan_marker` asks for the + "Columnar Projected Columns" key, which only the scan's explain callback + emits -- the faithful port of `pgc_is_columnar_scan`. + + 3. THE FIXTURE IS BUILT ONCE PER MODULE, not once per test. It is 200,000 + rows and six clustering operations. See the note on `locality` below for + what that costs in isolation and why it is still the right trade. +""" + +import pytest + +# THE PRIVATE IMPORT IS DELIBERATE AND IT IS A GAP, NOT A PREFERENCE. +# +# The vacuity layer walks an EXPLAIN JSON tree in `_plan_nodes`, and exposes no +# public way to READ a counter out of one -- `plan_marker` asserts a key is +# present and `plan_node` returns a node matched by type or provider, but this +# suite's whole subject is the VALUE under "Columnar Chunk Groups Read". The +# alternatives were to reimplement the traversal (a second copy of the tree +# shape, free to drift from the layer's) or to import the layer's own. Importing +# it keeps one traversal in the tree. See the report accompanying this file: +# `expect.counter(plan, key)` belongs in the layer. +from pgc_vacuity import _plan_nodes + +# ---- the fixture's constants, identical to the bash suite -------------------- + +ROWS = 200_000 +SPAN = 100_000 + +# NOT DYADIC, ON PURPOSE. 200000/1500 is 133.33, so no group boundary lines up +# with a power of two. The dyadic case is where the two curves cut the same +# blocks, and it is kept as a control (test_dense_dyadic_grid_is_one_partition) +# rather than allowed to become the fixture by accident. +STRIPE_ROWS = 1500 +GROUPS = 134 + +PLACEMENTS = 60 +STRIDE_X = 997 +STRIDE_Y = 7919 + +# The pins: box -> (Z-order total, Hilbert total) over the 60 placements. +# Measured on main f2af080, PG 18.4, and reproduced bit-for-bit by the bash +# suite and by this one. +# +# EXACT INTEGERS AND NOT A THRESHOLD. A threshold is the thing someone lowers +# when it reddens; it survives a regression that costs half the benefit, and it +# survives it quietly. Any change to the layout, the curve, the group sizing or +# the skip logic moves one of these eight numbers and names itself. +PINS = { + 2000: (241, 118), + 5000: (351, 209), + 12000: (588, 402), + 30000: (1624, 1313), +} + +# The partition digests the pins were measured over. +DIGEST_ZORDER = "2169ae4551d8" +DIGEST_HILBERT = "1706e49ef5a2" + +DENSE_SIDE = 256 +DENSE_STRIPE_ROWS = 1024 + +# The deterministic generator. hashint8 rather than random(): the pilot ran a +# random() INSERT once per table, so the two arms held DIFFERENT DATA and the +# ratio was a fact about the data rather than about the curve. +SRC_INSERT = f""" +INSERT INTO src (a, b, pad) +SELECT (hashint8(g) % {SPAN} + {SPAN}) % {SPAN}, + (hashint8(g * 2654435761) % {SPAN} + {SPAN}) % {SPAN}, + repeat('x', 40) +FROM generate_series(1, {ROWS}) g +""" + +# THE PARTITION DIGEST, ORDER-INDEPENDENT BY CONSTRUCTION. +# +# One string per row group from BOTH columns' min and max, and then those +# strings SORTED before hashing. What is hashed is the SET of group boxes, so +# two layouts that cut the same boxes hash equal however the curve numbered +# them. Ordering the groups by group_number instead -- the obvious first +# version -- makes the digest report the NUMBERING too, so it says "different" +# for two identical partitions and the dense control below passes its premise +# and goes on to report a ratio. That is the defect this form exists to avoid. +# +# vector_index = -1 is the row-group-level zone map: the box of the whole group, +# which is what decides whether the group can be skipped. +DIGEST_SQL = """ +SELECT substr(md5(string_agg(g, ',' ORDER BY g)), 1, 12) FROM ( + SELECT string_agg(column_index::text || ':' || + encode(minimum, 'hex') || ':' || + encode(maximum, 'hex'), '/' ORDER BY column_index) AS g + FROM pgcolumnar.zone_map + WHERE storage_id = pgcolumnar.get_storage_id(%s) + AND vector_index = -1 AND column_index IN (0, 1) + GROUP BY group_number) t +""" + +# Order-blind, exactly like pgc_set_hash in test/lib.sh, and that is right here: +# the arms are supposed to differ in ORDER and in nothing else, so an +# order-sensitive oracle would report a difference for the property under test. +SET_HASH_SQL = """ +SELECT coalesce(md5(string_agg(t, chr(10) ORDER BY t)), 'EMPTY') +FROM (SELECT r::text AS t FROM {table} r) s +""" + + +# ---- helpers ---------------------------------------------------------------- + + +def _scalar(conn, sql, params=None): + with conn.cursor() as cur: + cur.execute(sql, params) + row = cur.fetchone() + return row[0] if row else None + + +def _digest(conn, table): + """The partition digest, or a sentinel that no comparison can accept. + + A helper that returned None here would let two FAILED reads compare equal + and pass an equality arm, and would satisfy an INEQUALITY arm outright -- + which matters, because the arm that licenses the whole measurement is an + inequality. `expect.hash` refuses a QUERY_ERROR sentinel outright, so the + sentinel is the layer's own. + """ + value = _scalar(conn, DIGEST_SQL, (table,)) + return value if value else f"QUERY_ERROR.no-partition-for-{table}" + + +def _verdict(a, b): + """different | IDENTICAL | UNMEASURED[...] -- the bash suite's differs(). + + THE LAYER HAS NO INEQUALITY RECORDER. `expect` compares for equality + (num, text, hash, rows), asserts a floor (at_least), and asserts a plan key. + "these two differ" has to become "the verdict about these two equals the + string 'different'", which is what this function is for. Computing the + verdict here rather than writing `assert a != b` is not decoration: a bare + Python assert is not COUNTED by the vacuity layer, so a test that concluded + only that way would be failed for asserting nothing. + """ + for label, value in (("a", a), ("b", b)): + if not value or str(value).startswith("QUERY_ERROR") or value == "EMPTY": + return f"UNMEASURED[{label}={value}]" + return "different" if a != b else "IDENTICAL" + + +def _plan(conn, sql): + """The ANALYZEd plan as parsed JSON.""" + return _scalar( + conn, f"EXPLAIN (ANALYZE, TIMING OFF, COSTS OFF, FORMAT JSON) {sql}") + + +def _counter(plan, key): + """One Columnar counter out of a plan, or None when no node carries it. + + None rather than 0: `expect.num` refuses a non-number, so a counter that was + never reported fails the arm instead of being summed as nothing. + """ + for node in _plan_nodes(plan): + if key in node: + return node[key] + return None + + +def _window(table, ox, oy, box): + """The query under measurement. One shape, one place, so the arms and the + premises cannot drift into asking about two different queries.""" + return (f"SELECT count(*) FROM {table} " + f"WHERE a BETWEEN {ox} AND {ox + box} " + f"AND b BETWEEN {oy} AND {oy + box}") + + +def _origins(box): + """The 60 placements. Two coprime strides over the span, so the origins + neither repeat nor march in step with the group boundaries, and the same 60 + are used for both arms and every box.""" + span = SPAN - box + return [((i * STRIDE_X) % span, (i * STRIDE_Y) % span) + for i in range(1, PLACEMENTS + 1)] + + +def _groups_read(conn, table, box): + """(placements measured, groups read) over the 60 windows. + + BOTH numbers are returned. The sum alone cannot distinguish a genuinely + small number of groups from a run that measured fewer windows than it was + asked to -- and a truncated run reads as a BETTER result, which is the + direction in which a defect here would go unnoticed. + """ + measured, total = 0, 0 + for ox, oy in _origins(box): + value = _counter(_plan(conn, _window(table, ox, oy, box)), + "Columnar Chunk Groups Read") + if isinstance(value, int): + measured += 1 + total += value + return measured, total + + +# ---- the fixture ------------------------------------------------------------ + + +@pytest.fixture(scope="module") +def locality(pgc_cluster): + """The whole fixture, built once for the module. + + MODULE SCOPE, NOT THE `pgc_conn` FIXTURE, AND THAT IS A COMPROMISE. This + fixture is 200,000 rows and six clustering operations; per test it would be + paid fifteen times over. `pgc_conn` is function-scoped -- it gives every + test a private schema, which is the isolation this harness is built on -- + and a module-scoped fixture cannot depend on a function-scoped one, so this + opens its own connection and makes its own schema by hand, duplicating what + `pgc_conn` does. See the report accompanying this file: a module- or + session-scoped sibling of `pgc_conn` is the missing piece. + + SET max_parallel_workers_per_gather = 0, ON THIS CONNECTION, IS NOT TUNING. + Every digest below is a claim about physical group boundaries and every + count is read out of a plan; a parallel plan divides that work across + workers, so parallelism off is what makes the counters a fact about the + layout rather than about scheduling. The bash suite says this once, in the + server's config file, through PGC_EXTRA_CONF. `pgc_cluster.py` hard-codes + its postgresql.conf and has no equivalent hook, so it is a SET on the one + connection every query in this module runs through -- which is why the + module hands out a single connection rather than one per test. + """ + import psycopg + + conn = psycopg.connect(pgc_cluster.dsn(), autocommit=True) + schema = "pgc_hilbert_locality" + try: + conn.execute(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE') + conn.execute(f'CREATE SCHEMA "{schema}"') + conn.execute(f'SET search_path TO "{schema}", public') + conn.execute("SET max_parallel_workers_per_gather = 0") + + # The data is materialised ONCE, into a heap table, and both arms load + # from it. Two independent evaluations of the generator is how the arms + # come to hold different rows. + conn.execute("CREATE TABLE src (a int, b int, pad text)") + conn.execute(SRC_INSERT) + + for table in ("hz", "hh", "cz1", "cz2"): + conn.execute( + f"CREATE TABLE {table} (a int, b int, pad text) USING pgcolumnar") + conn.execute("SELECT pgcolumnar.set_options(%s, stripe_row_limit => %s)", + (table, STRIPE_ROWS)) + conn.execute(f"INSERT INTO {table} SELECT * FROM src") + + # hz is Z-order, hh is Hilbert. cz1 and cz2 are the control: the same + # verb twice, so an "the partitions differ" arm cannot be satisfied by + # an instrument that reports any two tables as different. + conn.execute("SELECT pgcolumnar.cluster('hz', 'a', 'b')") + conn.execute("SELECT pgcolumnar.cluster_hilbert('hh', 'a', 'b')") + conn.execute("SELECT pgcolumnar.cluster('cz1', 'a', 'b')") + conn.execute("SELECT pgcolumnar.cluster('cz2', 'a', 'b')") + + # The dense dyadic grid the design predicted the two curves would agree + # on: 256 x 256 cells, 1024 rows to a group, 64 full groups. + conn.execute("CREATE TABLE dsrc (a int, b int)") + conn.execute( + f"INSERT INTO dsrc SELECT (g - 1) / {DENSE_SIDE}, (g - 1) % {DENSE_SIDE} " + f"FROM generate_series(1, {DENSE_SIDE * DENSE_SIDE}) g") + for table in ("dz", "dh"): + conn.execute(f"CREATE TABLE {table} (a int, b int) USING pgcolumnar") + conn.execute("SELECT pgcolumnar.set_options(%s, stripe_row_limit => %s)", + (table, DENSE_STRIPE_ROWS)) + conn.execute(f"INSERT INTO {table} SELECT * FROM dsrc") + conn.execute("SELECT pgcolumnar.cluster('dz', 'a', 'b')") + conn.execute("SELECT pgcolumnar.cluster_hilbert('dh', 'a', 'b')") + + yield conn + finally: + try: + conn.execute(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE') + finally: + conn.close() + + +@pytest.fixture(scope="module") +def digests(locality): + """The two partitions under measurement, read once.""" + return _digest(locality, "hz"), _digest(locality, "hh") + + +# ============================================================================= +# ARM 1 PREMISE: THE TWO ARMS HOLD THE IDENTICAL ROW MULTISET +# ============================================================================= + + +def test_both_arms_hold_the_identical_row_multiset(locality, expect): + """bash: 'premise: the two arms hold the identical row multiset' and the + two arms that follow it. + + Without this every number below is a fact about two different tables. + """ + hz = _scalar(locality, SET_HASH_SQL.format(table="hz")) + hh = _scalar(locality, SET_HASH_SQL.format(table="hh")) + src = _scalar(locality, SET_HASH_SQL.format(table="src")) + expect.hash(hh, hz, "the two arms hold the identical row multiset") + # And that multiset is the source's. Comparing the arms only to each other + # passes if both loads went equally wrong. + expect.hash(hz, src, "and it is the source's multiset, so neither load dropped rows") + # A hash of nothing equals a hash of nothing. 'EMPTY' is what the oracle + # returns for a genuinely empty relation, and it is not what these are. + expect.at_least(_scalar(locality, "SELECT count(*) FROM hz"), ROWS, + "and it is a hash of rows: the Z-order arm holds them") + expect.at_least(_scalar(locality, "SELECT count(*) FROM hh"), ROWS, + "and it is a hash of rows: the Hilbert arm holds them") + + +def test_the_fixture_is_two_dimensional(locality, expect): + """bash: 'premise: column a spans the square' and its sibling. + + A generator that collapsed one column to a handful of values would make + both curves the identity in that dimension and the comparison meaningless, + and it would do so silently. + """ + for column in ("a", "b"): + expect.num( + _scalar(locality, + f"SELECT (min({column}) < {SPAN // 100} " + f"AND max({column}) > {SPAN - SPAN // 100})::int FROM src"), + 1, f"column {column} spans the square") + + +def test_both_arms_have_the_group_count_measured(locality, expect): + """bash: 'premise: hz has the group count this measurement was taken at'. + + The fixture is only the fixture that was measured if the groups are the + size they were measured at: 200,000 rows at 1,500 to a group is 134, the + last one short. + """ + counts = {} + for table in ("hz", "hh"): + counts[table] = _scalar( + locality, + "SELECT count(*) FROM pgcolumnar.row_group " + "WHERE storage_id = pgcolumnar.get_storage_id(%s)", + (table,)) + expect.num(counts["hz"], GROUPS, "hz has the group count measured") + expect.num(counts["hh"], counts["hz"], + "hh has the same group count, so a group is the same unit on both arms") + + +# ============================================================================= +# ARM 2 PREMISE: THE TWO PARTITIONS ARE TWO DIFFERENT PARTITIONS +# ============================================================================= + + +def test_the_two_partitions_differ(digests, expect): + """bash: 'premise: the Z-order and Hilbert partitions differ', and the two + pins under it. + + This is what licenses the measurement to attribute its difference to the + curve. Pinned as well as merely different: "different" is satisfied by any + change to either layout, while these two values say the layouts are the + ones the pinned integers were measured over. A moved digest beside moved + integers is a layout change; moved integers beside these digests would be a + change in the reader. + """ + dz, dh = digests + expect.text(_verdict(dz, dh), "different", + "the Z-order and Hilbert partitions differ") + expect.text(dz, DIGEST_ZORDER, + "the Z-order partition is the one these numbers were measured over") + expect.text(dh, DIGEST_HILBERT, + "the Hilbert partition is the one these numbers were measured over") + + +# ============================================================================= +# ARM 3 CONTROL: SAME CURVE TWICE -> THE DIGESTS ARE IDENTICAL +# ============================================================================= + + +def test_two_tables_on_the_same_curve_are_one_partition(locality, digests, expect): + """bash: 'control: two tables on the same curve have the identical partition'. + + The removal proof for the arm above. An inequality passes for any + instrument that reports "different" too readily -- including one that is + really reporting "these are two different tables". Two tables built the + same way from the same source and laid out by the SAME verb must hash + EQUAL, and that hash must be the measured Z-order arm's, so the control is + a control on THIS fixture rather than on an unrelated one. + """ + dz, _ = digests + cz1 = _digest(locality, "cz1") + cz2 = _digest(locality, "cz2") + expect.hash(cz1, cz2, "two tables on the same curve have the identical partition") + expect.hash(cz1, dz, "and that partition is the measured Z-order arm's") + + +# ============================================================================= +# ARM 4 CONTROL: A DENSE DYADIC GRID -> THE TWO CURVES AGREE +# ============================================================================= + + +def test_dense_dyadic_grid_is_one_partition(locality, expect): + """bash: 'control: on a dense dyadic grid the two curves cut the identical + partition'. + + The degenerate case the design predicted: over a dense grid whose side is a + power of two, cut into groups whose size is a power of two, the two curves + visit the same blocks in a different ORDER and produce the same set of + group boxes. It is here so the suite carries its own counterexample to + "Hilbert always changes the layout", and so a reader can see that arm 2's + "different" is a measurement rather than a foregone conclusion. + """ + expect.num( + _scalar(locality, + "SELECT count(*) FROM (SELECT a, b FROM dsrc " + "GROUP BY a, b HAVING count(*) <> 1) x"), + 0, "the dense grid is dense -- every cell present exactly once") + expect.num( + _scalar(locality, + "SELECT count(*) FROM pgcolumnar.row_group " + "WHERE storage_id = pgcolumnar.get_storage_id('dz')"), + DENSE_SIDE * DENSE_SIDE // DENSE_STRIPE_ROWS, + "the dense grid has the dyadic group count") + expect.hash(_digest(locality, "dz"), _digest(locality, "dh"), + "on a dense dyadic grid the two curves cut the identical partition") + + +# ============================================================================= +# ARM 5 PREMISE: BOTH ARMS PLAN AS A COLUMNAR SCAN +# ============================================================================= + + +def test_both_arms_plan_as_a_columnar_scan(locality, expect): + """bash: 'premise: the Z-order arm plans as a columnar scan'. + + plan_marker, not a provider name. The vectorized aggregate node reuses the + scan's registered methods and reports the same Custom Plan Provider, so + that predicate says yes for a plan with no columnar scan in it. + "Columnar Projected Columns" is emitted only by the scan's explain + callback. + """ + box = 2000 + ox, oy = _origins(box)[0] + for table in ("hz", "hh"): + expect.plan_marker(_plan(locality, _window(table, ox, oy, box)), + "Columnar Projected Columns", + name=f"the {table} arm plans as a columnar scan") + + +def test_parallelism_is_off_so_a_counter_is_a_fact_about_the_layout(locality, expect): + """No bash twin by name: the bash suite sets this in the server's config + through PGC_EXTRA_CONF, where it cannot be undone by a session. + + Here it is a SET on one connection, so it is asserted rather than assumed. + A parallel plan divides the scan across workers and the counters below + would be a fact about scheduling. + """ + expect.text(_scalar(locality, "SHOW max_parallel_workers_per_gather"), "0", + "parallelism is off on the connection every arm runs through") + + +# ============================================================================= +# ARMS 6-7 PREMISES ABOUT THE COUNTERS' OWN PLAN +# ============================================================================= + + +@pytest.mark.parametrize("box", sorted(PINS)) +def test_the_predicates_are_usable_and_the_denominators_match(locality, expect, box): + """bash: 'premise: box N, the Z-order arm can skip on all four predicates' + and the two arms after it. + + FOUR, NOT TWO. A BETWEEN is two scan keys, >= and <=, and the reader builds + a skip predicate from each; two BETWEENs over two columns are four usable + predicates. A "> 0" bound would accept an arm that had lost one whole + dimension -- which is the dimension the curve is about. + + "Pushed-Down Filters" is deliberately not the number checked: it counts what + the reader was HANDED, and a key it could not build a skip predicate from + excludes no group at all while still being reported as pushed down (#477). + + And the denominators are compared to EACH OTHER, not to a literal: a + literal would still be satisfied if both arms drifted together, and the + property is that a ratio is taken between two equal denominators. + """ + ox, oy = _origins(box)[0] + pz = _plan(locality, _window("hz", ox, oy, box)) + ph = _plan(locality, _window("hh", ox, oy, box)) + + expect.num(_counter(pz, "Columnar Usable Skip Predicates"), 4, + f"box {box}, the Z-order arm can skip on all four predicates") + expect.num(_counter(ph, "Columnar Usable Skip Predicates"), 4, + f"box {box}, the Hilbert arm can skip on all four predicates") + expect.num(_counter(ph, "Columnar Chunk Groups Total"), + _counter(pz, "Columnar Chunk Groups Total"), + f"box {box}, both arms have the same number of groups to read") + expect.num(_counter(pz, "Columnar Chunk Groups Total"), GROUPS, + f"box {box}, that denominator is the whole relation") + + +# ============================================================================= +# ARM 8 THE MEASUREMENT +# ============================================================================= + + +@pytest.mark.parametrize("box", sorted(PINS)) +def test_groups_read_over_sixty_placements(locality, digests, expect, box): + """bash: 'box N: groups read over 60 placements, Z-order' and the three + arms beside it. + + Reported only if the partitions differ. With no difference in layout there + is nothing for a ratio to be about, and printing one anyway is how the + pilot produced numbers for the dense grid. + """ + dz, dh = digests + verdict = _verdict(dz, dh) + if verdict != "different": + expect.cannot_run( + "UNMET_PRECONDITION", + f"the two partitions are not different ({verdict}), so a ratio " + f"between them is not about the curve") + return + + want_z, want_h = PINS[box] + measured_z, total_z = _groups_read(locality, "hz", box) + measured_h, total_h = _groups_read(locality, "hh", box) + + # EVERY PLACEMENT WAS MEASURED. A run that lost windows would sum fewer + # groups and read as a better result on whichever arm lost them. + expect.num(measured_z, PLACEMENTS, + f"box {box}: all {PLACEMENTS} placements reported a Z-order group count") + expect.num(measured_h, PLACEMENTS, + f"box {box}: all {PLACEMENTS} placements reported a Hilbert group count") + + expect.num(total_z, want_z, + f"box {box}: groups read over {PLACEMENTS} placements, Z-order") + expect.num(total_h, want_h, + f"box {box}: groups read over {PLACEMENTS} placements, Hilbert") + + # And, separately from the pins: the curve still wins here. This arm stays + # meaningful when the pins are re-taken, and it is what tells a reader + # "the layout moved" apart from "Hilbert stopped winning". + # + # A DIFFERENCE, BECAUSE THE LAYER HAS NO STRICT INEQUALITY. `at_least` is + # the only bound it offers and it refuses a floor of zero, so "h < z" is + # written as "z - h is at least 1", which asserts exactly that and nothing + # weaker. + expect.at_least(total_z - total_h, 1, + f"box {box}: Hilbert reads fewer groups than Z-order") + + # The ratio is PRINTED, from the two numbers just measured, and asserted + # nowhere: a ratio can be held constant by both arms getting worse together. + print(f"-- box {box}: z={total_z} h={total_h} " + f"z/h={total_z / total_h:.4f}") From 0a5cc09a1a921306e258d4ccb622eb9466be9fc3 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 9 Sep 2026 11:15:51 -0600 Subject: [PATCH 2/8] test: say what each arm of the locality suite catches, and pin the margin (#889) The eight pinned integers were presented as the arm the suite exists for. Four mutations against them say otherwise, and one of them reddened a rule the header told the reader to follow. WHAT MOVED, MEASURED ON PG 18.4, PREFIX /usr/local/pg18_fix889 Baseline, reproduced from a clean tree twice: 65 passed + 0 failed + 0 unrunnable, digests 2169ae4551d8 and 1706e49ef5a2, pins 241:118, 351:209, 588:402, 1624:1313, z/h 2.0424, 1.6794, 1.4627, 1.2369. The pytest twin, run against #897 head 5f3dedb in a scratch worktree, produced the same eight numbers: 18 passed. A CHANGED CURVE is caught by arm 2's digest pins, not by the integers. A point reflection inside cluster_hilbert_transpose gave digest e64e00017c5a and h=117/202/403/1301; a swap of the clustering axes before the transpose gave b858f6a8a300 and h=130/208/409/1316. Both reddened the digest pin on the same run as the integers, two hundred lines upstream of them. A CHANGED READER, AT AN UNCHANGED LAYOUT, is what only the integers catch. Refusing to skip odd-numbered row groups in src/columnar_reader.c left both digests exactly at their pins and arms 1 and 3-7 green, and moved all eight integers: z=4144/4196/4317/4848, h=4083/4133/4228/4678. AND IT KEPT h < z GREEN AT EVERY BOX while z/h fell from 2.0424 to 1.0149 -- Hilbert winning by 61 groups out of 4,144, reported as PASS. The header's rule that "the pins moved but h < z still holds" means a benign layout change was therefore false. It is corrected, and a per-box margin floor is asserted beside the pins: z/h at least 1.80, 1.48, 1.28, 1.10, about 88% of the measured ratio. THE GROSS CASE, the transpose gutted so cluster_hilbert() lays Z-order, is caught by arm 2 alone: 39 passed + 2 failed + 16 unrunnable = 57, both failures arm 2's, and nm -S reported the gutted function at 5 bytes in the installed .so. THE ONE HOLE FIXED The arm "control: and that partition is the measured Z-order arm's" was the only digest comparison in the file not routed through differs(). check_text refuses an empty expectation but NO_PARTITION is not empty, so two failed reads compared equal and passed. REMOVAL PROOFS - The margin floor: under the reader mutation all four floor arms report BELOW THE FLOOR (z/h=1.0149, 1.0152, 1.0211, 1.0363) while h < z passes at every box. Under the two valid curve variants the floors stay green and only the pins red, so the two arms say different things about one run. - The differs() fix: with partition_digest() pointed at a storage_id that does not exist, the arm goes from PASS on the old text to "got [UNMEASURED[a=NO_PARTITION]] want [IDENTICAL]" on the new one. Same mutation, one arm flipped: 36 passed + 5 failed before, 35 passed + 6 failed after. - The refusal: with the transpose gutted, sixteen UNRUN lines and "39 passed + 2 failed + 16 unrunnable = 57"; with both arms loaded FROM src OFFSET 1, the same sixteen refusals and "35 passed + 6 failed + 16 unrunnable = 57". THE PYTEST TWIN Its assertions now carry the bash check names verbatim, prefixes included, and nested calls are hoisted out of the expect() arguments so compare_to_bash.py can read them: 27 names missing before, 13 after, and all 13 are accounted for in the docstring -- eleven interpolate a shell variable, two are lost to the comparator's own regex, which takes the first string literal in the call. Four properties the port did not carry are added: the exact source row count, the exact 400,000-row sum in place of two at_least floors that an arm loaded twice would satisfy, a named premise per layout verb (the port of crun), and an empty-relation sentinel shaped like QUERY_ERROR. Measured: the old oracle's 'EMPTY' passes expect.hash on two genuinely empty relations; the new one is refused as "the left side is a failed query". Two gaps are recorded rather than papered over, both #897's to close. expect.cannot_run makes a test PASS, because pytest_runtest_call reads only rec.count -- with both arms laid Z-order the twin reports "1 failed, 17 passed" where bash reports sixteen unrunnable. And plan_marker has no removal proof: replacing its present-arm raise with pass leaves 50 tests green. Deliberately still not registered in test/run_all_versions.sh. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK --- test/hilbert_locality.sh | 155 ++++++++++-- test/pytest/test_hilbert_locality.py | 348 ++++++++++++++++++++------- 2 files changed, 396 insertions(+), 107 deletions(-) diff --git a/test/hilbert_locality.sh b/test/hilbert_locality.sh index 7484497d..dde3d51a 100755 --- a/test/hilbert_locality.sh +++ b/test/hilbert_locality.sh @@ -18,22 +18,76 @@ # "Columnar Chunk Groups Read" over 60 window placements at each of four window # sizes. The counts are PINNED AS EXACT INTEGERS. # -# WHY EXACT INTEGERS AND NOT A THRESHOLD +# WHY EXACT INTEGERS AND NOT A THRESHOLD -- AND WHAT EACH INSTRUMENT CATCHES # # A threshold is the thing someone lowers when it reddens. "Hilbert reads at # most 0.6x the groups Z-order does" survives a regression that costs half the -# benefit, and it survives it silently. An exact pin cannot: any change to the -# layout, to the curve, to the group sizing or to the skip logic moves one of -# these eight numbers and the suite says which. The pins are the measurement, -# not a target -- if one moves, the correct response is to find out what moved -# it and then, if the new layout is better, re-pin with the new numbers beside -# the reason. -# -# Alongside the pins, and separately, h < z is asserted AT EVERY BOX SIZE. That -# split is deliberate. If the pins move but h < z still holds, the layout -# changed. If h < z fails, HILBERT STOPPED WINNING, which is a different and -# much worse fact, and a reader must be able to tell the two apart from the -# output without re-deriving anything. +# benefit, and it survives it silently. An exact pin cannot be lowered without +# saying so, and it names which number moved. +# +# What an exact pin is NOT is automatically the sensitive instrument in this +# file. The eight integers were mutation-tested to find out what they, and only +# they, catch. Measured, not assumed: +# +# - A CHANGED CURVE is caught by the DIGEST PINS in arm 2, about two hundred +# lines above the integers, and the integers add nothing to it. Two +# independently built, valid Hilbert re-orientations -- a point reflection +# inside cluster_hilbert_transpose, and a swap of the clustering axes +# before the transpose -- each moved the Hilbert digest AND all four +# Hilbert integers on the same run (h=117/202/403/1301 and +# h=130/208/409/1316 against pins of 118/209/402/1313). That is structural +# rather than a lucky choice of mutation: the counts are a deterministic +# function of the group boxes, which the digest hashes, of the 60 fixed +# origins, and of the skip logic. So for a changed curve the integers are a +# strictly weaker copy of a check the suite already makes upstream. +# Re-pinning arm 2 and arm 8 together after a deliberate curve change is +# right, but the digest is the arm that did the work. +# - A CHANGED READER, AT AN UNCHANGED LAYOUT, is what only the integers +# catch. It is their own domain and it is why they are here. Refusing to +# skip odd-numbered row groups in src/columnar_reader.c left BOTH digests +# exactly at their pins and arms 1 and 3-7 green, and moved all eight +# integers (z=4144/4196/4317/4848, h=4083/4133/4228/4678). +# - A CHANGED ROW MULTISET is NAMED by arm 1 and by nothing else. Loading the +# Hilbert arm from a differently seeded 200,000-row source over the same +# square moved the Hilbert digest, all four Hilbert integers (220/330/567/ +# 1603) and all four margins -- but not one of those arms says WHY, and a +# reader looking at moved integers would go hunting in the curve. Arm 1 is +# what says the two arms no longer hold the same rows. Its SECOND check is +# the one that survives the case where both loads went equally wrong: +# loading both arms with OFFSET 1 keeps "the identical row multiset" green +# and reddens "it is the source's multiset" and "both arms hold every +# source row" (399998 against 400000), while the measurement below refuses +# and the integers say nothing at all. So none of the three instruments is +# redundant to the others, and a reader who drops arm 1 or the digest pins +# as "already covered by the pins" is not covered. +# - THE GROSS CASE -- the transpose gutted, so cluster_hilbert() silently +# lays Z-order -- is caught by arm 2 alone. The integers never execute: +# arm 8 refuses the measurement and prints sixteen UNRUN lines. A wall of +# UNMET_PRECONDITION here means the curve collapsed, not that the suite +# broke. +# +# The pins are the measurement, not a target -- if one moves, the correct +# response is to find out what moved it and then, if the new layout is better, +# re-pin with the new numbers beside the reason. +# +# WHY h < z IS NOT ENOUGH, AND WHAT THE MARGIN FLOOR IS FOR +# +# Alongside the pins, and separately, h < z is asserted AT EVERY BOX SIZE, and +# so is the MARGIN. An earlier version of this header told the reader that "the +# pins moved but h < z still holds" means the layout merely changed. THAT RULE +# WAS FALSE and it was reddened. The reader mutation above moves all eight +# integers, keeps h < z PASSING at every box, and takes z/h from 2.0424 to +# 1.0149 at box 2000: Hilbert won by 61 groups out of 4,144 and the suite +# printed PASS. A maintainer following the old rule would have re-pinned and +# accepted a change that took the feature's benefit from 2.04x to 1.01x. +# +# So h < z says only that Hilbert was not BEATEN. The per-box floor beside it +# says Hilbert still wins by the margin these numbers were measured at. Read +# the three arms together: +# +# pins move, margin holds the layout changed; find what moved it, re-pin. +# margin fails most of the benefit is gone, whatever h < z says. +# h < z fails HILBERT STOPPED WINNING, the worst of the three. # # WHAT THIS SUITE DOES NOT CLAIM # @@ -377,7 +431,15 @@ check_text "control: two tables on the same curve have the identical partition" "$(differs "$CZ1" "$CZ2")" "IDENTICAL" # And it is the same partition the measured Z-order arm has, so the control is # a control ON THIS FIXTURE rather than on an unrelated one. -check_text "control: and that partition is the measured Z-order arm's" "$CZ1" "$DZ" +# +# THROUGH differs(), LIKE EVERY OTHER DIGEST COMPARISON IN THIS FILE. Comparing +# $CZ1 with $DZ directly was the one exception, and the exception was a hole: +# check_text refuses an EMPTY expectation, but NO_PARTITION is not empty, so two +# FAILED digest reads compared equal and this arm printed PASS. Measured: with +# partition_digest() pointed at a storage_id that does not exist, every other +# digest arm reddened with UNMEASURED[a=NO_PARTITION] and this one was green. +check_text "control: and that partition is the measured Z-order arm's" \ + "$(differs "$CZ1" "$DZ")" "IDENTICAL" # ============================================================================= # ARM 4 CONTROL: A DENSE DYADIC GRID -> THE TWO CURVES AGREE @@ -475,17 +537,38 @@ done # header describes: with no difference in layout there is nothing for a ratio to # be about, and printing one anyway is how the pilot's first version produced # numbers for the dense grid. +# +# AND THE REFUSAL IS WHAT ANSWERS THE GROSS CASE. With the Hilbert transpose +# gutted, so that cluster_hilbert() lays Z-order, arm 2 reddens twice and every +# check below refuses: sixteen UNRUN lines and "39 passed + 2 failed + 16 +# unrunnable = 57", the run exiting 1 because a failure outranks an incomplete. +# A reader who sees that wall of UNMET_PRECONDITION is looking at a collapsed +# curve, not at a broken suite, and the two failures naming it are arm 2's. CURVE_DIFFERS="$(differs "$DZ" "$DH")" -# The pins. Measured on main f2af080, PG 18.4, and identical on two consecutive -# runs of the fixture above. box:zorder:hilbert. -PINS="2000:241:118 5000:351:209 12000:588:402 30000:1624:1313" +# The pins, and the margin floor beside each one. Measured on main f2af080, +# PG 18.4, identical on two consecutive runs of the fixture above, and +# reproduced from a clean tree on 2026-09-09. box:zorder:hilbert:floor. +# +# THE FLOOR IS NOT THE THRESHOLD THIS FILE'S HEADER REJECTS. The exact integers +# are still the primary detector; the floor is deliberately slack -- about 88% +# of the measured ratio -- so that it names a COLLAPSE of the benefit rather +# than tracking a layout that moved. It is here because h < z on its own is +# satisfied by a win of one group in four thousand, which is what a reader-only +# regression produces. Measured z/h at these pins: 2.0424, 1.6794, 1.4627, +# 1.2369. +# +# The slack was chosen against both mutation families and both are recorded so +# the next person can re-derive it. The reader regression takes z/h to 1.0149, +# 1.0152, 1.0211 and 1.0363, which is below every floor. The two valid curve +# re-orientations keep it at 1.8538 to 2.0598, 1.6875 to 1.7376, 1.4376 to +# 1.4590 and 1.2340 to 1.2483, which is above every floor -- so a layout that +# genuinely moved reddens the PINS and leaves this arm green, and the two arms +# say different things about the same run. +PINS="2000:241:118:1.80 5000:351:209:1.48 12000:588:402:1.28 30000:1624:1313:1.10" for pin in $PINS; do - box="${pin%%:*}" - rest="${pin#*:}" - want_z="${rest%%:*}" - want_h="${rest##*:}" + IFS=: read -r box want_z want_h floor <<<"$pin" if [ "$CURVE_DIFFERS" != different ]; then check_unrunnable "box $box: groups read, Z-order" UNMET_PRECONDITION \ @@ -494,6 +577,9 @@ for pin in $PINS; do "the two partitions are not different ($CURVE_DIFFERS), so a ratio between them is not about the curve" check_unrunnable "box $box: Hilbert reads fewer groups than Z-order" UNMET_PRECONDITION \ "the two partitions are not different ($CURVE_DIFFERS)" + check_unrunnable "box $box: and it wins by the margin measured, z/h at least $floor" \ + UNMET_PRECONDITION \ + "the two partitions are not different ($CURVE_DIFFERS)" continue fi @@ -517,10 +603,29 @@ for pin in $PINS; do "$(awk -v a="$sh" -v b="$sz" 'BEGIN { print (a + 0 < b + 0) ? "fewer" : "NOT FEWER" }')" \ "fewer" - # The ratio is PRINTED, from the two numbers just measured, and asserted - # nowhere. A ratio is the readable form of the result; it is not the pin, - # because a ratio can be held constant by both arms getting worse together. - echo "-- box $box: z=$sz h=$sh z/h=$(awk -v a="$sz" -v b="$sh" 'BEGIN { printf "%.4f", (b + 0 == 0) ? 0 : a / b }')" + # AND IT STILL WINS BY THE MARGIN IT WAS MEASURED AT. + # + # h < z above is satisfied by a win of ONE group, so on its own it cannot + # tell "the layout changed" from "the benefit is gone" -- which is what this + # file's header used to tell a reader it could. Refusing to skip + # odd-numbered row groups in the reader keeps h < z green at every box, moves + # all eight pins, and takes z/h from 2.0424 to 1.0149. This arm is what + # reddens there, and it is the removal proof for the h < z arm's meaning. + # + # The ratio is computed ONCE, here, and both this arm and the line printed + # below use that one value, so the number asserted and the number reported + # cannot drift. A zero Hilbert total yields 0 and fails the floor rather than + # dividing by zero. + ratio="$(awk -v a="$sz" -v b="$sh" 'BEGIN { printf "%.4f", (b + 0 == 0) ? 0 : a / b }')" + check_text "box $box: and it wins by the margin measured, z/h at least $floor" \ + "$(awk -v r="$ratio" -v f="$floor" \ + 'BEGIN { print (r + 0 >= f + 0) ? "at or above the floor" : "BELOW THE FLOOR (z/h=" r ")" }')" \ + "at or above the floor" + + # The ratio is PRINTED as well, and asserted nowhere beyond the floor above. + # A ratio is the readable form of the result; it is not the pin, because a + # ratio can be held constant by both arms getting worse together. + echo "-- box $box: z=$sz h=$sh z/h=$ratio" done pgc_summary diff --git a/test/pytest/test_hilbert_locality.py b/test/pytest/test_hilbert_locality.py index f5596076..9824b41c 100644 --- a/test/pytest/test_hilbert_locality.py +++ b/test/pytest/test_hilbert_locality.py @@ -10,12 +10,13 @@ committed here so that the port exists and can be reviewed beside the bash suite; it becomes runnable when #897 lands, and not before. -It was RUN, before being committed, against #897's harness assembled beside it -in a scratch tree (PostgreSQL 18.4, prefix /usr/local/pg18_loc889): 16 passed, -18.20s including the extension build and 5.13s on a warm one. The numbers below -are that run's, and they are the bash suite's numbers. +It was RUN against #897 head 5f3dedb, which merges cleanly with this branch, +in a scratch worktree on PostgreSQL 18.4: 18 passed, 3.20s on a warm tree. The eight pins below came out +identical to the bash suite's, from a different harness and a different way of +reading the counters -- 241/118, 351/209, 588/402, 1624/1313, and z/h of +2.0424, 1.6794, 1.4627, 1.2369. - pytest --pg-config .../bin/pg_config --pgc-expect-tests 16 \ + pytest --pg-config .../bin/pg_config --pgc-expect-tests 18 \ test_hilbert_locality.py AND THE BLOCKING DEPENDENCY IS WIDER THAN test/pytest/. #897 also adds @@ -31,16 +32,25 @@ ONE THING THIS FILE CANNOT DO THAT ITS BASH TWIN DOES, AND IT IS MEASURED The bash suite REFUSES to report a ratio when the two partitions are not -different: the twelve measurement arms print UNRUN, pgc_summary prints -"12 unrunnable" and the suite exits 67, INCOMPLETE. Here the same refusal goes +different: the sixteen measurement arms print UNRUN and pgc_summary counts +them as a third state, "16 unrunnable"; with nothing else red the suite exits +67, INCOMPLETE. Here the same refusal goes through `expect.cannot_run("UNMET_PRECONDITION", ...)`, and the layer records -the third state but nothing reports it. Measured, with the fixture mutated to -lay BOTH arms out with cluster(): "1 failed, 15 passed" -- the four -test_groups_read_over_sixty_placements cases PASSED while asserting nothing -about groups read. Only test_the_two_partitions_differ reddened, and it reddened -for its own reason. The suite is still red overall, so nothing ships silently; -but a reader counting greens counts four that never asked their question. See -the report accompanying this file. +the third state but nothing reports it. The mechanism is one line: +`cannot_run` sets `self.unrunnable` and calls `self._counted()`, and +`pytest_runtest_call` asks only whether `rec.count == 0` -- so DECLARING A TEST +UNRUNNABLE MAKES IT PASS. Measured against that head, with the fixture +mutated to lay BOTH arms out with cluster(): "1 failed, 17 passed", and all +four test_groups_read_over_sixty_placements cases were among the greens while +asserting nothing about groups read. Only test_the_two_partitions_differ +reddened, and it reddened for its own reason. + +The suite is still red overall, so nothing ships silently; but a reader +counting greens counts four that never asked their question, and DO NOT READ +THIS FILE AS CARRYING THE BASH SUITE'S REFUSAL until the layer converts an +unrunnable record into a non-pass outcome and a non-zero session exit -- the +pytest equivalent of PGC_EXIT_INCOMPLETE=67. That is a change to #897, not to +this file. WHAT THIS IS A PORT OF, AND WHAT IT IS FOR @@ -48,10 +58,38 @@ pgcolumnar.cluster() (Z-order) and once with pgcolumnar.cluster_hilbert() -- and counts the engine's own "Columnar Chunk Groups Read" over 60 deterministic window placements at each of four window sizes. Read that file first: its -header carries the reasoning, the three defects the shape exists to avoid, and -the list of things the measurement does NOT claim. Every property asserted here -is asserted there, under a name that matches, so the two can be compared -mechanically. +header carries the reasoning, the three defects the shape exists to avoid, the +account of which arm catches which regression, and the list of things the +measurement does NOT claim. + +HOW FAITHFUL THE PORT IS, MEASURED RATHER THAN CLAIMED + +Every assertion here carries the bash check's name string verbatim, prefixes +included, so `test/pytest/compare_to_bash.py` can diff the two by property. RUN +ON THIS PAIR IT STILL EXITS 1: 30 bash checks, 30 distinct names on this side, +13 reported missing. None of the 13 is a property this file fails to assert, +and both reasons are worth knowing before trusting the comparator: + + 1. ELEVEN OF THE THIRTEEN INTERPOLATE A SHELL VARIABLE. The comparator reads + SOURCE literals, so bash's "box $box: ..." can never equal this file's + f-string "box {box}: ...", and crun's one literal, "premise: $1 ran + without raising", stands for six runtime names. The names the two + harnesses PRINT are identical; the source literals cannot be. + 2. THE OTHER TWO ARE THE COMPARATOR'S OWN EXTRACTOR. It is a regular + expression that takes the FIRST string literal in an expect() call and + stops at the first closing parenthesis. For expect.text(got, WANT, NAME) + the first literal is the expected TEXT, so the comparator reports + "different" and "IDENTICAL" as assertion names and calls the two arms + behind them missing. The same shape makes it report "Columnar Usable Skip + Predicates" and "Columnar Chunk Groups Total" as names. + +Taking the bash prefixes verbatim took the missing count from 27 to 23, and +hoisting nested calls out of the expect() arguments -- so the extractor has a +clear shot at the name -- took it from 23 to 13. Closing the rest is a change +to compare_to_bash.py -- parse with `ast`, take the LAST string-literal +argument of each expect() call, and compare interpolated names after expansion +-- and that file belongs to #897. Until it lands, "the two can be compared +mechanically" is an aspiration, not a fact about this pair. WHAT THIS PORT DOES NOT CLAIM @@ -112,19 +150,32 @@ STRIDE_X = 997 STRIDE_Y = 7919 -# The pins: box -> (Z-order total, Hilbert total) over the 60 placements. -# Measured on main f2af080, PG 18.4, and reproduced bit-for-bit by the bash -# suite and by this one. +# The pins: box -> (Z-order total, Hilbert total, margin floor) over the 60 +# placements. Measured on main f2af080, PG 18.4, and reproduced bit-for-bit by +# the bash suite and by this one. # # EXACT INTEGERS AND NOT A THRESHOLD. A threshold is the thing someone lowers # when it reddens; it survives a regression that costs half the benefit, and it -# survives it quietly. Any change to the layout, the curve, the group sizing or -# the skip logic moves one of these eight numbers and names itself. +# survives it quietly. An exact pin cannot be lowered without saying so. +# +# WHAT THE EIGHT INTEGERS ACTUALLY CATCH, MEASURED. The bash suite's header +# carries the full account and it is the one to read; in short, a CHANGED CURVE +# is caught by the digest pins above, which move on every run the integers move +# on and sit upstream of them, and the integers' own domain is a CHANGED READER +# at an unchanged layout -- refusing to skip odd-numbered row groups left both +# digests exactly at their pins and moved all eight integers. So do not read +# these eight as the arm that guards the curve. +# +# THE MARGIN FLOOR BESIDE EACH PIN IS NOT THE THRESHOLD REJECTED ABOVE. It is +# about 88% of the measured ratio, so it names a COLLAPSE of the benefit rather +# than a layout that moved. It is here because "Hilbert reads fewer groups" is +# satisfied by a win of one group in four thousand: the reader regression above +# keeps that arm GREEN at every box while z/h falls from 2.0424 to 1.0149. PINS = { - 2000: (241, 118), - 5000: (351, 209), - 12000: (588, 402), - 30000: (1624, 1313), + 2000: (241, 118, "1.80"), + 5000: (351, 209, "1.48"), + 12000: (588, 402, "1.28"), + 30000: (1624, 1313, "1.10"), } # The partition digests the pins were measured over. @@ -171,8 +222,21 @@ # Order-blind, exactly like pgc_set_hash in test/lib.sh, and that is right here: # the arms are supposed to differ in ORDER and in nothing else, so an # order-sensitive oracle would report a difference for the property under test. +# +# AN EMPTY RELATION YIELDS A QUERY_ERROR SENTINEL, NOT THE STRING 'EMPTY'. +# pgc_set_hash returns EMPTY and the bash suite lists EMPTY as unmeasured in +# pgc_measured(). The layer does not: `expect.hash` refuses a value that starts +# with QUERY_ERROR, and its `_empty()` is `v is None or len(v) == 0`, which the +# five-character string EMPTY is not. Measured on two genuinely empty columnar +# relations, hashed by the oracle in its old form: the arm PASSED, comparing +# EMPTY with EMPTY. In its form below the same pair is REFUSED -- "the left side +# is a failed query". So two empty relations would have satisfied the first +# premise of this file and now cannot. Shaping +# the sentinel like the one the layer already refuses puts one guard over both +# cases instead of asking the layer for a second one. SET_HASH_SQL = """ -SELECT coalesce(md5(string_agg(t, chr(10) ORDER BY t)), 'EMPTY') +SELECT coalesce(md5(string_agg(t, chr(10) ORDER BY t)), + 'QUERY_ERROR.empty-relation') FROM (SELECT r::text AS t FROM {table} r) s """ @@ -235,6 +299,30 @@ def _counter(plan, key): return None +# The six layout verbs, and how each one ended. WRITTEN BY THE `locality` +# FIXTURE, READ BY test_every_layout_verb_ran_without_raising. +# +# THE PORT OF crun(). A cluster verb that RAISED would leave its table simply +# unclustered -- and an unclustered table has a partition, a group count and a +# plan, so it reddens the pins below as though the CURVE had changed. The bash +# suite runs every verb through crun(), which names the verb and its SQLSTATE. +# Letting the exception out of the fixture instead would abort the module with +# an error that names no property, so the outcome is recorded here and asserted +# by name, exactly as bash does it. +_VERB_STATES = {} + + +def _verb(conn, what, sql): + """Run one layout verb and record how it ended, under the bash suite's name.""" + import psycopg + + try: + conn.execute(sql) + _VERB_STATES[what] = "noerror" + except psycopg.Error as exc: + _VERB_STATES[what] = exc.sqlstate or "UNKNOWN" + + def _window(table, ox, oy, box): """The query under measurement. One shape, one place, so the arms and the premises cannot drift into asking about two different queries.""" @@ -322,10 +410,14 @@ def locality(pgc_cluster): # hz is Z-order, hh is Hilbert. cz1 and cz2 are the control: the same # verb twice, so an "the partitions differ" arm cannot be satisfied by # an instrument that reports any two tables as different. - conn.execute("SELECT pgcolumnar.cluster('hz', 'a', 'b')") - conn.execute("SELECT pgcolumnar.cluster_hilbert('hh', 'a', 'b')") - conn.execute("SELECT pgcolumnar.cluster('cz1', 'a', 'b')") - conn.execute("SELECT pgcolumnar.cluster('cz2', 'a', 'b')") + _verb(conn, "cluster() on the Z-order arm", + "SELECT pgcolumnar.cluster('hz', 'a', 'b')") + _verb(conn, "cluster_hilbert() on the Hilbert arm", + "SELECT pgcolumnar.cluster_hilbert('hh', 'a', 'b')") + _verb(conn, "cluster() on control table cz1", + "SELECT pgcolumnar.cluster('cz1', 'a', 'b')") + _verb(conn, "cluster() on control table cz2", + "SELECT pgcolumnar.cluster('cz2', 'a', 'b')") # The dense dyadic grid the design predicted the two curves would agree # on: 256 x 256 cells, 1024 rows to a group, 64 full groups. @@ -338,8 +430,10 @@ def locality(pgc_cluster): conn.execute("SELECT pgcolumnar.set_options(%s, stripe_row_limit => %s)", (table, DENSE_STRIPE_ROWS)) conn.execute(f"INSERT INTO {table} SELECT * FROM dsrc") - conn.execute("SELECT pgcolumnar.cluster('dz', 'a', 'b')") - conn.execute("SELECT pgcolumnar.cluster_hilbert('dh', 'a', 'b')") + _verb(conn, "cluster() on the dense control", + "SELECT pgcolumnar.cluster('dz', 'a', 'b')") + _verb(conn, "cluster_hilbert() on the dense control", + "SELECT pgcolumnar.cluster_hilbert('dh', 'a', 'b')") yield conn finally: @@ -360,25 +454,64 @@ def digests(locality): # ============================================================================= +def test_every_layout_verb_ran_without_raising(locality, expect): + """bash: the six `crun` premises, 'premise: WHAT ran without raising'. + + A CALL THAT RAISED LEAVES A TABLE MERELY UNCLUSTERED, and an unclustered + table has a partition, a group count and a plan -- so it reddens the pins + below as though the CURVE had changed. Six named arms, one per verb, so the + output says which verb failed and with what SQLSTATE rather than leaving a + reader to infer it from a moved integer. + """ + for what in ("cluster() on the Z-order arm", + "cluster_hilbert() on the Hilbert arm", + "cluster() on control table cz1", + "cluster() on control table cz2", + "cluster() on the dense control", + "cluster_hilbert() on the dense control"): + expect.text(_VERB_STATES.get(what, "NOT RUN"), "noerror", + f"premise: {what} ran without raising") + + +def test_the_source_holds_the_rows_both_arms_will_load(locality, expect): + """bash: 'premise: the source holds the rows both arms will load'. + + EXACT, NOT A FLOOR. Both arms load from this one heap table, so a source + that is short is a fixture that is short on both arms at once -- which is + the shape the arms' own comparison cannot see. + """ + src_rows = _scalar(locality, "SELECT count(*) FROM src") + expect.num(src_rows, ROWS, + "premise: the source holds the rows both arms will load") + + def test_both_arms_hold_the_identical_row_multiset(locality, expect): """bash: 'premise: the two arms hold the identical row multiset' and the - two arms that follow it. + three arms that follow it. Without this every number below is a fact about two different tables. """ hz = _scalar(locality, SET_HASH_SQL.format(table="hz")) hh = _scalar(locality, SET_HASH_SQL.format(table="hh")) src = _scalar(locality, SET_HASH_SQL.format(table="src")) - expect.hash(hh, hz, "the two arms hold the identical row multiset") + expect.hash(hh, hz, "premise: the two arms hold the identical row multiset") # And that multiset is the source's. Comparing the arms only to each other # passes if both loads went equally wrong. - expect.hash(hz, src, "and it is the source's multiset, so neither load dropped rows") - # A hash of nothing equals a hash of nothing. 'EMPTY' is what the oracle - # returns for a genuinely empty relation, and it is not what these are. - expect.at_least(_scalar(locality, "SELECT count(*) FROM hz"), ROWS, - "and it is a hash of rows: the Z-order arm holds them") - expect.at_least(_scalar(locality, "SELECT count(*) FROM hh"), ROWS, - "and it is a hash of rows: the Hilbert arm holds them") + expect.hash(hz, src, + "premise: and it is the source's multiset, so neither load dropped rows") + # A hash of nothing equals a hash of nothing. The oracle returns a + # QUERY_ERROR sentinel for an empty relation and `expect.hash` refuses it + # outright -- but a refusal is not a NAMED arm, and the bash suite has one, + # so the same property is asserted here by name as well. + hz_is_a_measurement = 0 if str(hz).startswith("QUERY_ERROR") else 1 + expect.num(hz_is_a_measurement, 1, + "premise: and it is a hash of rows, not of an empty or failed read") + # EXACT, NOT TWO FLOORS. `at_least(count, ROWS)` on each arm is satisfied by + # an arm that was loaded TWICE, which is a fixture defect that would move + # every number below it. The bash suite pins the sum; so does this. + total = _scalar(locality, + "SELECT (SELECT count(*) FROM hz) + (SELECT count(*) FROM hh)") + expect.num(total, ROWS * 2, "premise: both arms hold every source row") def test_the_fixture_is_two_dimensional(locality, expect): @@ -388,12 +521,19 @@ def test_the_fixture_is_two_dimensional(locality, expect): both curves the identity in that dimension and the comparison meaningless, and it would do so silently. """ + spans = {} for column in ("a", "b"): - expect.num( - _scalar(locality, - f"SELECT (min({column}) < {SPAN // 100} " - f"AND max({column}) > {SPAN - SPAN // 100})::int FROM src"), - 1, f"column {column} spans the square") + spans[column] = _scalar( + locality, + f"SELECT (min({column}) < {SPAN // 100} " + f"AND max({column}) > {SPAN - SPAN // 100})::int FROM src") + # UNROLLED, NOT LOOPED, because the two bash checks have two different + # names and a loop can only produce one. The names are the bash suite's, + # so compare_to_bash.py can match them. + expect.num( + spans["a"], 1, + "premise: column a spans the square, so this is a two-dimensional fixture") + expect.num(spans["b"], 1, "premise: column b spans the square too") def test_both_arms_have_the_group_count_measured(locality, expect): @@ -410,9 +550,11 @@ def test_both_arms_have_the_group_count_measured(locality, expect): "SELECT count(*) FROM pgcolumnar.row_group " "WHERE storage_id = pgcolumnar.get_storage_id(%s)", (table,)) - expect.num(counts["hz"], GROUPS, "hz has the group count measured") - expect.num(counts["hh"], counts["hz"], - "hh has the same group count, so a group is the same unit on both arms") + expect.num(counts["hz"], GROUPS, + "premise: hz has the group count this measurement was taken at") + expect.num( + counts["hh"], counts["hz"], + "premise: hh has the same group count, so a group is the same unit on both arms") # ============================================================================= @@ -432,8 +574,9 @@ def test_the_two_partitions_differ(digests, expect): change in the reader. """ dz, dh = digests - expect.text(_verdict(dz, dh), "different", - "the Z-order and Hilbert partitions differ") + verdict = _verdict(dz, dh) + expect.text(verdict, "different", + "premise: the Z-order and Hilbert partitions differ") expect.text(dz, DIGEST_ZORDER, "the Z-order partition is the one these numbers were measured over") expect.text(dh, DIGEST_HILBERT, @@ -458,8 +601,16 @@ def test_two_tables_on_the_same_curve_are_one_partition(locality, digests, expec dz, _ = digests cz1 = _digest(locality, "cz1") cz2 = _digest(locality, "cz2") - expect.hash(cz1, cz2, "two tables on the same curve have the identical partition") - expect.hash(cz1, dz, "and that partition is the measured Z-order arm's") + expect.hash(cz1, cz2, + "control: two tables on the same curve have the identical partition") + # THROUGH THE VERDICT, like its sibling and like every digest comparison in + # the bash suite. `expect.hash` refuses a QUERY_ERROR sentinel, so the hole + # the bash arm had -- two failed reads comparing equal -- is closed there by + # the layer; the verdict is used anyway so that the two arms of this control + # are read the same way. + control_verdict = _verdict(cz1, dz) + expect.text(control_verdict, "IDENTICAL", + "control: and that partition is the measured Z-order arm's") # ============================================================================= @@ -478,19 +629,25 @@ def test_dense_dyadic_grid_is_one_partition(locality, expect): "Hilbert always changes the layout", and so a reader can see that arm 2's "different" is a measurement rather than a foregone conclusion. """ + duplicated_cells = _scalar( + locality, + "SELECT count(*) FROM (SELECT a, b FROM dsrc " + "GROUP BY a, b HAVING count(*) <> 1) x") + dense_groups = _scalar( + locality, + "SELECT count(*) FROM pgcolumnar.row_group " + "WHERE storage_id = pgcolumnar.get_storage_id('dz')") + ddz = _digest(locality, "dz") + ddh = _digest(locality, "dh") expect.num( - _scalar(locality, - "SELECT count(*) FROM (SELECT a, b FROM dsrc " - "GROUP BY a, b HAVING count(*) <> 1) x"), - 0, "the dense grid is dense -- every cell present exactly once") + duplicated_cells, 0, + "control premise: the dense grid is dense -- every cell present exactly once") expect.num( - _scalar(locality, - "SELECT count(*) FROM pgcolumnar.row_group " - "WHERE storage_id = pgcolumnar.get_storage_id('dz')"), - DENSE_SIDE * DENSE_SIDE // DENSE_STRIPE_ROWS, - "the dense grid has the dyadic group count") - expect.hash(_digest(locality, "dz"), _digest(locality, "dh"), - "on a dense dyadic grid the two curves cut the identical partition") + dense_groups, DENSE_SIDE * DENSE_SIDE // DENSE_STRIPE_ROWS, + "control premise: the dense grid has the dyadic group count") + expect.hash( + ddz, ddh, + "control: on a dense dyadic grid the two curves cut the identical partition") # ============================================================================= @@ -506,13 +663,27 @@ def test_both_arms_plan_as_a_columnar_scan(locality, expect): that predicate says yes for a plan with no columnar scan in it. "Columnar Projected Columns" is emitted only by the scan's explain callback. + + AND THE HELPER UNDER THIS ARM IS NOT ITSELF PINNED. #897's + test_guards_pinned.py pins num, text, at_least, plan_node, outcomes, + cannot_run and hash, and has no plan_marker case. Measured on #897 head + 5f3dedb merged into this branch: replacing plan_marker's present-arm raise + with `pass` leaves test_hilbert_locality.py, test_connection.py, + test_guards_pinned.py and test_layer.py all green, 50 passed. So the arms + below rest on a helper that has no removal proof of its own; pin it in + #897 before reading them as strong. """ box = 2000 ox, oy = _origins(box)[0] - for table in ("hz", "hh"): - expect.plan_marker(_plan(locality, _window(table, ox, oy, box)), - "Columnar Projected Columns", - name=f"the {table} arm plans as a columnar scan") + pz = _plan(locality, _window("hz", ox, oy, box)) + ph = _plan(locality, _window("hh", ox, oy, box)) + # UNROLLED, AND THE NAMES ARE PLAIN LITERALS. A loop can carry only one + # name, and an f-string is invisible to compare_to_bash.py's extractor, so + # both arms would drop out of the port's name list. + expect.plan_marker(pz, "Columnar Projected Columns", + name="premise: the Z-order arm plans as a columnar scan") + expect.plan_marker(ph, "Columnar Projected Columns", + name="premise: the Hilbert arm plans as a columnar scan") def test_parallelism_is_off_so_a_counter_is_a_fact_about_the_layout(locality, expect): @@ -555,14 +726,14 @@ def test_the_predicates_are_usable_and_the_denominators_match(locality, expect, ph = _plan(locality, _window("hh", ox, oy, box)) expect.num(_counter(pz, "Columnar Usable Skip Predicates"), 4, - f"box {box}, the Z-order arm can skip on all four predicates") + f"premise: box {box}, the Z-order arm can skip on all four predicates") expect.num(_counter(ph, "Columnar Usable Skip Predicates"), 4, - f"box {box}, the Hilbert arm can skip on all four predicates") + f"premise: box {box}, the Hilbert arm can skip on all four predicates") expect.num(_counter(ph, "Columnar Chunk Groups Total"), _counter(pz, "Columnar Chunk Groups Total"), - f"box {box}, both arms have the same number of groups to read") + f"premise: box {box}, both arms have the same number of groups to read") expect.num(_counter(pz, "Columnar Chunk Groups Total"), GROUPS, - f"box {box}, that denominator is the whole relation") + f"premise: box {box}, that denominator is the whole relation") # ============================================================================= @@ -588,7 +759,7 @@ def test_groups_read_over_sixty_placements(locality, digests, expect, box): f"between them is not about the curve") return - want_z, want_h = PINS[box] + want_z, want_h, floor = PINS[box] measured_z, total_z = _groups_read(locality, "hz", box) measured_h, total_h = _groups_read(locality, "hh", box) @@ -604,9 +775,7 @@ def test_groups_read_over_sixty_placements(locality, digests, expect, box): expect.num(total_h, want_h, f"box {box}: groups read over {PLACEMENTS} placements, Hilbert") - # And, separately from the pins: the curve still wins here. This arm stays - # meaningful when the pins are re-taken, and it is what tells a reader - # "the layout moved" apart from "Hilbert stopped winning". + # And, separately from the pins: the curve still wins here. # # A DIFFERENCE, BECAUSE THE LAYER HAS NO STRICT INEQUALITY. `at_least` is # the only bound it offers and it refuses a floor of zero, so "h < z" is @@ -615,7 +784,22 @@ def test_groups_read_over_sixty_placements(locality, digests, expect, box): expect.at_least(total_z - total_h, 1, f"box {box}: Hilbert reads fewer groups than Z-order") - # The ratio is PRINTED, from the two numbers just measured, and asserted - # nowhere: a ratio can be held constant by both arms getting worse together. - print(f"-- box {box}: z={total_z} h={total_h} " - f"z/h={total_z / total_h:.4f}") + # AND IT STILL WINS BY THE MARGIN IT WAS MEASURED AT. + # + # "z - h is at least 1" is satisfied by a win of ONE group in four + # thousand, so on its own it cannot tell "the layout changed" from "the + # benefit is gone". Refusing to skip odd-numbered row groups in the reader + # moves all eight pins, keeps the arm above GREEN at every box, and takes + # z/h from 2.0424 to 1.0149. This arm is what reddens there. The ratio is + # computed once and both this arm and the line printed below use that one + # value, so the number asserted and the number reported cannot drift. + ratio = total_z / total_h if total_h else 0.0 + expect.text( + "at or above the floor" if ratio >= float(floor) + else f"BELOW THE FLOOR (z/h={ratio:.4f})", + "at or above the floor", + f"box {box}: and it wins by the margin measured, z/h at least {floor}") + + # The ratio is PRINTED as well, and asserted nowhere beyond the floor above: + # a ratio can be held constant by both arms getting worse together. + print(f"-- box {box}: z={total_z} h={total_h} z/h={ratio:.4f}") From 98f57fd2e61e68a43bea47fa2c66e2f8d1f1c4f9 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 9 Sep 2026 11:24:01 -0600 Subject: [PATCH 3/8] test: register hilbert_locality now that it is green (#889) The red-suite rule the earlier commits followed: a suite stays out of SUITES until it passes, because a red suite in the matrix is everyone's problem. It passes -- 65 checks, 0 failed, and every pinned integer and both digests reproduce on a second prefix and build dir. The pytest twin is NOT registered anywhere, and cannot be: it is blocked on #897 and its header now pins that dependency to b785795d7ccd rather than to a branch name. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK --- test/pytest/test_hilbert_locality.py | 17 +++++++++++++++-- test/run_all_versions.sh | 1 + 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/test/pytest/test_hilbert_locality.py b/test/pytest/test_hilbert_locality.py index 9824b41c..0d029ee0 100644 --- a/test/pytest/test_hilbert_locality.py +++ b/test/pytest/test_hilbert_locality.py @@ -10,8 +10,21 @@ committed here so that the port exists and can be reviewed beside the bash suite; it becomes runnable when #897 lands, and not before. -It was RUN against #897 head 5f3dedb, which merges cleanly with this branch, -in a scratch worktree on PostgreSQL 18.4: 18 passed, 3.20s on a warm tree. The eight pins below came out +It was RUN against **#897 at b785795d7ccd** -- the head as of 2026-09-09 -- and +passed 18 of 18 there. It was originally written against 5f3dedb; those two trees +are NOT identical (e09554995ebe vs c8120cdfca1e, +925 lines including the +`cannot_run` fix this file depends on), so it was re-run rather than relabelled. +The SHA is pinned rather than the branch name because a branch name cannot be +checked six weeks from now, and this one moved three times while the file was +being written. + +ONE THING THIS FILE CANNOT SATISFY ON ITS OWN. #897 at b785795 adds +`test_docs_cover_the_corpus.py`, a gate requiring every pytest file and every +`def test_` to be named in `test/pytest/TESTS.md`. With this file present and +undocumented that gate is RED -- measured, 2 failed, `got '(66, 6)' want +'(78, 7)'`. TESTS.md lives on #897's branch, so the entry cannot be written from +here; it lands when the two branches meet. That is a real dependency and not an +oversight. identical to the bash suite's, from a different harness and a different way of reading the counters -- 241/118, 351/209, 588/402, 1624/1313, and z/h of 2.0424, 1.6794, 1.4627, 1.2369. diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index b6330593..f5ec2c48 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -94,6 +94,7 @@ SUITES=( harness_selftest hilbert_cluster hilbert_curve + hilbert_locality iceberg_catalog iceberg_data_files iceberg_deletes From ec083818a70c9c79accaed1751154c25ae74d3f8 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 9 Sep 2026 10:13:30 -0600 Subject: [PATCH 4/8] docs: alpha4's Hilbert item is done, and its claim is now measured (#889) The plan asserted that Hilbert "gives better locality" and cited no measurement of ours. It now cites one, and records what the measurement does not license. box Z-order Hilbert ratio 2000 241 118 2.04 5000 351 209 1.68 12000 588 402 1.46 30000 1624 1313 1.24 200,000 rows, two int columns, non-dyadic stripe_row_limit, 60 query-box placements per size, summing the engine's own Columnar Chunk Groups Read. Both arms load from one materialised heap table, so they hold identical rows, and the two curves are asserted to partition those rows differently before any ratio is taken. The decaying ratio is the part that makes it credible. A large box must read most groups whichever curve laid them out, so a locality effect must decay; a constant offset would have been an artifact, and an earlier single-origin version of this measurement produced exactly that -- differences of 1, 1, 0 and 1 groups, with a z/h of 2.000 resting on one group. The claim had to be measured rather than assumed for a specific reason now recorded in the plan: on a dense power-of-two-aligned grid whose groups are perfect dyadic sub-cubes the two curves produce the IDENTICAL partition, so the obvious fixture cannot separate them at all. That case is carried as a control, alongside a null control that clusters both arms with Z-order; both correctly refuse to report a ratio rather than reporting 1.0. What the entry now says it does NOT license: two int columns, uniform, one shape. Not three or four columns, not mixed types -- the ordinal layer caps the benefit for unequal widths, and bool against int4 showed no separation at all. Chunk groups are work, not time; no timing claim is made and the per-row key build has never been measured inside PostgreSQL. Full method and the three defects the pilot found in itself are on #889. docs_style.sh: 9 checks, PASSED. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK --- CONTEXT.md | 19 ++++++++++++++ design/RELEASE_PLAN_1.0.md | 53 +++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 60515373..33d41b0c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -163,6 +163,25 @@ because it describes the run rather than the plan. A **suite** is `test/.sh`. It stands up its own cluster, runs **checks**, and ends with `pgc_summary`. +**Every new test is written twice: once as a `.sh` suite and once as a pytest +test, in the same change.** Owner's rule, 2026-09-09. Not "ported later" and not +"one or the other" -- a test that exists in only one harness is not finished. + +The reason is the reason #432 exists. The bash harness carries 4,429 anchored +assertions and is the gate; the pytest harness returns typed results and refuses +shapes bash passes silently. Writing new tests in only the old one grows the +port debt with every change, and writing them in only the new one puts a property +outside the gate. Writing both keeps the two harnesses honest about each other: +where they disagree, one of them is wrong, and that is worth finding at the time +rather than during a port. + +**A sequencing note that will stop being true.** As of 2026-09-09 the pytest +harness is PR #897 and is not on `main`, so this rule cannot be satisfied for a +test written today. Until it lands, write the `.sh` suite, write the pytest twin +alongside it in the same change, and say in both headers that the twin is blocked +on #897. Do not let "the harness is not merged yet" become a standing excuse: the +twin is written either way, so the debt is never deferred, only its execution is. + - Register every suite in `SUITES` in `test/run_all_versions.sh`. That array is **one name per line and sorted**; insert in sorted position, never at the end. `harness_selftest` fails if the order decays. diff --git a/design/RELEASE_PLAN_1.0.md b/design/RELEASE_PLAN_1.0.md index fe94a677..eb766c9d 100644 --- a/design/RELEASE_PLAN_1.0.md +++ b/design/RELEASE_PLAN_1.0.md @@ -93,10 +93,55 @@ Nothing further should be added. It is 11 days into a 14-day cycle. ### 1.0-alpha4, target 2026-09-15. Theme: skipping and layout -- **Hilbert curve clustering.** Confirmed for 1.0 by the owner on 2026-08-29. - Z-order ships; Hilbert is the open half and gives better locality on the same - machinery. It is a new key kind for `cluster` and `recluster`, so it is - user-visible surface. Under the freeze rule it is this release or 2.0. +- **Hilbert curve clustering. DONE**, merged as #899 (issue #889). It ships as + two new verbs, `pgcolumnar.cluster_hilbert` and `pgcolumnar.recluster_hilbert`, + rather than a key-kind argument: PostgreSQL refuses to extend the existing + `(regclass, VARIADIC name[])` signature in either direction, and an + array-plus-kind overload breaks the documented `cluster('t','a','b')` call + style. Both measured on 18.4. + + **The "better locality" claim above was an assertion when this plan was + written, and it is now a measurement.** It had to be, because the obvious + fixture cannot show it: on a dense power-of-two-aligned grid whose groups are + perfect dyadic sub-cubes, the two curves produce the **identical** row-to-group + partition, so no query can distinguish them. That case is carried as a control. + + On 200,000 rows over two `int` columns with a deliberately non-dyadic + `stripe_row_limit`, summing the engine's own chunk-group counter over 60 query + boxes per size, across **two seeds**: + + | query box | Z-order groups read | Hilbert groups read | Hilbert advantage | + | --- | --- | --- | --- | + | 2000 | 233 / 241 | 122 / 118 | 1.91x - 2.04x | + | 5000 | 339 / 351 | 212 / 209 | 1.60x - 1.68x | + | 12000 | 592 / 588 | 412 / 402 | 1.44x - 1.46x | + | 30000 | 1623 / 1624 | 1308 / 1313 | 1.24x | + + **Reading the table: in the two count columns FEWER IS BETTER, and in the last + column LARGER IS BETTER.** A chunk group that is read is a group the scan had + to open and decode; one that is skipped costs nothing. `groupsSkipped++` and + `groupsRead++` are the two arms of the same loop in `src/columnar_reader.c`, so + the count is work done and not work available. + + **The claim is the decay, not the headline.** The advantage shrinks from about + 2x to about 1.24x as the query box grows, and that is the shape a locality + effect has: a large box must read most groups whichever curve laid them out. A + constant offset would have been an artifact, and an earlier single-origin + version of this measurement produced exactly that. + + **The most selective cell is the least reproducible, so do not quote it alone.** + Across the two seeds the ratio moved +6.94%, +5.03%, +1.80% and -0.32% as the + box grows -- so the 2x headline is the least stable number in the table and the + decay is the most stable. Two seeds is a spread, not a distribution; + a threshold set from this would need more. + + **What that does not license.** Two `int` columns, uniform, one shape. It is + not evidence for three or four columns or for mixed types, and the ordinal + layer caps the benefit for types of unequal width -- `bool` against `int4` + showed no separation at all. It is a count of chunk groups, which is work and + not time; no timing claim is made, and the per-row key build has never been + measured inside PostgreSQL. Recorded on #889 with the instrument's own defects, + three of which produced wrong numbers before they were found. - **Per-tier block compression defaults.** On fast local storage, block compression can cost more CPU than it saves in I/O. The finding reverses for object storage. Make the default depend on the tier. Low effort, and it changes From 5cbc27a0e68cfee5eee09d2bacc2cf6ad74222da Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 9 Sep 2026 12:00:48 -0600 Subject: [PATCH 5/8] docs: pin the SHA, not the branch, and record that #897 moved (#889) Two changes from the #902 review, both from OffgridwithJD. CONTEXT.md's twin rule now says to pin the SHA the twin was tested against rather than the branch name. Their argument is the one that convinced me: a branch name is not checkable later, and it is why they could verify my claim at all. The harness branch moved three times while the first twin was being written, and two of those moves changed its content -- so "blocked on #897" and "blocked on #897 at b785795d7ccd" are different claims and only one can be falsified. Same reason a tag is read from the API rather than from a local ref, which I got wrong earlier today and filed a false issue over. The twin's header records that #897 moved a fourth time, to 9064a46, and DELIBERATELY DOES NOT UPDATE THE PIN. The point of a SHA is to say what was tested. What is recorded instead is why the pin still describes the current head, verified here rather than taken from the push notice: b785795 test/pytest tree = b20ad7e388e0 9064a46 test/pytest tree = b20ad7e388e0 whole delta = 30 lines in one test/selftest/ file the harness never reads NOT CHANGED, deliberately: the five x86_64 build failures on this PR are the PGDG apt mirror, not this branch. The mirror is serving a Release file created at 17:16:59 alongside a component index last modified at 09:41:12, so the index cannot match the manifest describing it. Two attempts twenty minutes apart produced byte-identical hashes, which rules out a race. #898 at 6939bba and #897 at b785795 both went fully green before 17:16 and both #897 at 9064a46 and this branch fail after it, with #897's delta being thirty lines in a directory no build job reads. aarch64 passed all five majors throughout. Patching ci.yml around a mirror that is mid-sync would outlive the outage and get copied. docs_style.sh: 9 checks, PASSED. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK --- CONTEXT.md | 8 ++++++++ test/pytest/test_hilbert_locality.py | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/CONTEXT.md b/CONTEXT.md index 33d41b0c..f93a1d9e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -182,6 +182,14 @@ alongside it in the same change, and say in both headers that the twin is blocke on #897. Do not let "the harness is not merged yet" become a standing excuse: the twin is written either way, so the debt is never deferred, only its execution is. +**And pin the SHA you tested the twin against, not the branch name.** A branch +name is not checkable later and moves under you -- the pytest harness branch +moved three times while the first twin was being written, and two of those moves +changed its content. `blocked on #897 at b785795d7ccd` costs the same to type as +`blocked on #897` and is falsifiable: a reader can diff that SHA against the +branch and see whether the claim still holds. Same reason a tag is read from the +API rather than from a local ref. + - Register every suite in `SUITES` in `test/run_all_versions.sh`. That array is **one name per line and sorted**; insert in sorted position, never at the end. `harness_selftest` fails if the order decays. diff --git a/test/pytest/test_hilbert_locality.py b/test/pytest/test_hilbert_locality.py index 0d029ee0..c6485f96 100644 --- a/test/pytest/test_hilbert_locality.py +++ b/test/pytest/test_hilbert_locality.py @@ -18,6 +18,14 @@ checked six weeks from now, and this one moved three times while the file was being written. +It moved a fourth time after this header was written: #897 is now 9064a46eb57d. +That pin is deliberately NOT updated, because the point of a SHA is to say what +was tested. `test/pytest/` is byte-identical between the two -- both trees hash +to b20ad7e388e0, and the whole delta is thirty lines in one +`test/selftest/` file the pytest harness never reads -- so the 18-of-18 result +above still describes the current head. Verified here rather than taken from the +push notice. + ONE THING THIS FILE CANNOT SATISFY ON ITS OWN. #897 at b785795 adds `test_docs_cover_the_corpus.py`, a gate requiring every pytest file and every `def test_` to be named in `test/pytest/TESTS.md`. With this file present and From 2088712bb2d3fcb1351933a0f4f384110f6dc5b5 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 9 Sep 2026 13:36:22 -0600 Subject: [PATCH 6/8] docs: the twin is documented, and it is no longer blocked (#889) the first time and the dependency the header described is now satisfiable. The corpus gate #897 brought with it went RED the moment the rebase put the two together, exactly as the header predicted: FAIL every test file and every test in the corpus is named in TESTS.md: got [[13: test_hilbert_locality.py test_every_layout_verb_ran_without_raising ...]] FAIL and the totals it states are the totals on disk: got [74 6] want [86 7] That is the gate working, not a problem: it names the file and every test in it rather than reporting a count that moved. So TESTS.md gains section 9 -- twelve tests, each with the wrong state it refuses -- and the totals become 86 in 7. The section says what the twin does NOT carry, because that is the part a reader would otherwise assume: the exact-integer pins are the bash suite's, and the twin asserts only that Hilbert reads fewer groups at every box. hilbert_locality.sh's header records why the integers exist at all -- for a CURVE change the digest pins upstream catch it first, so their real domain is a changed READER at an unchanged layout. The SHA pins in the twin's header are kept. They are the record of what was tested against what, and #897's branch moved four times while this file was being written -- twice with a changed tree. A pin that is deleted once the dependency lands destroys the only evidence that the claim was ever checkable. harness_selftest 342, hilbert_locality 65, docs_style 9. COPT=-Werror, 0 warnings, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK --- test/pytest/TESTS.md | 50 +++++++++++++++++++++++++--- test/pytest/test_hilbert_locality.py | 18 ++++++---- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index 1b455763..c6dc5c17 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -4,7 +4,7 @@ Reference for anyone reading, running, or adding to `test/pytest/`. The design a the decisions behind the harness are in `design/ISSUE_432_PYTEST_HARNESS.md`. This file covers the tests themselves. -**78 tests in 6 files.** Sixty-three of them test the harness rather than the +**90 tests in 7 files.** Sixty-three of them test the harness rather than the product, and they come first, because a harness that can report a false green makes every other result in this directory worthless. @@ -32,8 +32,9 @@ behaviour, the source of that number is named. - [6. test_docs_cover_the_corpus.py: this document, checked](#6-test_docs_cover_the_corpuspy-this-document-checked) - [7. test_connection.py: the cluster and the direct connection](#7-test_connectionpy-the-cluster-and-the-direct-connection) - [8. test_native_projection.py: the ported suite](#8-test_native_projectionpy-the-ported-suite) -- [9. Adding a test](#9-adding-a-test) -- [10. Traps this corpus records](#10-traps-this-corpus-records) +- [9. test_hilbert_locality.py: what the Hilbert curve buys](#9-test_hilbert_localitypy-what-the-hilbert-curve-buys) +- [10. Adding a test](#10-adding-a-test) +- [11. Traps this corpus records](#11-traps-this-corpus-records) ## 1. How to read a test in here @@ -588,7 +589,46 @@ The mutation makes `PgColumnarProjectionFanoutRow` return without writing. Each builds and installs once, and both harnesses print the `.so` md5 they measured, so an arm where the two differ is void rather than reported. -## 9. Adding a test +## 9. test_hilbert_locality.py: what the Hilbert curve buys + +The pytest twin of `test/hilbert_locality.sh`, written in the same change under the +owner's rule of 2026-09-09 that every new test ships in both harnesses. Twelve tests. + +It measures one thing -- how many chunk groups a range query reads under Z-order +against Hilbert -- and spends most of its arms refusing to measure it when the +comparison would be meaningless. + +| test | what it refuses | +| --- | --- | +| `test_every_layout_verb_ran_without_raising` | a verb that raised looks identical to a verb that no-opped | +| `test_the_source_holds_the_rows_both_arms_will_load` | an empty fixture | +| `test_both_arms_hold_the_identical_row_multiset` | the two arms holding DIFFERENT DATA, which made the first pilot's ratio a fact about the data rather than the curve | +| `test_the_fixture_is_two_dimensional` | a fixture where one column does not span, so the curve has nothing to interleave | +| `test_both_arms_have_the_group_count_measured` | a group meaning a different unit on each arm | +| `test_the_two_partitions_differ` | the case where the curves cut the SAME partition, where no query can separate them | +| `test_two_tables_on_the_same_curve_are_one_partition` | the null control: same curve twice must be one partition | +| `test_dense_dyadic_grid_is_one_partition` | the dense power-of-two grid, which provably cannot separate the curves | +| `test_both_arms_plan_as_a_columnar_scan` | a counter read from a plan that is not the columnar scan | +| `test_parallelism_is_off_so_a_counter_is_a_fact_about_the_layout` | a per-worker counter read as a whole-query one | +| `test_the_predicates_are_usable_and_the_denominators_match` | unequal denominators, and zero usable skip predicates | +| `test_groups_read_over_sixty_placements` | nothing -- this is the measurement | + +**The two controls REFUSE rather than returning 1.0.** Both are cases where the +curves genuinely cut the same partition, so a ratio would be arithmetic on two +identical numbers. A harness that reported `1.0000` there would look like a +measurement and be an artifact. + +**Why sixty placements and not one.** A single query-box origin measures where that +box happened to land. At one origin the differences were 1, 1, 0 and 1 groups, and +the ratio read `2.000` off a single group. + +**What the twin does NOT carry**, and the bash suite does: the exact-integer pins. +The twin asserts Hilbert reads fewer groups at every box; `test/hilbert_locality.sh` +pins the eight counts exactly. Its header records why -- for a CURVE change the +digest pins upstream catch it first and the integers add nothing, so their real +domain is a changed READER at an unchanged layout. + +## 10. Adding a test 0. **Write it twice.** Every test in this tree ships as a `.sh` suite and a pytest test **in the same change** (jd, 2026-09-09). Not ported later, not one or the @@ -615,7 +655,7 @@ an arm where the two differ is void rather than reported. failed the selftest on both majors of the matrix, which is how it was found. A new directory under `test/` inherits every rule the old ones follow. -## 10. Traps this corpus records +## 11. Traps this corpus records Recorded because each one produced a confident wrong result before it was caught, and all are the same family as the defect the layer exists to prevent. diff --git a/test/pytest/test_hilbert_locality.py b/test/pytest/test_hilbert_locality.py index c6485f96..93b32866 100644 --- a/test/pytest/test_hilbert_locality.py +++ b/test/pytest/test_hilbert_locality.py @@ -26,13 +26,17 @@ above still describes the current head. Verified here rather than taken from the push notice. -ONE THING THIS FILE CANNOT SATISFY ON ITS OWN. #897 at b785795 adds -`test_docs_cover_the_corpus.py`, a gate requiring every pytest file and every -`def test_` to be named in `test/pytest/TESTS.md`. With this file present and -undocumented that gate is RED -- measured, 2 failed, `got '(66, 6)' want -'(78, 7)'`. TESTS.md lives on #897's branch, so the entry cannot be written from -here; it lands when the two branches meet. That is a real dependency and not an -oversight. +#897 HAS SINCE MERGED, as `6364e22`, so this file is no longer blocked and the +corpus gate it depends on is now satisfiable from one tree. It is documented in +`test/pytest/TESTS.md` section 9, and `test/selftest/350` checks that document +against disk -- with this file undocumented the gate was RED, measured, `got +[74 6] want [86 7]`. Those four integers are what the gate said THEN, on a tree +holding #897 and this file and nothing else; #903 has since added four harness +tests, so the same gate now reads 90 in 7. The old pair is left as the record of +the run rather than regenerated, for the reason a corrected number dies when the +summary is rebuilt from the pre-fix text: a count belongs to the revision it +counted. The SHA pins above are kept on the same footing -- the record of what was +tested when, not a live dependency. identical to the bash suite's, from a different harness and a different way of reading the counters -- 241/118, 351/209, 588/402, 1624/1313, and z/h of 2.0424, 1.6794, 1.4627, 1.2369. From 4ab0e25846e2db85a231416d4b941cd339c044c8 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 9 Sep 2026 14:08:26 -0600 Subject: [PATCH 7/8] docs: the corpus total counts functions, a run counts items (#889) The rebase onto `bfdd1f9` collided on TESTS.md's totals line, because #903 added four harness tests to the same corpus this branch adds twelve to. Both sides of the conflict were wrong for the composed tree, so the number was COUNTED with the gate's own function rather than picked from either side: python3 -c "... corpus_tests(Path('test/pytest')) ..." files=7 tests=90 test_build_refusal.py 22 test_connection.py 8 test_docs_cover_the_corpus.py 8 test_guards_pinned.py 19 test_hilbert_locality.py 12 test_layer.py 14 test_native_projection.py 7 Then I passed 90 to `--pgc-expect-tests` and the run refused: ERROR: collected 96 test(s) but expected 90. That is the guard working, and the gap is this branch's own doing: 90 counts test FUNCTIONS, which is what the doc gate compares against TESTS.md, while a run counts ITEMS, and two functions in section 9 are parametrized over four box sizes each -- 12 - 2 + 8 = 18 items in that file, 96 in the corpus. Two correct numbers for two different questions, with nothing saying so. The header now says which is which and which one `--pgc-expect-tests` wants. The twin's `got [74 6] want [86 7]` is NOT regenerated. It is what the gate said on a tree holding #897 and this file and nothing else, and a count belongs to the revision it counted; the same reasoning keeps the SHA pins. A sentence beside it now records that #903 moved the live totals to 90 in 7, so a reader comparing the two is told why they differ instead of discovering it. Verified on the rebased head, my own prefix /usr/local/pg17_904: harness_selftest.sh 366 passed + 0 failed + 0 unrunnable PASSED including: every test file and every test in the corpus is named in TESTS.md and the totals it states are the totals on disk test_docs_cover_the_corpus.py 8 passed hilbert_locality.sh 65 passed + 0 failed + 0 unrunnable PASSED docs_style.sh PASSED Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK --- test/pytest/TESTS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index c6dc5c17..2c80c0e3 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -8,6 +8,12 @@ file covers the tests themselves. product, and they come first, because a harness that can report a false green makes every other result in this directory worthless. +That total counts test FUNCTIONS, which is what `test_docs_cover_the_corpus.py` +compares against this document. A run reports test ITEMS and prints a larger +number -- 96 at the time of writing -- because two functions in section 9 are +parametrized over four query-box sizes each. Pass the ITEM count to +`--pgc-expect-tests`, not the total above. + That ratio is not an accident of taste. Two of those files exist because a reviewer neutered the guards one at a time and found most of them deletable with the suite still green, and because the corpus once reported 25 passed against source carrying From 3ed61d3f4d9c7c11d5b58e0f29074b854b8e33e0 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 9 Sep 2026 14:14:41 -0600 Subject: [PATCH 8/8] docs: the refusal paragraph reads as history, because the gap is closed (#889) OffgridwithJD's fourth review point, and they had to make it twice. I reported this paragraph gone and it was not: my check was grep -n "DECLARING A TEST UNRUNNABLE MAKES IT PASS" test/pytest/... and the file wraps that phrase across lines 66 and 67, so the grep found nothing and I read nothing as evidence of absence. I reported "ABSENT at HEAD (good)" to a peer who then checked the artifact and found it byte-identical at two heads. A single-line grep for a phrase that is line-wrapped cannot report what it was asked; the same class as every instrument this branch has been cataloguing, and this one was mine. The re-check is a regex tolerant of the break, and it is what now says all four stale claims are gone. The paragraph itself was worse than merely stale: line 29 said "#897 HAS SINCE MERGED, so this file is no longer blocked" and line 77 told the reader the change it needed from #897 had not happened. The contradiction had moved inside one file rather than being resolved. Re-measured here rather than taken from the merge or from the peer: UNRUN test_p.py::test_cannot: ABSENT_FIXTURE: no corpus checks unrunnable: 1 exit code = 67 So the twin DOES carry the bash suite's refusal. One precision the replacement adds beyond what was suggested, because I hit it while measuring: pytest's own per-item tally still prints "1 passed" for the declaring test. The session exit and the unrunnable count are what carry the refusal, not the tally -- and a reader who greps for "passed" reaches the wrong conclusion, which is the same mistake in the other direction. The measurement against 5f3dedb is KEPT, reframed as history. It is the record of what the gap was and it should outlive the gap; deleting it would leave the claim "this was once broken" resting on nothing. The section heading moved with it, since "ONE THING THIS FILE CANNOT DO" was false as of #897. Verified on this head, prefix /usr/local/pg17_904: harness_selftest.sh 366 passed + 0 failed + 0 unrunnable PASSED hilbert_locality.sh 65 passed + 0 failed + 0 unrunnable PASSED pytest corpus 96 passed (90 functions in 7 files) docs_style.sh PASSED Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK --- test/pytest/test_hilbert_locality.py | 43 +++++++++++++++++----------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/test/pytest/test_hilbert_locality.py b/test/pytest/test_hilbert_locality.py index 93b32866..ada183c7 100644 --- a/test/pytest/test_hilbert_locality.py +++ b/test/pytest/test_hilbert_locality.py @@ -54,28 +54,39 @@ So "blocked on #897" means the whole of #897, not only the directory. -ONE THING THIS FILE CANNOT DO THAT ITS BASH TWIN DOES, AND IT IS MEASURED +THE REFUSAL THIS FILE ONCE COULD NOT CARRY, AND NOW DOES The bash suite REFUSES to report a ratio when the two partitions are not different: the sixteen measurement arms print UNRUN and pgc_summary counts them as a third state, "16 unrunnable"; with nothing else red the suite exits 67, INCOMPLETE. Here the same refusal goes through `expect.cannot_run("UNMET_PRECONDITION", ...)`, and the layer records -the third state but nothing reports it. The mechanism is one line: -`cannot_run` sets `self.unrunnable` and calls `self._counted()`, and -`pytest_runtest_call` asks only whether `rec.count == 0` -- so DECLARING A TEST -UNRUNNABLE MAKES IT PASS. Measured against that head, with the fixture -mutated to lay BOTH arms out with cluster(): "1 failed, 17 passed", and all -four test_groups_read_over_sixty_placements cases were among the greens while -asserting nothing about groups read. Only test_the_two_partitions_differ -reddened, and it reddened for its own reason. - -The suite is still red overall, so nothing ships silently; but a reader -counting greens counts four that never asked their question, and DO NOT READ -THIS FILE AS CARRYING THE BASH SUITE'S REFUSAL until the layer converts an -unrunnable record into a non-pass outcome and a non-zero session exit -- the -pytest equivalent of PGC_EXIT_INCOMPLETE=67. That is a change to #897, not to -this file. +the third state and reports it. + +THE PARAGRAPH BELOW WAS TRUE WHEN WRITTEN AND IS NOT ANY MORE. It is kept as history +because the measurement is the valuable part: it is the record of what the gap +actually was, and it should outlive the gap. + +What was true at 5f3dedb: `cannot_run` set `self.unrunnable` and nothing read +it, so declaring a test unrunnable made it PASS. Measured against that head, +with the fixture mutated to lay BOTH arms out with cluster(): "1 failed, 17 +passed", and all four test_groups_read_over_sixty_placements cases were among +the greens while asserting nothing about groups read. Only +test_the_two_partitions_differ reddened, and it reddened for its own reason. + +#897 CLOSED IT, and this file therefore DOES carry the bash suite's refusal. +Re-measured here on the merged tree rather than taken from the merge: + + UNRUN test_p.py::test_cannot: ABSENT_FIXTURE: no corpus + checks unrunnable: 1 + exit code = 67 + +Read that exit code and that count, NOT pytest's own tally, which still prints +"1 passed" for the item. The per-item outcome is the thing that has not +changed; the session-level refusal is what PGC_EXIT_INCOMPLETE=67 means in the +bash suite and it is what this run produces. A reader who greps for "passed" +will reach the wrong conclusion, which is how this paragraph came to be stale +and then to be reported fixed while it was not. WHAT THIS IS A PORT OF, AND WHAT IT IS FOR