From 7c1f2c395e41a83db038e3afd1ec1f99e72f539a Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Wed, 5 Aug 2026 11:17:10 -0600 Subject: [PATCH] test: add check_text, for the oracles check_num cannot express (#418) I said on #422 and #420 that I would replace native_index_projection.sh's local non-empty guard with check_num once the shared helpers landed. That was wrong, and finding out why exposed the larger half of #418. check_num requires a NUMBER. An md5 over an ordered result is not one: FAIL two identical md5 hashes: not a measurement, so nothing was compared: got [9dd4e461268c8034f5c8564e155c67a6] want [9dd4e461268c8034f5c8564e155c67a6] So a suite comparing a non-numeric oracle has nothing to reach for and falls back to plain check, where "" equals "" and prints PASS. That is not a corner: 35 places in this tree compare an md5(string_agg(...)) oracle, across audit, concurrency, arrow_import, encode_invariants, encode_effort and column_projection, and every one is a down cluster or an errored query away from comparing nothing with nothing. check_text asserts presence rather than shape, which is the most a shared helper can know. A caller that knows the shape should still say so: agree() keeps its 32-hex-character test, because "not empty" would accept a psql error message. harness_selftest.sh gains five probes, including the one that records why this exists at all -- check_num refusing an md5 -- so the next person does not repeat my mistake by adopting the wrong helper. 33 checks green on PG18; native_index_projection.sh 35 green. --- test/harness_selftest.sh | 11 +++++++++++ test/lib.sh | 27 +++++++++++++++++++++++++++ test/native_index_projection.sh | 10 +++++----- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/test/harness_selftest.sh b/test/harness_selftest.sh index e232613..4881557 100755 --- a/test/harness_selftest.sh +++ b/test/harness_selftest.sh @@ -285,6 +285,17 @@ check "check_num still fails two unequal numbers" \ check "check_num accepts a decimal and a sign" \ "$(_probe check_num "decimal" "-1.5" "-1.5")" "0" +check "check_text refuses two empty strings, where plain check passes" \ + "$(_probe check_text "empty vs empty" "" "")" "1" +check "check_text refuses one empty side" \ + "$(_probe check_text "one empty" "abc" "")" "1" +check "check_text compares two md5 hashes, which check_num cannot" \ + "$(_probe check_text "md5" "9dd4e461268c8034f5c8564e155c67a6" "9dd4e461268c8034f5c8564e155c67a6")" "0" +check "check_text still fails two different strings" \ + "$(_probe check_text "differ" "abc" "def")" "1" +check "check_num refuses an md5, which is why check_text exists" \ + "$(_probe check_num "md5" "9dd4e461268c8034f5c8564e155c67a6" "9dd4e461268c8034f5c8564e155c67a6")" "1" + check "check_ratio refuses an empty measurement" \ "$(_probe check_ratio "empty" "" "100" "0.5")" "1" check "check_ratio refuses a zero denominator rather than dividing by it" \ diff --git a/test/lib.sh b/test/lib.sh index 4e63340..7e67cfb 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -337,6 +337,33 @@ pgc_is_number() { # $1 -> 0 when $1 is a number return 0 } +# check_text LABEL GOT WANT -- check, with both sides required to be non-empty. +# +# check_num covers a measurement that is a NUMBER. Plenty of oracles are not: an +# md5 over an ordered result, a plan node name, a returned string. check_num +# rejects those outright -- it refuses two identical md5 hashes, because an md5 +# is not a number -- so a suite comparing one has nothing to reach for and falls +# back to plain check, where "" equals "" and prints PASS (#418). +# +# That is the same defect, on the larger half: 35 places in this tree compare an +# md5(string_agg(...)) oracle, and every one of them is a down cluster or an +# errored query away from comparing nothing with nothing. +# +# Deliberately weaker than check_num: it asserts presence, not shape. A caller +# that knows the shape should say so, and native_index_projection.sh's agree() +# additionally requires 32 hex characters before it trusts either side. +check_text() { + local name="$1" got="$2" want="$3" + if [ -z "$got" ] || [ -z "$want" ]; then + PGC_CHECKS=$((PGC_CHECKS + 1)) + PGC_FAIL=1 + echo "FAIL $name: a side is empty, so nothing was compared:" \ + "got [$got] want [$want]" + return 1 + fi + check "$name" "$got" "$want" +} + # check_num LABEL GOT WANT -- check, with both sides required to be numbers. check_num() { local name="$1" got="$2" want="$3" diff --git a/test/native_index_projection.sh b/test/native_index_projection.sh index 919fa6e..77a1664 100755 --- a/test/native_index_projection.sh +++ b/test/native_index_projection.sh @@ -93,9 +93,9 @@ check "partial index finds the row inside its predicate" \ # prints PASS (#418). This oracle is the strongest assertion in the file, so it is the # worst one to have silently comparing two empty strings. # -# Local guard on purpose. #418 proposes `check_num` in `test/lib.sh` and -# @ChronicallyJD owns it; this file should adopt that helper when it lands and drop -# the check below. +# The emptiness half is `check_text` in `test/lib.sh` now (#418, #422), so this file no +# longer carries its own version of it. The shape half stays here: an md5 is 32 hex +# characters, and this file knows that where the shared helper only knows "not empty". agree() { # $1 label, $2 predicate, $3 selected expression local viaix viaseq viaix=$(qset "SET enable_seqscan=off; SET enable_bitmapscan=off" \ @@ -105,11 +105,11 @@ agree() { # $1 label, $2 predicate, $3 selected expression "SELECT md5(string_agg(t::text, ',' ORDER BY t)) FROM (SELECT $3 AS t FROM w WHERE $2) s") if ! grep -qE '^[0-9a-f]{32}$' <<<"$viaix" || ! grep -qE '^[0-9a-f]{32}$' <<<"$viaseq"; then - check "$1 (both sides must be a real result, not empty)" \ + check "$1 (both sides must be a real md5, not empty or an error)" \ "index=[$viaix] seq=[$viaseq]" "two md5 hashes" return fi - check "$1" "$viaix" "$viaseq" + check_text "$1" "$viaix" "$viaseq" } agree "plain index agrees with a sequential scan" "k BETWEEN 1000 AND 9999" "k" agree "late-column index agrees" "c18 > repeat('x',80)||'99000'" "k"