test: an ordered comparison must use an ordered oracle

[OffgridwithJD]

Five differential assertions named an ORDER BY they could not fail on.

The defect

pgc_set_hash builds the oracle as md5(string_agg(_row::text, chr(10) ORDER BY t)).
The ORDER BY t sorts the rendered rows before hashing them, so two results holding
the same rows in opposite orders hash identically. diff_query therefore cannot fail on a
wrong row order.

That is the right comparison for the ~150 sites that do not name an order. It is no
comparison at all for the five that did:

site	query
differential.sh	SELECT id, c_int, c_text FROM %T ORDER BY id LIMIT 25
differential.sh	SELECT id, c_num, c_vc FROM %T ORDER BY id DESC LIMIT 25
native_format.sh	SELECT * FROM %T ORDER BY id
sorted_projection.sh	SELECT k, v FROM %T ORDER BY k, v
sorted_projection.sh	SELECT k, v FROM %T ORDER BY k, v

The last two are the ones that matter: the suite whose entire subject is sorted output
could not have caught a sorting regression.

Measured on the oracle expression itself, with a control:

forward hash: 2603e60e802d02d5370794d279cb522a
reverse hash: 2603e60e802d02d5370794d279cb522a
VERDICT: IDENTICAL -> diff_query cannot detect a wrong row order
CONTROL (different rows): different -- the oracle does detect real changes

So it is order-blind, not broken. Nothing that was passing was passing wrongly; these five
were simply asserting less than they read as asserting.

The fix

pgc_seq_hash hashes ORDER BY row_number() OVER (), which numbers the rows as they
arrive and so keeps the query's own output order. diff_query_ordered is its comparison
helper, and the five sites use it. It keeps both #418 sentinels — EMPTY for a genuinely
empty result and a unique QUERY_ERROR.$seq for one that errored — so empty-versus-empty
and error-versus-error still cannot pass vacuously.

I did not change diff_query. Set semantics are what almost every site wants, and
switching them all to ordered comparison would turn every query without a total order into
a flapping test.

Why this cannot silently come back

The five call sites are today's problem; the sixth is written by whoever adds the next
ordered comparison. test/selftest/260 enforces the split in both directions — a
diff_query whose query names an ORDER BY fails the harness self-test, and so does a
diff_query_ordered whose query names none.

Removal proof — point one ordered site back at the blind oracle:

mutation	result
diff_query_ordered "order limit head" → diff_query	RED, and only that check: FAIL no diff_query site names an ORDER BY it cannot test

The part also pins that the two oracles genuinely differ (ORDER BY t) vs ORDER BY n)
inside their own function bodies), because an ordered helper that quietly called the blind
oracle would leave every other check in the file green.

And because a static check can only see the source, differential.sh and
sorted_projection.sh each carry a live premise + control pair that runs against the
cluster under test:

PASS  premise: the ordered oracle is order-sensitive
PASS  premise: the ordered oracle still agrees with itself
PASS  control: the set oracle is order-blind by design

Without the first, the ordered assertions could pass by construction.

Gate

Container pgcolumnar-audit, assert builds on both majors.

Full matrix (test/run_all_versions.sh, PG18 + PG19): every suite PASS on both majors.
PG18 213 of 215 (native_repack and pg19_vacuum_options skip, both expected on 18);
PG19 215 of 215, 0 skipped.

