From 0f6e1ef795684bca9f96718dfb66c1e099ce82e9 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Sat, 12 Sep 2026 00:54:09 +0000 Subject: [PATCH] test: the object-storage loops name the case each iteration tests (#982) objstore_module lost the most records of any suite to key collapsing: two loops of three iterations, where the names did not carry what the iteration varies. clean main 30 records 24 distinct keys 3 colliding 6 lost this branch 30 records 30 distinct keys 0 colliding 0 lost Record count unchanged, so this adds and removes no checks. THE TWO LOOPS SHOW BOTH SUB-SHAPES. In the first the headline already interpolated the scheme and only the continuation was short. In the second NEITHER name carried the metacharacter, so three iterations produced one key for each of TWO checks -- the headline collided too, which the first loop did not. Both are fixed by the rule #984 proposed: the continuation carries the same discriminator its headline names, and where the headline names none either, it gains one. The discriminators come from the loop variable by parameter expansion, so they were already in scope: a remote glob [*] is handled remotely, an object-storage error not a local one and the [*] glob is NOT reported as a local filesystem miss and the https URL is NOT reported as a missing file The first loop's headline now reads its scheme from a variable rather than a cut subshell because both names need it; the resulting check name is byte-identical, so no ledger row moves on account of it. No ledger change: objstore_module is not one of the two suites the ledger covers, which is why 21 of the 24 collisions matter for #432's seeding rather than for today's census. objstore_module 30 checks 0 FAIL on both trees | docs_style 9/9 shellcheck clean | ledger and budget untouched The commit message for this change had to be written to a file rather than passed inline: an apostrophe in a -m argument closed the quote and the shell then tried to execute the glob examples above. Third inline-quoting fault of the session and the rule is mine: write the script, do not inline it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 39 +++++++++++++++++++++++++++++++++++++++ test/objstore_module.sh | 17 +++++++++++++---- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c4059d5..14c81059 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1848,6 +1848,45 @@ true until the next version shipped. `moved s4d1's layout`, `no row was lost from s4d2` -- for the same reason. No ledger change: `hilbert_cluster` is not one of the two suites the ledger covers. +- The object-storage loops name the case each iteration tests, so six checks stop + sharing three ledger keys (#982, second of eight). + + `objstore_module` lost the most records of any suite to key collapsing: two loops of + three iterations each, where the check names did not carry the thing the iteration + varies. Measured against a control run on clean `main`: + + clean main 30 records 24 distinct keys 3 colliding 6 lost + this branch 30 records 30 distinct keys 0 colliding 0 lost + + The record count is unchanged, so this adds and removes no checks. + + **The two loops show both sub-shapes of the same defect.** In the first, the headline + already interpolated the scheme and only the continuation was short: + + check "a s3 URL reports an object-storage error, not a missing file" + check_num "and does NOT report it as a missing file" <- identical three times + + In the second, *neither* name carried the metacharacter, so three iterations produced + one key for each of **two** checks -- the headline collided as well. + + Both are fixed by the rule #984 proposed: the continuation carries the same + discriminator its headline names, and where the headline does not name one either, it + gains it. The discriminators come from the loop variable by parameter expansion + (`${url%%:*}` and stripping the fixed prefix and suffix off the pattern), so they are + already in scope: + + a remote glob (*) is handled remotely (an object-storage error, not a local one) + and the * glob is NOT reported as a local filesystem miss + and the https URL is NOT reported as a missing file + + The first loop's headline now reads its scheme from a variable rather than a `cut` + subshell, because both names need it. The resulting check name is byte-identical, so + no ledger row moves on account of it. + + No ledger change at all: `objstore_module` is not one of the two suites the ledger + covers, so its check names have no rows. That is also why the twenty-four collisions + matter for #432 rather than for the census today -- twenty-one of them are in suites + that become covered only when the 240 are seeded. ## [1.0-alpha3] - 2026-09-02 diff --git a/test/objstore_module.sh b/test/objstore_module.sh index 4f03b08b..a54984b8 100755 --- a/test/objstore_module.sh +++ b/test/objstore_module.sh @@ -102,10 +102,14 @@ check_num "positive control: it IS defined in the module, so nm really looked" \ # A remote path must report a remote error, from the reader, without a connection. for url in "s3://bucket/key.parquet" "gs://bucket/key.parquet" "https://host/key.parquet"; do + # The scheme, by parameter expansion rather than a `cut` subshell, because BOTH + # names need it now: the headline already carried it and the continuation did not, + # which is why three iterations produced one ledger key (#982). + scheme="${url%%:*}" out=$(psql_run "SELECT * FROM pgcolumnar.read_parquet('$url') AS (a int)" 2>&1) - check "a $(cut -d: -f1 <<<"$url") URL reports an object-storage error, not a missing file" \ + check "a $scheme URL reports an object-storage error, not a missing file" \ "$([ "$(grep -c 'object storage is not implemented\|is not supported\|requires the object-store module\|requires AWS_\|could not resolve\|could not connect\|objstore_allowed_endpoints' <<<"$out")" -ge 1 ] && echo yes || echo no)" "yes" - check_num "and does NOT report it as a missing file" \ + check_num "and the $scheme URL is NOT reported as a missing file" \ "$(grep -c 'No such file or directory' <<<"$out")" "0" done @@ -119,10 +123,15 @@ done # storage error, and the invariant this arm still guards is that it is NEVER a # local filesystem miss. `*`, `?` and `[` are all legal in an S3 key. for pat in "s3://bucket/a*.parquet" "s3://bucket/a?.parquet" "s3://bucket/a[0-9].parquet"; do + # The metacharacter under test, which is what distinguishes the three iterations. + # Here BOTH names collided: unlike the loop above, the headline did not carry it + # either, so three iterations produced one key for each of two checks (#982). + meta="${pat#s3://bucket/a}" + meta="${meta%.parquet}" out=$(psql_run "SELECT * FROM pgcolumnar.read_parquet('$pat') AS (a int)" 2>&1) - check "a remote glob is handled remotely (an object-storage error, not a local one)" \ + check "a remote glob ($meta) is handled remotely (an object-storage error, not a local one)" \ "$([ "$(grep -c 'requires AWS_\|object storage\|objstore_allowed_endpoints\|requires the object-store module\|could not resolve\|could not connect\|is not in pgcolumnar' <<<"$out")" -ge 1 ] && echo yes || echo no)" "yes" - check_num "and it is NOT reported as a local filesystem miss" \ + check_num "and the $meta glob is NOT reported as a local filesystem miss" \ "$(grep -c 'no files match pattern\|matched no regular files\|No such file or directory' <<<"$out")" "0" done