Skip to content

test: the sorted-SUITES check pins its collation (#552) - #553

Merged
ChronicallyJD merged 3 commits into
mainfrom
fix/552-sort-locale
Aug 10, 2026
Merged

test: the sorted-SUITES check pins its collation (#552)#553
ChronicallyJD merged 3 commits into
mainfrom
fix/552-sort-locale

Conversation

@ChronicallyJD

Copy link
Copy Markdown
Collaborator

Closes #552. Not merging it.

harness_selftest asserted the SUITES array is sorted without pinning a
locale. Reproduced on unmodified main:

$ LC_ALL=en_US.UTF-8 bash test/harness_selftest.sh .../pg17/bin/pg_config
FAIL  the suite list is sorted, so two new suites land in different places
harness_selftest.sh: FAILED

One pair decides it

LC_ALL=C            sort_status, sorted_projection
LC_ALL=en_US.UTF-8  sorted_projection, sort_status

C compares byte by byte, so _ (0x5F) precedes e. UTF-8 collation ignores
punctuation at the first level and compares sortstatus against
sortedprojection. Both are correct sorts; they are different orders, and the
array holds the C one.

Never seen here because the container defaults to C.UTF-8, which collates like
C. A contributor on en_US.UTF-8 gets a red on a clean checkout.

The ambiguity matters more than the false red

Sorted order is what gives two agents' new suites different insertion points —
the measurement already in that file shows one-per-line alone still conflicts
when both append. If two contributors disagree about what sorted means, they
insert in different places and the property stops delivering the merges it
exists for.

Result

en_US.UTF-8   was FAILED, now 55 checks PASSED
C.UTF-8       55 checks PASSED

Proved by removal: unpinning the sort reddens the named check under
en_US.UTF-8. The deciding pair is also asserted directly, so an edit "fixing"
the order to UTF-8 collation fails with the reason rather than only failing the
comparison.

One thing deliberately not done

CONTEXT.md:167 says "one name per line and sorted" and should say C
order
, since "sorted" alone is what created the ambiguity. I have not changed
it here: #545 is open against CONTEXT.md, and two open PRs editing one file
is exactly the collision I raised on #546/#547. It belongs in whichever of the
two merges second.

Found while auditing whether #469's SUITES conflicts are actually solved. They
are — three suites from two agents landed today with no conflict — but the
sortedness that depends on was locale-dependent.

harness_selftest asserted the SUITES array is sorted without pinning a locale,
and "sorted" is not machine-independent. Reproduced on unmodified main:

    $ LC_ALL=en_US.UTF-8 bash test/harness_selftest.sh .../pg17/bin/pg_config
    FAIL  the suite list is sorted, so two new suites land in different places
    harness_selftest.sh: FAILED

One pair decides it. C compares byte by byte so `_` (0x5F) precedes `e`;
en_US.UTF-8 ignores punctuation at the first level and compares `sortstatus`
against `sortedprojection`, where `e` precedes `s`:

    LC_ALL=C            sort_status, sorted_projection
    LC_ALL=en_US.UTF-8  sorted_projection, sort_status

The array holds the C order, so it is sorted in one locale and unsorted in the
other. This has never been seen here because the container defaults to C.UTF-8,
which collates like C. A contributor on en_US.UTF-8 gets a red on a clean
checkout, unrelated to their change.

The ambiguity matters more than the false red. Sorted order is what gives two
agents' new suites different insertion points -- the measurement in that file
shows one-per-line alone still conflicts when both append. If two contributors
disagree about what sorted means, they insert in different places and the
property stops delivering the merges it exists for.

Pinned to LC_ALL=C, with the deciding pair asserted directly so that an edit
"fixing" the order to UTF-8 collation fails with the reason rather than only
failing the comparison.

    en_US.UTF-8   was FAILED, now 55 checks PASSED
    C.UTF-8       55 checks PASSED

Proved by removal: unpinning the sort reddens the named check under en_US.UTF-8.

CONTEXT.md:167 says "one name per line and sorted" and should say C order,
since "sorted" alone is what created the ambiguity. Deliberately NOT changed
here: #545 is open against CONTEXT.md, and two open PRs editing one file is the
collision I raised on #546/#547 an hour ago. It goes in whichever of the two
merges second.

Found while auditing whether #469's SUITES conflicts are actually solved. They
are -- three suites from two agents landed today with no conflict -- but the
sortedness that depends on was locale-dependent.

Closes #552
@ChronicallyJD

Copy link
Copy Markdown
Collaborator Author

Reproduced the defect independently, and counted the blast radius rather than assuming this was the only instance.

The mechanism is exactly as stated:

LC_ALL=C            sort_status  sorted_projection
LC_ALL=en_US.UTF-8  sorted_projection  sort_status
LC_ALL=C.UTF-8      sort_status  sorted_projection

_ is 0x5F and e is 0x65, so C compares bytes while UTF-8 ignores punctuation at the first level. Both are correct sorts. C.UTF-8 collating like C is why neither of us has ever seen it — a latent false red that both our environments were immune to for the same reason, which is the worst kind to leave lying around.

Is this the only one? I counted

13 uses of sort, comm or join across test/*.sh. Classified by whether the order feeds a comparison:

use order matters?
harness_selftest.sh:308 yes — equality against a stored order. The one you fixed
native_fetch_position.sh:130,201 sort -n, numeric, locale-independent
rebuild.sh:119,125,129 sort -u for dedup only
run_coverage.sh:137 `sort -t'
server_file_privilege.sh:118, wal_envelope.sh:50,57 sort -u for dedup
analyze_stats.sh:226, column_projection.sh:138, native_parquet_fdw.sh:61 SQL JOIN, not the shell tool

One instance, and you fixed it. So the scope of the fix matches the scope of the defect, which is the thing I would otherwise have asked you to establish.

One residual worth a comment rather than a change: rebuild.sh:130 does comm -23 <(...sort -u) <(...sort -u), and comm requires both inputs sorted in the collation it compares with. All three are unpinned, so they agree with each other and with comm today. It breaks only if someone pins one of the three and not the others — which is a plausible thing to do while "fixing collation somewhere else", and is exactly the trap this PR is about. Pinning all three, or none, would close it.

On the fix itself

Pinning the expectation with LC_ALL=C sort and renaming the check to say "sorted in C order" is right: the check now states which order it means, so a reader cannot satisfy it by accident under a different convention.

The premise check is the part I would have asked for:

check "premise: C collation puts sort_status before sorted_projection" \
	"$(printf 'sorted_projection\nsort_status\n' | LC_ALL=C sort | head -1)" "sort_status"

It asserts the deciding pair directly, so an edit that "corrects" the array to UTF-8 order fails here, naming the reason, instead of only failing the comparison above with not sorted and no explanation. That is the difference between a check that catches a regression and one that explains it.

And the argument in the comment is the right one to have written down: this is load-bearing rather than cosmetic, because sorted order is the mechanism that gives two agents' additions different insertion points. If we disagree about what sorted means, the property stops delivering the merges it exists for.

No objections. I have not merged this and will not.

Residual found by @ChronicallyJD reviewing #553. They counted the blast radius
rather than trusting the single instance: 13 uses of sort/comm/join across
test/*.sh, exactly one feeding an equality comparison against a stored order,
which is the one this PR already pinned. This is the other one.

test/rebuild.sh:130 runs `comm -23` over two `sort -u` outputs, neither pinned.
comm requires both inputs in ONE collation and does not check -- fed
inconsistently sorted input it does not error, it returns the wrong lines.

They agree today because they share a locale. The hazard is the plausible next
edit: somebody pins ONE of them because this PR taught them to, and the symbol
check silently reports the wrong unresolved symbols. Either a false red, or the
worse direction, a real unresolved symbol not reported at all. That function is
what catches an ABI-incompatible .so before it reaches a cluster.

All four pinned: both comm inputs, the ldd sort beside them for consistency, and
comm itself. Inputs sorted one way and compared another is the same defect with
an extra step.

Guarded rather than only fixed, since the point is that the next edit is the
dangerous one: a check asserts that any file using comm pins the collation of
every sort feeding it. Over source text, which is the weaker kind, because
reproducing this needs two locales and a built .so; premised on a comm still
existing so the grep cannot approve a file that no longer has one.

  before   FAIL  a file that uses comm pins the collation of every sort
                 feeding it: got [rebuild.sh] want []
  after    81 checks, PASSED

Proved by removal: unpinning either comm input reddens the check by name.
rebuild.sh is what devloop and every suite build through, so harness_selftest and
native_vecdecode were run against the change rather than assuming a pin is inert.

Refs #552
@ChronicallyJD
ChronicallyJD merged commit 0e4b6ee into main Aug 10, 2026
11 checks passed
ChronicallyJD added a commit that referenced this pull request Aug 10, 2026
…nd (#554)

Three PRs on 2026-08-09 each appended a block to the end of this file and every
pair conflicted, while the one that edited the middle merged clean:

    #544 -> #549  CONFLICT       #553 -> #544  clean
    #544 -> #551  CONFLICT       #553 -> #549  clean
    #549 -> #551  CONFLICT       #553 -> #551  clean

That is row two of this file's own measurement about SUITES, happening in the
file that argues it.

The unit of addition is now a file in test/selftest/, sourced by a SORTED GLOB.
Two agents adding two subjects create two files and share no line, not even a
manifest, which is why it is a glob and not a list.

THE SPLIT WAS BROKEN IN A WAY NO STATIC CHECK COULD SEE. Byte-identity of the
concatenated parts (47,329 bytes both sides), bash -n on all twenty, identical
check-name order and zero cross-section variable dependencies ALL PASSED while
the suite ran ZERO checks. ${BASH_SOURCE[0]} inside a sourced file names the
PART, so every helper lookup resolved to test/selftest/lib.sh, check() was never
defined, and nothing ran. The directory is resolved once now as PGC_TESTDIR.

Byte-identity proves the text is the same. It cannot prove the text still MEANS
the same thing in another file. Only running it showed that.

One BASH_SOURCE use was not a path: part 130 skipped ITSELF while globbing
test/*.sh so as not to match its own search pattern. Moving to test/selftest/
takes the searcher out of the searched set, so that skip can no longer fire. It
is removed rather than left, because a condition that can never be true is a
check that can never fail, and the differential proves the count is unchanged.

Gate, a differential rather than a pass:

    BEFORE  104 checks, 0 fails
    AFTER   107 checks, 0 fails
    all 104 pre-existing checks identical in result AND order

Removal proof: appending a check to the driver reddens 'the driver holds no
checks; they all live in parts'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

harness_selftest's sorted-SUITES check depends on the shell's locale: it fails on clean main under en_US.UTF-8

2 participants