From b95e68d7aba2537046428f7d66f24bdecf9276f0 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 23 Sep 2026 19:16:31 -0600 Subject: [PATCH 1/3] docs: say what the count counted, and why these two are not terminal (#1215) `arrow_import.sh` explains why it does not use `pgc_skip` and said "THE OTHER 29 pyarrow SUITES USE" it. Recounted, the number is 25: the suites whose source, with comments stripped, calls `pgc_skip` with the pyarrow capability. 29 is not reproducible now under any population except one counting `lib.sh`, where the function is defined, plus three suites that gate on other capabilities and name pyarrow only in prose. Whether it was right when written is unknown, and the comment now says that rather than carrying a bare number. The recount is a worked example of the defect it fixes. The first attempt returned 26, because searching `arrow_import.sh` for `pgc_skip` matches the sentence explaining why it does not call `pgc_skip`. Comments stripped, that file scores 0. `native_parquet_schema.sh` and `parquet_nested_import.sh` decline arm by arm rather than being terminal, as `arrow_import.sh` does, but unlike it said nothing about why. A reader arriving at either had no way to tell a deliberate deviation from an oversight. Both now carry the measurement that justifies it: 29 of 35 arms and 5 of 7 need no pyarrow, the name set is identical with and without it, and both runs still exit 0. Comments only. No test changed, no check added, no behaviour changed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP --- CHANGELOG.md | 32 ++++++++++++++++++++++++++++++++ test/arrow_import.sh | 14 +++++++++++++- test/native_parquet_schema.sh | 12 ++++++++++++ test/parquet_nested_import.sh | 8 ++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fde889b4..e4ab7211 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,38 @@ true until the next version shipped. operator with the same rows the scan returns* can be satisfied without an index scan happening. Reported on #1236 rather than repaired here, because this change records evidence and alters no test. +- Three suite comments now say what they counted and why they deviate (#1215). + + `arrow_import.sh` explained why it does not use `pgc_skip` and said "THE OTHER + 29 pyarrow SUITES USE" it. Recounted, the number is **25** -- the suites whose + source, comments stripped, calls `pgc_skip` with the pyarrow capability. 29 is + not reproducible now under any population except one counting `lib.sh`, where + the function is defined, plus three suites that gate on other capabilities and + name pyarrow only in prose. Whether it was right when written is unknown, and + the comment now says so rather than carrying a bare number. + + The recount is also a worked example of the defect it fixes: the first attempt + returned 26, because searching `arrow_import.sh` for `pgc_skip` matches the + sentence explaining why it does not call `pgc_skip`. With comments stripped + that file scores 0. + + `native_parquet_schema.sh` and `parquet_nested_import.sh` decline arm by arm + instead of being terminal, like `arrow_import.sh`, but unlike it said nothing + about why -- leaving a reader unable to tell a deliberate deviation from an + oversight. Both now carry the measurement that justifies it: + + ``` + native_parquet_schema with pyarrow 35 passed + 0 skipped = 35 + without 29 passed + 6 skipped = 35 + parquet_nested_import with pyarrow 7 passed + 0 skipped = 7 + without 5 passed + 2 skipped = 7 + ``` + + Identical name sets either way -- 35 and 7 records in both runs -- and both + still exit 0. Terminal is right for a suite with nothing else to do, and + neither is that suite. + + Comments only. No test changed, no check added, no behaviour changed. - Fifteen `native_join_runtime_filter` arms now carry a mutation that reddens them (#1236). No test changed and no code changed: the arms were attacked and diff --git a/test/arrow_import.sh b/test/arrow_import.sh index 4b9de5fd..3dc93fed 100755 --- a/test/arrow_import.sh +++ b/test/arrow_import.sh @@ -68,7 +68,19 @@ fifo_release() { exec 9<>"$1" 2>/dev/null; exec 9>&- 2>/dev/null; } # sites produce 56 records because some sit in loops, so a list copied from the # `check` lines would have been short by five. # -# WHY NOT `pgc_skip`, WHICH THE OTHER 29 pyarrow SUITES USE. It is terminal: it +# WHY NOT `pgc_skip`, WHICH THE 25 SUITES THAT CALL IT FOR pyarrow USE. +# The count names its population, because an unqualified one goes stale in +# silence: it is the suites under test/ whose source, with comments stripped, +# calls pgc_skip with the pyarrow capability. Stripping matters -- searching +# this file for the helper's name matches the sentence you are reading, which +# is how the first recount came back one too high. +# +# It said 29 before. That is not reproducible now under any population I could +# construct except one counting lib.sh, where the function is defined, plus +# three suites that gate on other capabilities and name pyarrow only in prose +# (#1215). Whether it was right when written is unknown. +# +# pgc_skip is terminal: it # ends the run with a named FAIL, or a named SKIP when the dependency is waived. # That is right for a suite with nothing else to do and wrong here, because 16 of # these checks need no pyarrow at all -- the round trip through our own writer, diff --git a/test/native_parquet_schema.sh b/test/native_parquet_schema.sh index 746a8d29..6ec9de4d 100755 --- a/test/native_parquet_schema.sh +++ b/test/native_parquet_schema.sh @@ -84,6 +84,18 @@ chmod 644 "$notpq" err="$(q "SELECT count(*) FROM pgcolumnar.parquet_schema('$notpq');" 2>&1)" check "bad magic rejected (empty At output)" "$err" "" +# WHY NOT `pgc_skip`, which 25 pyarrow suites use (#1215). It is terminal, and +# that is right for a suite with nothing else to do. Measured on pg18a, this +# suite is not that suite: +# +# with pyarrow 35 passed + 0 skipped = 35 +# without pyarrow 29 passed + 6 skipped = 35 +# +# 29 of the 35 arms need no pyarrow, the name set is identical in both runs -- +# 35 records either way -- and the run still exits 0. Ending the suite would +# trade one silence for a larger loss. The six that do need it decline by name +# below, which is the #1159 model: the coverage lost stays visible as skips +# rather than becoming a pass. # ---- nullable=false path (needs a REQUIRED column; only pyarrow writes one) --- if python3 -c 'import pyarrow.parquet' 2>/dev/null; then REQ="$PGC_WORKDIR/required.parquet" diff --git a/test/parquet_nested_import.sh b/test/parquet_nested_import.sh index 6a07b246..cdae8488 100755 --- a/test/parquet_nested_import.sh +++ b/test/parquet_nested_import.sh @@ -46,6 +46,14 @@ check "round-trips after delete" \ "$(pgc_set_hash "$SEL pna")" # --------------------------------------------------------------------------- +# WHY NOT `pgc_skip`, which 25 pyarrow suites use (#1215). It is terminal, and +# only two of these seven arms need pyarrow. Measured on pg18a: +# +# with pyarrow 7 passed + 0 skipped = 7 +# without pyarrow 5 passed + 2 skipped = 7 +# +# Same name set both ways, 7 records either way, and rc=0 without it. The two +# that need the reference writer decline by name below. # pyarrow-written nested Parquet file: list and struct. # This proves we read the Dremel levels the reference writer produces, not just # our own exporter's byte layout. From 6e4e4bc6d6a8fc31d548c669a4ccee525f9058fb Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 23 Sep 2026 21:11:43 -0600 Subject: [PATCH 2/3] docs: 29 was counted honestly and attached to the wrong population (#1215) The previous version of this comment said 29 "is not reproducible now under any population I could construct" and left whether it was right when written unknown. Both halves were wrong to leave standing. Counted in a detached worktree at f738dd4d, the commit that wrote the sentence, excluding arrow_import.sh, lib.sh and the three drivers: at f738dd4d today names pyarrow anywhere 34 35 runs `import pyarrow` 29 30 calls pgc_skip pyarrow 25 25 So 29 reproduces at that commit, by the `import pyarrow` reading and only that one. The sentence is about the suites that USE pgc_skip, and 25 did -- then and now. The number was right for one population and attached to another in the same breath, which is a more useful thing for the next reader to learn than a number having gone stale. Raised by @jdatcmd, who reconstructed the 29. Their message also said it held "by either pattern"; the wide reading was already 34 at that commit, so only the narrow one gives 29. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP --- CHANGELOG.md | 25 +++++++++++++++++++------ test/arrow_import.sh | 35 +++++++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4ab7211..a7f04e6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,12 +49,25 @@ true until the next version shipped. - Three suite comments now say what they counted and why they deviate (#1215). `arrow_import.sh` explained why it does not use `pgc_skip` and said "THE OTHER - 29 pyarrow SUITES USE" it. Recounted, the number is **25** -- the suites whose - source, comments stripped, calls `pgc_skip` with the pyarrow capability. 29 is - not reproducible now under any population except one counting `lib.sh`, where - the function is defined, plus three suites that gate on other capabilities and - name pyarrow only in prose. Whether it was right when written is unknown, and - the comment now says so rather than carrying a bare number. + 29 pyarrow SUITES USE" it. **29 was honestly counted and attached to the wrong + noun.** At `f738dd4d`, the commit that wrote the sentence, 29 suites other than + that one ran `import pyarrow` -- but the sentence is about the suites that USE + `pgc_skip`, and 25 did. Measured in a worktree at that commit, excluding + `arrow_import.sh`, `lib.sh` and the three drivers: + + ``` + at f738dd4d today + names pyarrow 34 35 + runs `import pyarrow` 29 30 + calls pgc_skip pyarrow 25 25 + ``` + + 29 reproduces at that commit by the `import pyarrow` reading and only that one; + naming pyarrow anywhere gave 34 even then. 25 has not moved. The count that + drifted is the one the sentence did not mean -- `capability_sweep.sh` joined the + pyarrow population when #1235 landed. The comment now carries all three rows, so + a reader learns the transferable thing: a number can be correct and still be + attached to the wrong population. The recount is also a worked example of the defect it fixes: the first attempt returned 26, because searching `arrow_import.sh` for `pgc_skip` matches the diff --git a/test/arrow_import.sh b/test/arrow_import.sh index 3dc93fed..09257b4b 100755 --- a/test/arrow_import.sh +++ b/test/arrow_import.sh @@ -68,17 +68,32 @@ fifo_release() { exec 9<>"$1" 2>/dev/null; exec 9>&- 2>/dev/null; } # sites produce 56 records because some sit in loops, so a list copied from the # `check` lines would have been short by five. # -# WHY NOT `pgc_skip`, WHICH THE 25 SUITES THAT CALL IT FOR pyarrow USE. -# The count names its population, because an unqualified one goes stale in -# silence: it is the suites under test/ whose source, with comments stripped, -# calls pgc_skip with the pyarrow capability. Stripping matters -- searching -# this file for the helper's name matches the sentence you are reading, which -# is how the first recount came back one too high. +# WHY NOT `pgc_skip`, WHICH 25 SUITES USE FOR pyarrow. The count names its +# population because the number this sentence used to carry did not, and that +# is the whole lesson here. # -# It said 29 before. That is not reproducible now under any population I could -# construct except one counting lib.sh, where the function is defined, plus -# three suites that gate on other capabilities and name pyarrow only in prose -# (#1215). Whether it was right when written is unknown. +# IT SAID 29, AND 29 WAS HONESTLY COUNTED. At f738dd4d, the commit that wrote +# the sentence, 29 suites other than this one ran `import pyarrow`. But the +# sentence is about the suites that USE `pgc_skip`, and 25 did. The number was +# right for one population and attached to another in the same breath. +# Measured, excluding this file, lib.sh and the three drivers: +# +# at f738dd4d today +# names pyarrow 34 35 +# runs `import pyarrow` 29 30 +# calls pgc_skip pyarrow 25 25 +# +# So 29 is reproducible at that commit by the `import pyarrow` reading and by +# that reading only -- naming pyarrow anywhere gave 34 even then. 25 has not +# moved at all. The number that drifted is the one the sentence did not mean: +# capability_sweep.sh joined the pyarrow population when #1235 landed. +# +# Read at that commit with `git worktree add --detach`. `git archive` returns +# nothing for test/*.sh, because .gitattributes export-ignores them. +# +# COUNTING HERE NEEDS COMMENTS STRIPPED: searching this file for the helper's +# name matches the sentence you are reading, which is how a recount came back +# one too high. (#1215) # # pgc_skip is terminal: it # ends the run with a named FAIL, or a named SKIP when the dependency is waived. From 8bd737bc34d7f9b1836b4883b9717da3cbe6ace2 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Wed, 23 Sep 2026 21:22:49 -0600 Subject: [PATCH 3/3] docs: name the predicate on every row of the pyarrow count (#1215) The previous version gave three rows and said 29 reproduces "by the `import pyarrow` reading and by that reading only -- naming pyarrow anywhere gave 34 even then". True, and it omitted the predicate that actually separates them, which is comment stripping. Measured four ways at f738dd4d and today, same exclusions: at f738dd4d today names pyarrow anywhere 34 35 names pyarrow in CODE 29 30 runs `import pyarrow` in code 29 30 imports it AND calls pgc_skip 25 25 The five in the 34/29 gap name pyarrow only to say they do NOT use it: native_parquet_stack and parallel_export_parquet hand-craft their fixtures, and avro_manifest, iceberg_deletes and iceberg_name_mapping gate on other capabilities. Which is the same house-style trap as searching this file for `pgc_skip` and matching the sentence that says why not to use it. Three rows invite the reader to guess which "names pyarrow" means. Two reviewers guessing differently is how 34 and 29 were taken for a disagreement when both were right about different populations. @jdatcmd asked for the fourth row and the predicate on each. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP --- CHANGELOG.md | 27 +++++++++++++++++---------- test/arrow_import.sh | 23 ++++++++++++++++------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7f04e6d..7f66a1ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,18 +56,25 @@ true until the next version shipped. `arrow_import.sh`, `lib.sh` and the three drivers: ``` - at f738dd4d today - names pyarrow 34 35 - runs `import pyarrow` 29 30 - calls pgc_skip pyarrow 25 25 + at f738dd4d today + names pyarrow anywhere 34 35 + names pyarrow in CODE 29 30 + runs `import pyarrow` in code 29 30 + imports it AND calls pgc_skip 25 25 ``` - 29 reproduces at that commit by the `import pyarrow` reading and only that one; - naming pyarrow anywhere gave 34 even then. 25 has not moved. The count that - drifted is the one the sentence did not mean -- `capability_sweep.sh` joined the - pyarrow population when #1235 landed. The comment now carries all three rows, so - a reader learns the transferable thing: a number can be correct and still be - attached to the wrong population. + **The predicate separating 34 from 29 is comment stripping**, and the five in + the gap name pyarrow only to say they do not use it -- two hand-craft their + fixtures, three gate on other capabilities. Same house-style trap as searching + `arrow_import.sh` for `pgc_skip` and matching the sentence that says why not to + use it. + + So 29 was honestly counted, of the suites whose *code* names pyarrow. 25 has not + moved. The count that drifted is the one the sentence did not mean -- + `capability_sweep.sh` joined the population when #1235 landed. All four rows go + in the comment, with the predicate named on each, because three rows invite the + reader to guess which "names pyarrow" means -- and guessing differently is how + two reviewers got different totals for the same tree before reconciling them. The recount is also a worked example of the defect it fixes: the first attempt returned 26, because searching `arrow_import.sh` for `pgc_skip` matches the diff --git a/test/arrow_import.sh b/test/arrow_import.sh index 09257b4b..9f37624e 100755 --- a/test/arrow_import.sh +++ b/test/arrow_import.sh @@ -76,15 +76,24 @@ fifo_release() { exec 9<>"$1" 2>/dev/null; exec 9>&- 2>/dev/null; } # the sentence, 29 suites other than this one ran `import pyarrow`. But the # sentence is about the suites that USE `pgc_skip`, and 25 did. The number was # right for one population and attached to another in the same breath. -# Measured, excluding this file, lib.sh and the three drivers: +# Measured four ways, because three invites the reader to guess which "names +# pyarrow" means -- and guessing wrong is how two of us got different totals for +# the same tree. Excluding this file, lib.sh and the three drivers: # -# at f738dd4d today -# names pyarrow 34 35 -# runs `import pyarrow` 29 30 -# calls pgc_skip pyarrow 25 25 +# at f738dd4d today +# names pyarrow anywhere 34 35 +# names pyarrow in CODE 29 30 +# runs `import pyarrow` in code 29 30 +# imports it AND calls pgc_skip 25 25 # -# So 29 is reproducible at that commit by the `import pyarrow` reading and by -# that reading only -- naming pyarrow anywhere gave 34 even then. 25 has not +# THE PREDICATE THAT SEPARATES 34 FROM 29 IS COMMENT STRIPPING. The five in the +# gap name pyarrow only to say they do NOT use it: native_parquet_stack and +# parallel_export_parquet hand-craft their fixtures, and avro_manifest, +# iceberg_deletes and iceberg_name_mapping gate on other capabilities. Which is +# the same house-style trap as searching this file for the helper's name and +# matching the sentence that says WHY NOT to use it. +# +# So 29 was honestly counted, of the suites whose CODE names pyarrow. 25 has not # moved at all. The number that drifted is the one the sentence did not mean: # capability_sweep.sh joined the pyarrow population when #1235 landed. #