fix(parquet): correct nested clipping when one column has two cast targets, plus coverage

[adriangb]

Which issue does this PR close?

• N/A — this targets the comet-4859-nested-projection-pruning branch behind feat: prune unread Parquet leaves when a nested column is cast to a narrower type apache/datafusion#24090 rather than main.

Rationale for this change

Review follow-up for apache#24090: one correctness bug, two smaller fixes, and the coverage to keep them fixed. Meant to be folded into your branch — squash it, cherry-pick individual commits, or take just the tests.

Take commit 1 if you take nothing else. The rest is optional and each commit stands alone:

#	commit	what it is	drop it?
1	fix(parquet) two cast targets	wrong results / spurious failure, reachable from plain SQL	no
2	fix(parquet) zero-overlap struct level	not reachable today; makes the module enforce its own precondition. Also a one-line panic guard	fine to drop
3	perf(parquet) O(projected columns)	three non-linear steps in the planning path	fine to defer
4	test(parquet) coverage	test-only, no production changes	fine to drop

1. Two cast targets on one column read too few leaves

build_read_plan_with_cast_clipping skipped every cast access on a root it had already clipped:

if whole_roots.contains(&root)
    || fallback_roots.contains(&root)
    || clipped_by_root.contains_key(&root)   // <-- second cast on this root: dropped
{ continue; }

When a projection consumes one column through two different narrowing casts, only the first target's leaves end up in the mask. The second cast then evaluates against a struct missing the children it names:

• overlapping targets → cast_column null-fills them → silently wrong results
• disjoint targets → validate_struct_compatibility rejects the cast → the query fails where it succeeds without pruning

This is reachable from plain SQL, not only from a custom adapter: ProjectionExec is merged into the scan via ParquetSource::try_pushdown_projection, so a query-level CAST(col AS STRUCT<...>) lands in the scan's projection and reaches the same analysis as an adapter-inserted one.

Against a table whose schema is inferred from the file (so no adapter cast is interposed):

-- file: s Struct<x Int64, y Utf8, pad Utf8>
SELECT CAST(s AS STRUCT<x BIGINT>), CAST(s AS STRUCT<x BIGINT, y VARCHAR>) FROM t;
-- before: {x: 100} {x: 100, y: NULL}      <- y should be 's1'
-- after:  {x: 100} {x: 100, y: s1}

SELECT CAST(s AS STRUCT<x BIGINT>), CAST(s AS STRUCT<pad VARCHAR>) FROM t;
-- before: Error: Cannot cast struct with 1 fields to 1 fields because there is
--         no field name overlap
-- after:  {x: 100} {pad: sp1}

Both are regressions: with with_cast_collection disabled they return correct data.

Fix: a second cast with a different target demotes the root to a full read. Identical repeated targets — the shape the expression adapter actually produces when one column is referenced several times — still clip.

2. A zero-overlap nested struct level predicted a type the reader never emits

clip_type could return Struct([]) for a name-matched child whose own children shared no name with their target, while still keeping that child in the emitted type. arrow-rs drops a struct child whose leaves are all masked out (parquet/src/arrow/schema/complex.rs, if children.is_empty() { return Ok(None) }), so projected_schema promised a field the decoder does not produce.

I could not reach this through the default stack — validate_struct_compatibility rejects such a cast at planning time, and the logical planner rejects the user-written equivalent — so this is not a live bug today. But the module doc's safety argument depends on a caller invariant, and the Comet use case is precisely a custom PhysicalExprAdapter. clip_for_cast now detects the empty level and declines to clip, so the module is self-sufficient; a test pins the arrow-rs behaviour the argument rests on.

Same commit: leaves_by_root[root] is a panicking BTreeMap index on a path where the same function already guards the "root with no parquet leaves" case forty lines earlier. Latent — I could not construct a file that reaches it — but a one-line fix.

3. Performance, for the "is everything O(projected_columns)" question

Three things were not:

• clip_type matched struct fields with a linear find per physical child — Θ(N·M) String comparisons per struct level, ~375k for a 1000→500 subfield struct. Now a name map above a small width, which is what Spark's ParquetReadSupport.clipParquetGroupFields does.
• The fast-path gate scanned every field of the file schema. Widening it from matches!(.., Struct(_)) to contains_struct is the right fix for the List<Struct> hole, but it also made Map, Dictionary<_, Struct> and every array-of-records column send the projection down the PushdownChecker path, whose Schema::index_of is a linear name scan per column node — O(|exprs| · |schema|), paid once per file. The gate now looks only at the projected roots, which is both O(projected) and keeps the List<Struct> fix. Columns whose index does not line up with the file schema fall through to the name-resolving path, so stale Column indices are still handled.
• An O(G²) field_with_name re-lookup for get_field roots, replaced by a positional zip (build_filter_schema already emits fields in ascending root order). This also removes a latent wrong-field pick under duplicate root names.

