Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 37 additions & 1 deletion test/arrow_import.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions test/native_parquet_schema.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 8 additions & 0 deletions test/parquet_nested_import.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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<int32> and struct<x:int,y:string>.
# This proves we read the Dremel levels the reference writer produces, not just
# our own exporter's byte layout.
Expand Down
Loading