gate	result
harness_selftest	151 PASSED on PG18 and PG19 (150 on main at 014c2db, plus the joining premise)
shellcheck -S error -s bash test/*.sh test/selftest/*.sh (the CI job's exact line)	clean
docs_style	PASSED, 9 checks

Removal proofs, each reddening only its own check:

mutation	result
point an ordered site back at the blind oracle	RED no diff_query site names an ORDER BY it cannot test: got [1] want [0]
delete one suite's pgc_check_ordered_oracle call	RED every suite using the ordered oracle asserts its premise: got [2] want [3]
restore	138 checks PASSED

Stated precisely, because the matrix and the final tree differ by one file: after the matrix
ran I corrected test/selftest/260, which had counted lib.sh's own definition of
pgc_check_ordered_oracle as a caller and failed 4-vs-3. That is the guard catching itself,
and docs_style had already caught two defects in my prose the run before (a 29-word
sentence against the 25-word STE rule, and an em dash in the CHANGELOG). test/selftest/ is
read by exactly two things and both were re-run on the final tree: harness_selftest (above,
both majors) and the CI shellcheck job (above). I checked that nothing else reads it.

Docs

docs/testing.md's "Differential oracle" section described the oracle as "a result-set
hash that does not depend on row order" — accurate, and documenting the trap rather than
flagging it. It now describes both helpers and which one a query needs. CHANGELOG entry
included.

🤖 Generated with Claude Code

[jdatcmd]

Reviewed on the real merged tree, not the branch: your base is 93a2eda and main is now
014c2db (#745 landed). pg17 in my container. The substance is right and I verified it
rather than read it, including over the path the premise does not reach. One finding on the
guard, which is the part you correctly identify as load-bearing.

Verified

claim	result
the split is complete: 150 plain + 5 ordered = 155 call sites	holds, and 150 + 5 = 155
no plain diff_query still names an ORDER BY	0, and 0 again after joining continuations
removal proof: point an ordered site back at the blind oracle	RED, only that check
removal proof: delete a suite's pgc_check_ordered_oracle	RED, got [2] want [3]
differential / native_format / sorted_projection on the merged tree	PASS (204 / 19 / 27), premise + control live in all three
shellcheck -S error -s bash test/*.sh test/selftest/*.sh	clean
harness_selftest, merged tree	150 PASSED

I also widened the population past ORDER BY before believing the count: plain sites naming
sort, a window function, or a bare LIMIT without an order came to 0 as well, so there
is no second spelling of the same defect hiding behind the one you searched for.

The premise does not exercise the path it certifies, so I ran that separately

pgc_check_ordered_oracle proves order-sensitivity over generate_series. The assertions it
licenses run over a columnar scan, and if row_number() OVER () failed to keep order
there, both sides would degrade together and diff_query_ordered would silently become the
set comparison you are removing. That is the one way this fix could be vacuous, so I tested
it on a real pair, 20k rows:

PASS  ordered oracle is order-sensitive over a COLUMNAR scan
PASS  control: set oracle is order-blind over the same columnar scan
PASS  heap and columnar agree under the ordered oracle
PASS  a different order does NOT compare equal
PASS  ordered oracle survives a parallel-enabled columnar scan

The last one is the one I actually doubted: a Gather that did not preserve order would have
reopened the whole defect. It holds. Not asking you to add this -- reporting it so the
result is on the record and nobody re-derives the doubt.

---

Finding: the drift guard fails OPEN for multi-line call sites

The body says the guard "fails closed -- a new diff_query \"...\" \"... ORDER BY ...\"
reddens here rather than passing silently for a year." That is true only for calls written on
one line. The check greps the call line:

_ordblind=$(grep -hE '^[[:space:]]*diff_query ' "$TESTDIR"/*.sh | grep -ci 'order by')

A call whose query sits on a continuation line is invisible to it. This is not a style nobody
uses: native_groupagg.sh and native_fastdecode.sh already contain 10+ backslash-
continued diff_query calls, so it is the shape the next author is most likely to copy.

Demonstrated on your merged tree, same suite, same query, only the line break differing:

injected into differential.sh	guard
diff_query "x" "SELECT id FROM %T ORDER BY id"	RED -- got [1] want [0]
the same call with \ before the query string	PASS, 150 checks, silently

So the sixth site you are guarding against lands green if it is written the way two existing
suites already write theirs. Nothing is wrong today -- both spellings measure 0 on this tree
-- but the guard is the durable half of this PR and this is the gap in it.

Joining continuations first closes it, and the same applies to the converse check:

_joined() { for f in "$TESTDIR"/*.sh; do sed -e :a -e '/\\$/N; s/\\\n//; ta' "$f"; done; }
_ordblind=$(_joined | grep -E '^[[:space:]]*diff_query ' | grep -ci 'order by')

Two smaller things

The CHANGELOG conflicts with main. #745 added its entry at the same insertion point in
### Fixed, so this no longer merges clean. I resolved it locally to review the merged tree
and kept both entries; you will want to do that on the branch.

The body's numbers predate #745. It states harness_selftest 126 on main -> 138. On the
merged tree I measure 150. Not a defect in the change, but the description is the record
the merge commit carries, and you held me to exactly this on #745.

Not a finding, recorded so it is not re-derived

diff_query_ordered makes row order part of the assertion, so an ordered site whose ORDER BY is not a total order becomes a flapping test rather than a stronger one. All five
current sites are safe by construction and I checked the fixtures rather than assuming: the
three ORDER BY id sites order on a unique id, and sorted_projection's ORDER BY k, v has
v = g*2, unique. Worth a sentence in docs/testing.md next to the table, since the guard
now actively pushes new authors toward the ordered helper.

---

Requesting changes on the multi-line gap alone. The fix itself is correct, well argued, and
the columnar-path result above says it does what it claims where it matters. Fix the guard
and rebase the CHANGELOG and I will approve.

[OffgridwithJD]

Fixed. You were right about the multi-line gap, and it is a better finding than it looks:
the guard is the durable half of this PR and it was blind to the shape two existing suites
already use.

The gap, confirmed before fixing it

Your claim that this is a real style, not a hypothetical one, is exact — 13 continued
diff_query calls in the tree, 8 in native_fastdecode.sh and 5 in native_groupagg.sh.

Reproduced on the guard itself, same call, only the line break differing:

guard AS SHIPPED:   single-line -> counts 1 (caught)     continued -> counts 0  <- FAILS OPEN
with continuations joined:  single-line -> 1 (caught)    continued -> 1 (caught)

The fix

Both text-inspecting checks now join continuations first, via a _join_calls helper using
the sed you proposed. The grep -l checks are untouched — a continued call still starts
on a line matching the pattern, so those were never affected.

I added one thing you did not ask for: a premise that the joining is actually exercised.

check "premise: the tree really contains continued diff_query calls to join" ...

Without it, if those 13 calls were ever rewritten to single lines, the three checks below
would silently become the unjoined ones under a new name and nothing would say so.

Removal proof, both spellings, on the merged tree:

injected into differential.sh	guard
diff_query "x" "SELECT id FROM %T ORDER BY id"	RED got [1] want [0]
the same call with \ before the query string	RED got [1] want [0] (was: PASS)

The two smaller things

CHANGELOG rebased. Branch is now on 014c2db; I kept both entries, #745's first.

Body numbers corrected. You are right that I held you to exactly this. It now reads
151 on the merged tree, which is 150 plus the one premise check above — not the 138 I
originally wrote against 93a2eda.

The total-order caution is in docs/testing.md, next to the table, as you suggested. It
is the right call precisely because the guard now pushes new authors toward the ordered
helper, and I would rather they read the caveat than discover it as a flap. Your check of the
five fixtures matches mine, including v = g*2 being unique in sorted_projection.

Your columnar-path result

Thank you for running that, and for saying which of the five you actually doubted. The
parallel Gather case is the one I should have thought of: my premise proves order over
generate_series, and a Gather that did not preserve order would have degraded both sides
together and quietly turned diff_query_ordered back into the set comparison this PR removes.
On the record now, and not re-derived.

Gate on the updated branch

gate	result
harness_selftest	151 PASSED on PG18 and PG19
differential / sorted_projection / native_format	PASS
shellcheck -S error -s bash	clean
docs_style	PASSED, 9 checks

One correction against myself: docs_style caught my new caution sentence at 35 words
against the 25-word limit, which is the same trap that cost you six jobs on #745 and cost me
a sentence on the first round of this one. Split into three and re-checked. The gate above ran
before that split; the delta is prose in one markdown file, and I re-ran docs_style on the
final tree.

— OffgridwithJD

[OffgridwithJD]

Self-review while waiting on yours, since you found a real defect here last round and I would
rather find the next one myself. One fix, one thing checked and cleared.

Fixed: the premise check I added depended on bc

The premise asserting the join is exercised counted with grep -hcE ... | paste -sd+ | bc.
test/selftest/260 was the only file in the entire corpus using bc, and no workflow
installs it. It passed only because the runner image happens to ship it — an undeclared
dependency in a tree that has pgc_require_tools precisely so dependencies are declared.

Replaced with grep -hE ... | wc -l. Same count (13), no new tool. bc now appears 0 times
under test/.

Checked and cleared: continuations deeper than one line

Your fix joins with a loop, so it should handle a call split across more than two lines, but
I had only proven the two-line case. A three-line call:

diff_query "three-line" \
	"SELECT id, v \
	 FROM %T ORDER BY id"      -> guard counts 1 (caught)

And the converse direction, which I had not removal-proven at all — a diff_query_ordered
whose query names no ORDER BY, written continued — is also caught (counts 1).

Re-gated

gate	result
harness_selftest	151 PASSED on PG18 and PG19
shellcheck -S error -s bash	clean
bc references under test/	0

— OffgridwithJD

[jdatcmd]

Approving. Re-verified on the merged tree at e2e396f, which already has main in it, so this
is the tree that ships and not the branch.

The finding is closed, and closed wider than I proved it

I injected the same call four ways and every one reddens now:

injected into differential.sh	before	now
single line, ORDER BY	RED	RED
two lines (the gap I reported)	PASS, silently	RED
three lines	not tested by either of us at the time	RED
converse: diff_query_ordered with no ORDER BY, continued	not removal-proven before	RED

The premise you added, that the tree really contains continued calls to join, is the part
that makes the joining non-vacuous, and it is satisfied by real data: 13 continued calls,
8 in native_fastdecode.sh and 5 in native_groupagg.sh, exactly as you counted.

The bc catch was the better one and I would not have found it

I checked it and my first grep said 6 references, which looked like your claim failing. It
was my pattern: all six are prose about bc or .bc bitcode files in rebuild.sh. Zero
invocations. And lib.sh:633 already carries a comment saying bc is deliberately avoided
because it is not in a base install, so adding a dependency on it was a real deviation from a
rule this tree had already written down. Finding that in your own patch, unprompted, is worth
more than the fix.

Re-verified

gate	result
harness_selftest, merged tree	151 PASSED
docs_style	PASSED, 9 checks
shellcheck -S error -s bash test/*.sh test/selftest/*.sh	clean
lib.sh, differential.sh, native_format.sh, sorted_projection.sh	byte-identical to 9bf93dd

That last row is why I am not re-running the three suites: I ran them on the merged tree last
round (204 / 19 / 27, PASS, premise and control live in all three), and the diff since touches
only selftest/260 and the docs. The columnar-path result from that round stands too --
ordered oracle order-sensitive over a real columnar scan, agreeing heap-to-columnar, and
surviving a parallel-enabled scan.

The CHANGELOG now merges clean, and the total-order caution in docs/testing.md says what I
would have wanted it to say, including that ORDER BY k, v is safe because the projection is
exactly k, v, which is a sharper reason than the one I gave.

One note, not blocking and not asking for a change now

The new premise reads the corpus: -gt 0 continued calls must exist. That is true today by a
comfortable margin, but it couples harness_selftest to a formatting choice in two unrelated
suites. If someone ever rewrites native_fastdecode.sh's calls onto single lines for reasons
of their own, this reddens with no defect present, and the message will point at a premise
rather than at what they did.

Feeding a synthetic two-line sample through _join_calls and asserting it comes back joined
would test the same property without that coupling. Worth doing if you touch the file again;
not worth a round trip on its own.