Measured with a criterion benchmark over 1000-column schemas and 1000-subfield structs across 32 files of 8 rows each, so per-file planning dominates. Before → after, back to back on the same box:

case	change
flat_wide/column_projection (control, untouched path)	−0.1% (p = 0.93)
wide_struct/full_schema (control, untouched path)	+1.1% (p = 0.63)
wide_struct/unrelated_column — wide struct in the schema, not projected	−13.6% (p = 0.01)
wide_struct/narrowed_schema — 1000 → 500 subfield clip	−7.2% (p = 0.01)

The feature itself also holds up at that width: the narrowed schema runs 90.8 ms against 166.2 ms for the full one, ~45% less wall time.

The benchmark is deliberately not in this PR — it is a one-off measurement harness, not something worth carrying in-tree. It is on claude/pr-24090-review-full-harness if you want to reproduce the numbers; happy to submit it separately if you'd rather have it.

What changes are included in this PR?

Four commits; each builds and tests green on its own.

1. fix(parquet): read the whole root when one column has two cast targets
2. fix(parquet): decline to clip a struct level with no field-name overlap
3. perf(parquet): keep nested read-plan analysis O(projected columns)
4. test(parquet): cover the gaps found by mutation testing and SLT — test-only

Are these changes tested?

Yes, two ways.

SLT. parquet_nested_schema_pruning.slt goes from 110 to ~380 lines, moving most of what was Rust-only into SQL: SELECT *, mixed whole-column and field access, aggregation and filtering over a clipped column, a declared field order differing from the file's, struct-in-struct narrowing, the two-level ARRAY<STRUCT<..., ARRAY<STRUCT<..>>>> shape from the Comet issue, a MAP sibling that is never clipped, a scan mixing a physically narrow and a wide file, the zero-overlap declaration that must be rejected, and the query-level cast cases from item 1 (which fail loudly with the fix reverted).

One small correction while I was in there: the comment on full_schema says no cast is inserted. One is — VARCHAR maps to Utf8View in the SLT context while the file holds Utf8 — it just is not a narrowing cast, so every leaf is still read and the baseline is valid. Comment updated to say that.

Mutation testing, used to find the gaps rather than to gate CI. cargo-mutants over nested_schema_pruning.rs and projection_read_plan.rs: 121 mutants, 82 viable, 16 survivors. Eight were real gaps and commit 4 adds a test for each, verified by re-applying the mutation by hand:

• start + o → start - o when rebasing clip offsets onto absolute leaf indices — every existing clip test cast to the struct's first field, where the only offset is 0 and the two are identical.
• The entire get_field_accesses branch of build_read_plan_with_cast_clipping was dead in tests: nothing combined a cast on one root with a field access on another.
• Dropping !whole_roots in the get_field filter needs a root referenced both as a whole column and via get_field while another root is clipped.
• contains_struct had no direct test; count_leaves's dictionary assertion was vacuous (Dictionary(Int32, Utf8) counts 1 with or without the arm) and RunEndEncoded was untested. Both now use struct values.
• Duplicate physical field names under one target field.

Three survivors around LINEAR_FIELD_SCAN_MAX are equivalent by construction — the map and linear matching paths agree, which is what their survival demonstrates. One redundant clipped_by_root test in the get_field filter is replaced by a debug_assert for the invariant that makes it redundant.

I also ran two randomised differential harnesses while developing this — one checking clip_for_cast's predicted type and kept leaves against the real arrow-rs reader, one comparing a clipped scan against cast_column over an unclipped one end to end — for ~63k and ~25k cases respectively. They found nothing beyond the bugs above. They are not in this PR either: the specific shapes they turned up are now covered by the deterministic tests, and a permanent seeded fuzzer is a maintenance cost the fixes do not need. Also on claude/pr-24090-review-full-harness if you disagree and want them in-tree.

Verified green: cargo fmt --check, cargo clippy -p datafusion-datasource-parquet -p datafusion --all-targets --all-features -- -D warnings, 194 lib tests, 230 parquet_integration tests, and the Parquet SLT files.

Are there any user-facing changes?

No public API changes. Behaviour changes are bug fixes: a projection consuming one column through two different nested casts now returns correct results, and a column that cannot be clipped safely falls back to reading in full.

---

Generated by Claude Code

[adriangb]

Moved these to SLT tests (faster builds, easier to match to user reachable behavior).

[adriangb]

@mbutrovich could you run CI please?

[mbutrovich]

@mbutrovich could you run CI please?

On it, thanks!