Skip to content

21 suites report PASSED with zero checks when pyarrow is missing, so the whole Arrow and Parquet surface can leave the gate silently #447

Description

@ChronicallyJD

What happened

I set out to verify #446 by running the Parquet suites myself. They all reported PASSED. Then
I looked at the check counts:

  native_parquet_pushdown             0 checks  PASSED
  native_parquet_projection           0 checks  PASSED
  native_parquet_multifile            0 checks  PASSED
  native_parquet_partition            0 checks  PASSED
  native_parquet_streaming            0 checks  PASSED
  native_parquet_codecs               0 checks  PASSED
  native_parquet_hardening            0 checks  PASSED
  fuzz_parquet                        0 checks  PASSED

The bench had no pyarrow. Each of those suites does this and stops:

echo "SKIP  pyarrow not available; Parquet hardening suite needs it"
pgc_summary
exit 0

exit 0 is a pass. After apt-get install python3-pyarrow on the same tree and the same
build, the same ten suites ran 190 checks. Nothing about the code changed between the two
runs. Only the dependency did.

Why this matters more than it looks

21 suites gate on pyarrow, which is the entire Arrow and Parquet surface:

arrow_export arrow_import arrow_nested fuzz_arrow fuzz_parquet
native_parquet_codecs native_parquet_flba native_parquet_hardening
native_parquet_multifile native_parquet_partition native_parquet_projection
native_parquet_pushdown native_parquet_schema native_parquet_streaming
native_parquet_units parquet_export parquet_import parquet_nested
parquet_nested_import native_read_parquet native_parquet_fdw

On a machine without pyarrow, run_all_versions.sh runs them, gets exit 0 from every one,
and reports the run green. The output says PASSED next to each. There is no aggregate line
that says 21 of 123 suites tested nothing.

That includes both Parquet fuzzers. The suites most likely to catch a decode bug on
attacker-influenced bytes are the ones that vanish most quietly, because their skip looks
exactly like their pass.

This is the same defect class we already fixed once

#422 fixed run_all_versions.sh reporting ALL VERSIONS PASSED when five of six versions
were skipped, because the default prefixes did not match the machine. The fix was a
VERSIONS_RUN counter, NO VERSIONS RAN at zero, and VERSIONS RUN PASSED (n of m)
otherwise. This is the identical shape one level down: a skipped unit reporting the same
verdict as a passed one.

Suggested fix, matching what #422 did

  1. A suite that ran zero checks should not print PASSED. pgc_summary already knows the
    count, so it can print SKIPPED (0 checks) and exit with a distinct status.
  2. run_all_versions.sh should count skipped suites and report n of m suites ran, failing
    if that is below a floor, the same way it now fails at zero versions.
  3. Optionally, a PGC_REQUIRE_PYARROW=1 for gate runs, so a gate treats the missing
    dependency as an error while a developer laptop keeps the graceful skip.

I am happy to write this if nobody else has it.

Scope note on my own past gate runs

Every gate I ran on the bench before today reported the Parquet and Arrow suites green
without executing them. I do not know yet how far back that goes, and I am not claiming any
specific merged PR is affected. I am flagging it because the same is true of anyone else's
runs on a box without pyarrow, and because it changes what "the suites are green" is worth
until item 1 above exists.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions