diff --git a/CHANGELOG.md b/CHANGELOG.md index fde889b4..7f66a1ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,58 @@ 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. **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 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 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 + 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..9f37624e 100755 --- a/test/arrow_import.sh +++ b/test/arrow_import.sh @@ -68,7 +68,43 @@ 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 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, 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 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 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 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. +# +# 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. # 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.