mbutrovich commented on code in PR #23398:
URL: https://github.com/apache/datafusion/pull/23398#discussion_r3631392573
##########
datafusion/common/src/config.rs:
##########
@@ -1181,6 +1181,14 @@ config_namespace! {
/// (reading) Use any available bloom filters when reading parquet
files
pub bloom_filter_on_read: bool, default = true
+ /// (reading) If true, when a projected nested column (struct, or
+ /// struct nested in lists) is read through a cast to a narrower
+ /// nested type — as happens when the table schema declares fewer
+ /// struct fields than the parquet file contains — the reader only
+ /// fetches and decodes the leaf columns the cast retains, instead of
+ /// the entire column
+ pub nested_projection_pruning: bool, default = true
Review Comment:
This flag is responsible for a large share of the PR's surface: the six
`proto-*` files, `parquet_writer.rs`, `file_formats.rs`,
`information_schema.slt`, `configs.md`, and the threading through `source ->
morselizer -> PreparedParquetOpen -> opener -> DecoderProjection`. It's also
what triggers the `cargo-semver-checks` major-version failure CI already
reported (new pub field on the externally-constructible `ParquetOptions`).
The PR description itself notes there's "no scenario where a
correctly-working clip should be disabled" and that the flag could be
deprecated after a release or two. For a change that's semantically invisible
(IO reduction only, worst case = today's behavior), a permanent serialized knob
seems disproportionate.
Options, roughly in order of preference:
1. Drop the flag entirely. The total-fallback design already makes this safe.
2. If a bisection escape hatch is genuinely wanted, make it a
non-serialized/session-only setting so it stays off the proto wire format and
out of the semver surface.
Happy to be wrong if there's a concrete reason it must be plan-serialized,
but as-is it roughly doubles the file count and forces a major-version bump for
an optimization that's invisible to results.
##########
datafusion/datasource-parquet/src/nested_schema_pruning.rs:
##########
@@ -0,0 +1,560 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Schema-driven nested projection pruning.
+//!
+//! When a scan's projection consumes a nested column only through a cast to a
+//! *narrower* nested type — e.g. the file contains
+//! `events: List<Struct<x, y, z, ...>>` but the expression is
+//! `CAST(events AS List<Struct<x, y>>)` — the parquet reader does not need to
+//! fetch or decode the leaves the cast target never names. This module
+//! computes which parquet leaves survive such a cast by walking the physical
+//! and target type trees in parallel, matching struct fields by name (the
+//! equivalent of Spark's `ParquetReadSupport.clipParquetSchema`).
+//!
+//! This situation arises whenever a table's logical schema declares a nested
+//! column narrower than the physical parquet file: the physical expression
+//! adapter rewrites the projected column into exactly such a whole-column
+//! cast (see `datafusion_physical_expr_adapter`). Engines like Spark
+//! communicate nested projection pruning to the scan this way — as a clipped
+//! read *schema* rather than as `get_field` expressions.
+//!
+//! # Safety of clipping
+//!
+//! The runtime cast for nested types
+//! ([`datafusion_common::nested_struct::cast_column`]) consumes source struct
+//! children exclusively by looking up the *target* field names, recursively
+//! through list wrappers. Physical subtrees not named by the target are
+//! provably dead: removing them from the read cannot change the cast's
+//! output. Struct-level nullability is preserved because the parquet reader
+//! reconstructs ancestor validity from the definition levels of any surviving
+//! leaf, and the clip always keeps at least one leaf per struct level.
+//!
+//! The clip is *total*: any type shape it does not understand (maps,
+//! dictionaries, wrapper-kind mismatches, ...) keeps all of its leaves, so
+//! the worst case is today's behavior of reading the full column. Map values
+//! are deliberately not clipped: the runtime cast routes maps through Arrow's
+//! positional struct cast, which requires all children to be present.
+
+use std::collections::BTreeSet;
+use std::sync::Arc;
+
+use arrow::datatypes::{DataType, Field, FieldRef};
+
+/// Clip `physical` against `cast_target`, returning which parquet leaves the
+/// cast actually consumes, as offsets relative to the root column's first
+/// leaf (sorted ascending and non-empty). The Arrow type the reader will
+/// emit for these offsets is derived with [`prune_type_by_kept_offsets`].
+///
+/// Returns `None` when nothing can be pruned (every leaf is consumed, or the
+/// shapes do not allow safe clipping), in which case the caller should read
+/// the whole column as before. This function never fails: unknown shapes
+/// degrade to keeping all leaves.
+pub(crate) fn clip_for_cast(
+ physical: &DataType,
+ cast_target: &DataType,
+) -> Option<Vec<usize>> {
+ let total = count_leaves(physical);
+ let mut kept = Vec::new();
+ let mut next_leaf = 0;
+ clip_type(physical, cast_target, &mut next_leaf, &mut kept);
+ debug_assert_eq!(next_leaf, total, "leaf accounting must cover the type");
+ if kept.is_empty() || kept.len() >= total {
+ return None;
+ }
+ Some(kept)
+}
+
+/// Number of parquet leaf columns a (parquet-derived) Arrow type occupies.
+pub(crate) fn count_leaves(dt: &DataType) -> usize {
+ match dt {
+ DataType::Struct(fields) => {
+ fields.iter().map(|f| count_leaves(f.data_type())).sum()
+ }
+ DataType::List(f)
+ | DataType::LargeList(f)
+ | DataType::ListView(f)
+ | DataType::LargeListView(f)
+ | DataType::FixedSizeList(f, _)
+ | DataType::Map(f, _) => count_leaves(f.data_type()),
+ DataType::Dictionary(_, value) => count_leaves(value),
+ DataType::RunEndEncoded(_, value) => count_leaves(value.data_type()),
+ _ => 1,
+ }
+}
+
+/// Recursive walker: advances `next_leaf` across every leaf of `physical`,
+/// pushing the offsets the cast target consumes into `kept`.
+fn clip_type(
+ physical: &DataType,
+ target: &DataType,
+ next_leaf: &mut usize,
+ kept: &mut Vec<usize>,
+) {
+ match (physical, target) {
+ (DataType::Struct(p_children), DataType::Struct(t_children)) => {
+ let matches: Vec<Option<&FieldRef>> = p_children
+ .iter()
+ .map(|pc| t_children.iter().find(|tc| tc.name() == pc.name()))
+ .collect();
+
+ if matches.iter().all(Option::is_none) {
+ // No field name overlap at this level. The runtime cast will
+ // fail (or a custom cast may do something unusual), so keep
+ // the first non-empty child wholesale to preserve at least
+ // one leaf and the struct's definition levels; behavior is
+ // then identical to an unclipped read.
+ return keep_first_child(p_children, next_leaf, kept);
+ }
+
+ for (pc, tc) in p_children.iter().zip(matches) {
+ match tc {
+ Some(tc) => {
+ clip_type(pc.data_type(), tc.data_type(), next_leaf,
kept)
+ }
+ None => skip_leaves(pc.data_type(), next_leaf),
+ }
+ }
+ }
+ (DataType::List(p_item), DataType::List(t_item))
Review Comment:
`clip_type` only recurses through `List`/`LargeList`; `ListView`,
`LargeListView`, and `Dictionary(value)` fall to the `_` arm and keep all
leaves. That's safe, but `cast_column` *does* route those through the
name-based recursive cast (`nested_struct.rs:186-201`), and
`requires_nested_struct_cast` returns true for them
(`nested_struct.rs:477-483`) — so they're clippable in principle and are just
left on the table here.
Suggest a one-line comment noting this is a deliberate conservative choice
(safe, matches "worst case = full read") and a candidate follow-up, so a future
reader doesn't assume these shapes are unclippable.
##########
datafusion/datasource-parquet/src/projection_read_plan.rs:
##########
@@ -0,0 +1,657 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Resolution of expressions against a Parquet file's schema into a
+//! [`ParquetReadPlan`]: the leaf-level [`ProjectionMask`] to install on the
+//! decoder plus the Arrow schema the decoder will emit under that mask.
+//!
+//! This is shared by the opener's projection handling (via
+//! [`build_projection_read_plan`]) and row-filter construction (via
+//! [`crate::row_filter`]), which both need to translate column and struct
+//! field references into Parquet leaf indices.
+
+use std::collections::{BTreeMap, BTreeSet};
+use std::sync::Arc;
+
+use arrow::datatypes::{DataType, Field, Schema, SchemaRef};
+use parquet::arrow::ProjectionMask;
+use parquet::schema::types::SchemaDescriptor;
+
+use datafusion_common::tree_node::TreeNode;
+use datafusion_physical_expr::PhysicalExpr;
+use datafusion_physical_expr::expressions::Column;
+use datafusion_physical_expr::utils::collect_columns;
+
+use crate::nested_schema_pruning::{
+ CastColumnAccess, clip_for_cast, count_leaves, field_with_type,
+ prune_type_by_kept_offsets,
+};
+use crate::row_filter::PushdownChecker;
+
+/// The result of resolving which Parquet leaf columns and Arrow schema fields
+/// are needed to evaluate an expression against a Parquet file
+///
+/// This is the shared output of the column resolution pipeline used by both
+/// the row filter to build `ArrowPredicate`s and the opener to build
`ProjectionMask`s
+#[derive(Debug, Clone)]
+pub(crate) struct ParquetReadPlan {
+ /// Projection mask built from leaf column indices in the Parquet schema.
+ /// Using a `ProjectionMask` directly (rather than raw indices) prevents
+ /// bugs from accidentally mixing up root vs leaf indices.
+ pub projection_mask: ProjectionMask,
+ /// The projected Arrow schema containing only the columns/fields required
+ /// Struct types are pruned to include only the accessed sub-fields
+ pub projected_schema: SchemaRef,
+}
+
+/// Records a struct field access via `get_field(struct_col, 'field1',
'field2', ...)`.
+///
+/// This allows the row filter to project only the specific Parquet leaf
columns
+/// needed by the filter, rather than all leaves of the struct.
+#[derive(Debug, Clone)]
+pub(crate) struct StructFieldAccess {
+ /// Arrow root column index of the struct in the file schema.
+ pub(crate) root_index: usize,
+ /// Field names forming the path into the struct.
+ /// e.g., `["value"]` for `s['value']`, `["outer", "inner"]` for
`s['outer']['inner']`.
+ pub(crate) field_path: Vec<String>,
+}
+
+/// Builds a unified [`ParquetReadPlan`] for a set of projection expressions
+///
+/// Unlike [`crate::row_filter::build_parquet_read_plan`] (which is used for
+/// filter pushdown and returns `None` when an expression references
+/// unsupported nested types or missing columns), this function always
+/// succeeds. It collects every column that *can* be resolved in the file and
+/// produces a leaf-level projection mask. Columns missing from the file are
+/// silently skipped since the projection layer handles those by inserting
+/// nulls.
+pub(crate) fn build_projection_read_plan(
+ exprs: impl IntoIterator<Item = Arc<dyn PhysicalExpr>>,
+ file_schema: &Schema,
+ schema_descr: &SchemaDescriptor,
+ prune_nested_casts: bool,
+) -> ParquetReadPlan {
+ // fast path: if every expression is a plain Column reference, skip all
+ // struct analysis and use root-level projection directly
+ let exprs = exprs.into_iter().collect::<Vec<_>>();
+ let all_plain_columns = exprs.iter().all(|e|
e.downcast_ref::<Column>().is_some());
+
+ if all_plain_columns {
+ let mut root_indices: Vec<usize> = exprs
+ .iter()
+ .map(|e| e.downcast_ref::<Column>().unwrap().index())
+ .collect();
+ root_indices.sort_unstable();
+ root_indices.dedup();
+
+ let projection_mask =
+ ProjectionMask::roots(schema_descr, root_indices.iter().copied());
+ let projected_schema = Arc::new(
+ file_schema
+ .project(&root_indices)
+ .expect("valid column indices"),
+ );
+
+ return ParquetReadPlan {
+ projection_mask,
+ projected_schema,
+ };
+ }
+
+ // secondary fast path: if no column contains a struct at any nesting
+ // level, there are no leaves to prune and we can skip PushdownChecker
+ // traversal and use root-level projection
+ let has_struct_columns = file_schema
+ .fields()
+ .iter()
+ .any(|f| contains_struct(f.data_type()));
+
+ if !has_struct_columns {
+ let mut root_indices = exprs
+ .into_iter()
+ .flat_map(|e| collect_columns(&e).into_iter().map(|col|
col.index()))
+ .collect::<Vec<_>>();
+
+ root_indices.sort_unstable();
+ root_indices.dedup();
+
+ let projection_mask =
+ ProjectionMask::roots(schema_descr, root_indices.iter().copied());
+
+ let projected_schema = Arc::new(
+ file_schema
+ .project(&root_indices)
+ .expect("valid column indices"),
+ );
+
+ return ParquetReadPlan {
+ projection_mask,
+ projected_schema,
+ };
+ }
+
+ let mut all_root_indices = Vec::new();
+ let mut all_struct_accesses = Vec::new();
+ let mut all_cast_accesses = Vec::new();
+
+ for expr in exprs {
+ let mut checker = PushdownChecker::new(file_schema, true)
+ .with_cast_collection(prune_nested_casts);
+ let _ = expr.visit(&mut checker);
+ let columns = checker.into_sorted_columns();
+
+ all_root_indices.extend_from_slice(&columns.required_columns);
+ all_struct_accesses.extend(columns.struct_field_accesses);
+ all_cast_accesses.extend(columns.cast_accesses);
+ }
+
+ all_root_indices.sort_unstable();
+ all_root_indices.dedup();
+
+ // A whole-column reference reads every leaf of the root, so clipping the
+ // same root for a cast (or a struct field access) would be overridden
+ // anyway: drop those accesses up front.
+ let whole_roots: BTreeSet<usize> =
all_root_indices.iter().copied().collect();
+ all_cast_accesses.retain(|c| !whole_roots.contains(&c.root_index));
+
+ if !all_cast_accesses.is_empty() {
+ return build_read_plan_with_cast_clipping(
+ file_schema,
+ schema_descr,
+ &all_root_indices,
+ &all_struct_accesses,
+ &all_cast_accesses,
+ );
+ }
+
+ // when no struct field accesses were found, fall back to root-level
projection
+ // to match the performance of the simple path
+ if all_struct_accesses.is_empty() {
+ let projection_mask =
+ ProjectionMask::roots(schema_descr,
all_root_indices.iter().copied());
+ let projected_schema = Arc::new(
+ file_schema
+ .project(&all_root_indices)
+ .expect("valid column indices"),
+ );
+
+ return ParquetReadPlan {
+ projection_mask,
+ projected_schema,
+ };
+ }
+
+ let leaf_indices = {
+ let mut out =
+ leaf_indices_for_roots(all_root_indices.iter().copied(),
schema_descr);
+ let struct_leaf_indices =
+ resolve_struct_field_leaves(&all_struct_accesses, file_schema,
schema_descr);
+
+ out.extend_from_slice(&struct_leaf_indices);
+ out.sort_unstable();
+ out.dedup();
+
+ out
+ };
+
+ let projection_mask =
+ ProjectionMask::leaves(schema_descr, leaf_indices.iter().copied());
+
+ let projected_schema =
+ build_filter_schema(file_schema, &all_root_indices,
&all_struct_accesses);
+
+ ParquetReadPlan {
+ projection_mask,
+ projected_schema,
+ }
+}
+
+/// Does this type contain a struct at any nesting depth?
+fn contains_struct(dt: &DataType) -> bool {
+ match dt {
+ DataType::Struct(_) => true,
+ DataType::List(f)
+ | DataType::LargeList(f)
+ | DataType::ListView(f)
+ | DataType::LargeListView(f)
+ | DataType::FixedSizeList(f, _)
+ | DataType::Map(f, _) => contains_struct(f.data_type()),
+ DataType::Dictionary(_, value) => contains_struct(value),
+ DataType::RunEndEncoded(_, value) =>
contains_struct(value.data_type()),
+ _ => false,
+ }
+}
+
+/// Builds a [`ParquetReadPlan`] when at least one projected root column is
+/// consumed through a cast to a narrower nested type.
+///
+/// Per root, in ascending root-index order:
+/// - roots referenced as whole columns keep every leaf and their full
+/// physical field (whole-column reads take precedence; cast accesses on
+/// such roots were already dropped by the caller);
+/// - roots consumed only through casts and/or `get_field` accesses keep the
+/// union of the leaves those accesses consume, and their projected field
+/// type is derived from that union.
+///
+/// Falls back to keeping all leaves of a root whenever its clipping is not
+/// provably safe (see [`crate::nested_schema_pruning`]).
+fn build_read_plan_with_cast_clipping(
Review Comment:
`build_read_plan_with_cast_clipping` + `prune_type_by_kept_offsets` union
the leaves of a narrowing cast and `get_field` accesses on the *same* root. For
the case this PR targets (Spark/Comet bake pruning into `requiredSchema` as a
whole-column narrower type), the scan sees a pure `CAST(col AS narrow)` — no
sibling `get_field` on that root. The `get_field(CAST(col))` case (the
`SUM(s['x'])` example in the description) is *nested*, and the visitor already
handles it by clipping to the inner cast target, so it doesn't exercise the
union path either.
Is there a concrete plan shape that produces a cast and a separate
`get_field` on the same root at the scan boundary? If not,
`prune_type_by_kept_offsets` and the union bookkeeping are speculative and
could be deferred to a follow-up (with the simple per-root path covering the
shipped feature), which would also shrink the review surface. If there is, a
test that constructs that shape would document why the machinery exists.
##########
datafusion/datasource-parquet/src/nested_schema_pruning.rs:
##########
@@ -0,0 +1,560 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Schema-driven nested projection pruning.
+//!
+//! When a scan's projection consumes a nested column only through a cast to a
+//! *narrower* nested type — e.g. the file contains
+//! `events: List<Struct<x, y, z, ...>>` but the expression is
+//! `CAST(events AS List<Struct<x, y>>)` — the parquet reader does not need to
+//! fetch or decode the leaves the cast target never names. This module
+//! computes which parquet leaves survive such a cast by walking the physical
+//! and target type trees in parallel, matching struct fields by name (the
+//! equivalent of Spark's `ParquetReadSupport.clipParquetSchema`).
+//!
+//! This situation arises whenever a table's logical schema declares a nested
+//! column narrower than the physical parquet file: the physical expression
+//! adapter rewrites the projected column into exactly such a whole-column
+//! cast (see `datafusion_physical_expr_adapter`). Engines like Spark
+//! communicate nested projection pruning to the scan this way — as a clipped
+//! read *schema* rather than as `get_field` expressions.
+//!
+//! # Safety of clipping
+//!
+//! The runtime cast for nested types
+//! ([`datafusion_common::nested_struct::cast_column`]) consumes source struct
+//! children exclusively by looking up the *target* field names, recursively
+//! through list wrappers. Physical subtrees not named by the target are
+//! provably dead: removing them from the read cannot change the cast's
+//! output. Struct-level nullability is preserved because the parquet reader
+//! reconstructs ancestor validity from the definition levels of any surviving
+//! leaf, and the clip always keeps at least one leaf per struct level.
+//!
+//! The clip is *total*: any type shape it does not understand (maps,
+//! dictionaries, wrapper-kind mismatches, ...) keeps all of its leaves, so
+//! the worst case is today's behavior of reading the full column. Map values
+//! are deliberately not clipped: the runtime cast routes maps through Arrow's
+//! positional struct cast, which requires all children to be present.
+
+use std::collections::BTreeSet;
+use std::sync::Arc;
+
+use arrow::datatypes::{DataType, Field, FieldRef};
+
+/// Clip `physical` against `cast_target`, returning which parquet leaves the
+/// cast actually consumes, as offsets relative to the root column's first
+/// leaf (sorted ascending and non-empty). The Arrow type the reader will
+/// emit for these offsets is derived with [`prune_type_by_kept_offsets`].
+///
+/// Returns `None` when nothing can be pruned (every leaf is consumed, or the
+/// shapes do not allow safe clipping), in which case the caller should read
+/// the whole column as before. This function never fails: unknown shapes
+/// degrade to keeping all leaves.
+pub(crate) fn clip_for_cast(
+ physical: &DataType,
+ cast_target: &DataType,
+) -> Option<Vec<usize>> {
+ let total = count_leaves(physical);
+ let mut kept = Vec::new();
+ let mut next_leaf = 0;
+ clip_type(physical, cast_target, &mut next_leaf, &mut kept);
+ debug_assert_eq!(next_leaf, total, "leaf accounting must cover the type");
+ if kept.is_empty() || kept.len() >= total {
+ return None;
+ }
+ Some(kept)
+}
+
+/// Number of parquet leaf columns a (parquet-derived) Arrow type occupies.
+pub(crate) fn count_leaves(dt: &DataType) -> usize {
+ match dt {
+ DataType::Struct(fields) => {
+ fields.iter().map(|f| count_leaves(f.data_type())).sum()
+ }
+ DataType::List(f)
+ | DataType::LargeList(f)
+ | DataType::ListView(f)
+ | DataType::LargeListView(f)
+ | DataType::FixedSizeList(f, _)
+ | DataType::Map(f, _) => count_leaves(f.data_type()),
+ DataType::Dictionary(_, value) => count_leaves(value),
+ DataType::RunEndEncoded(_, value) => count_leaves(value.data_type()),
+ _ => 1,
+ }
+}
+
+/// Recursive walker: advances `next_leaf` across every leaf of `physical`,
+/// pushing the offsets the cast target consumes into `kept`.
+fn clip_type(
+ physical: &DataType,
+ target: &DataType,
+ next_leaf: &mut usize,
+ kept: &mut Vec<usize>,
+) {
+ match (physical, target) {
+ (DataType::Struct(p_children), DataType::Struct(t_children)) => {
+ let matches: Vec<Option<&FieldRef>> = p_children
+ .iter()
+ .map(|pc| t_children.iter().find(|tc| tc.name() == pc.name()))
+ .collect();
+
+ if matches.iter().all(Option::is_none) {
+ // No field name overlap at this level. The runtime cast will
+ // fail (or a custom cast may do something unusual), so keep
+ // the first non-empty child wholesale to preserve at least
+ // one leaf and the struct's definition levels; behavior is
+ // then identical to an unclipped read.
+ return keep_first_child(p_children, next_leaf, kept);
+ }
+
+ for (pc, tc) in p_children.iter().zip(matches) {
+ match tc {
+ Some(tc) => {
+ clip_type(pc.data_type(), tc.data_type(), next_leaf,
kept)
+ }
+ None => skip_leaves(pc.data_type(), next_leaf),
+ }
+ }
+ }
+ (DataType::List(p_item), DataType::List(t_item))
+ | (DataType::LargeList(p_item), DataType::LargeList(t_item)) => {
+ clip_type(p_item.data_type(), t_item.data_type(), next_leaf, kept)
+ }
+ // Anything else — leaf pairs, wrapper-kind mismatches, maps,
+ // dictionaries, fixed-size lists, views — is kept wholesale.
+ _ => keep_all_leaves(physical, next_leaf, kept),
+ }
+}
+
+/// Fallback for a struct level with zero name overlap: keep the first child
+/// that owns at least one leaf, skip the rest.
+fn keep_first_child(
Review Comment:
The zero-name-overlap branch in `clip_type` (`nested_schema_pruning.rs:116`)
exists to preserve one leaf when a struct level has no matching target field.
But a cast with no field-name overlap is rejected at planning time:
`validate_struct_compatibility` (`nested_struct.rs:345`) returns a plan error
("no field name overlap") before the reader is ever built. So for the
`DefaultPhysicalExprAdapter` path this branch looks unreachable.
If it's kept for custom adapters that bypass that validation, worth a
comment saying so; otherwise it and `keep_first_child` could be dropped in
favor of the `_` keep-all arm. Either way a targeted unit test (or a note on
why it can't be tested) would clarify intent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
