Skip to content

Commit 9746f3f

Browse files
committed
perf: clip the row-filter read plan to the retained cast target
`check_cast_struct_field_access` pushed the struct root into `required_columns`, so a retained Struct cast decoded every leaf of that root — even though `retain_field_path` had already pruned the cast target down to the selected path, leaving a cast that consumes a single leaf. Record a `CastColumnAccess` instead and route `build_parquet_read_plan` through `build_read_plan_with_cast_clipping`, which the projection path already uses for this expression shape. Clipping follows the *cast target*, not the `get_field` key, so an explicit query cast — whose target names every field the user asked to convert — keeps its siblings in the read and still evaluates their conversions. A target that covers every leaf, or that cannot be clipped safely, falls back to a full read of that root inside the helper. `build_read_plan_with_cast_clipping` now returns its leaf indices, sorted and deduplicated, so `size_of_columns` cannot double-count a repeat. Also adds SQL-level coverage for the all-null Struct decimal behavior, and records the one shape the container-unwrapping carve-out leaves open.
1 parent e79e912 commit 9746f3f

4 files changed

Lines changed: 180 additions & 31 deletions

File tree

datafusion/datasource-parquet/src/projection_read_plan.rs

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -178,15 +178,16 @@ pub(crate) struct PushdownChecker<'schema> {
178178
/// Struct field accesses via `get_field`.
179179
struct_field_accesses: Vec<StructFieldAccess>,
180180
/// Whole-column casts to a narrower nested type
181-
/// (`CAST(col AS narrower_struct)`). Only collected when
182-
/// [`Self::with_cast_collection`] enables it (projection analysis);
183-
/// filter pushdown leaves this off.
181+
/// (`CAST(col AS narrower_struct)`), collected either when
182+
/// [`Self::with_cast_collection`] enables it (projection analysis) or when
183+
/// [`Self::allow_struct_casts`] accepts a retained cast under a `get_field`
184+
/// (filter pushdown).
184185
cast_accesses: Vec<CastColumnAccess>,
185186
/// Whether to collect [`Self::cast_accesses`].
186187
collect_cast_accesses: bool,
187188
/// Allow `get_field(CAST(struct_column AS Struct(...)), 'field', ...)`
188-
/// after schema adaptation, preserving the cast and reading the full source
189-
/// Struct. Both source and target must be Struct types.
189+
/// after schema adaptation, preserving the cast and reading the leaves its
190+
/// target names. Both source and target must be Struct types.
190191
/// Planning keeps this disabled so explicit casts retain a residual filter.
191192
allow_struct_casts: bool,
192193
/// Whether nested list columns are supported by the predicate semantics.
@@ -258,8 +259,17 @@ impl<'schema> PushdownChecker<'schema> {
258259
None
259260
}
260261

261-
/// Preserve a Struct cast retained by schema adaptation and read its full
262-
/// root. Pruning siblings or moving the cast could change errors or nulls.
262+
/// Preserve a Struct cast retained by schema adaptation and record the
263+
/// leaves its target consumes.
264+
///
265+
/// The cast is kept intact — moving it could change errors or nulls — but
266+
/// the target itself names every field the conversion touches, so the read
267+
/// can be clipped to those leaves. `cast_struct_column` resolves source
268+
/// children by name and ignores the rest, so a leaf the target does not
269+
/// name cannot affect the result. Note this clips by the *cast target*, not
270+
/// by the `get_field` key: an explicit query cast names every field the
271+
/// user asked to convert, so its siblings stay in the read and their
272+
/// conversions still run.
263273
fn check_cast_struct_field_access(
264274
&mut self,
265275
func: &ScalarFunctionExpr,
@@ -304,7 +314,10 @@ impl<'schema> PushdownChecker<'schema> {
304314
.data_type();
305315
}
306316

307-
self.required_columns.push(index);
317+
self.cast_accesses.push(CastColumnAccess {
318+
root_index: index,
319+
target_type: cast.cast_type().clone(),
320+
});
308321
Some(TreeNodeRecursion::Jump)
309322
}
310323

@@ -607,13 +620,14 @@ pub(crate) fn build_projection_read_plan(
607620
all_cast_accesses.retain(|c| all_root_indices.binary_search(&c.root_index).is_err());
608621

609622
if !all_cast_accesses.is_empty() {
610-
return build_read_plan_with_cast_clipping(
623+
let (read_plan, _leaf_indices) = build_read_plan_with_cast_clipping(
611624
file_schema,
612625
schema_descr,
613626
&all_root_indices,
614627
&all_struct_accesses,
615628
&all_cast_accesses,
616629
);
630+
return read_plan;
617631
}
618632

619633
// when no struct field accesses were found, fall back to root-level projection
@@ -657,13 +671,16 @@ enum RootRead {
657671
/// `nested_schema_pruning::clip_for_cast`), an access that resolves to no
658672
/// leaf at all, or a merged leaf set whose emitted Arrow type can't be
659673
/// derived safely, falls back to a full read of that root.
660-
fn build_read_plan_with_cast_clipping(
674+
///
675+
/// Also returns the resolved Parquet leaf indices, sorted and deduplicated, so
676+
/// callers can size the columns the decoder will read.
677+
pub(crate) fn build_read_plan_with_cast_clipping(
661678
file_schema: &Schema,
662679
schema_descr: &SchemaDescriptor,
663680
whole_root_indices: &[usize],
664681
struct_accesses: &[StructFieldAccess],
665682
cast_accesses: &[CastColumnAccess],
666-
) -> ParquetReadPlan {
683+
) -> (ParquetReadPlan, Vec<usize>) {
667684
// Every referenced root's Parquet leaves, grouped in one pass over the
668685
// schema descriptor rather than one `leaf_indices_for_roots` scan per
669686
// root (this function may look up several roots).
@@ -767,17 +784,24 @@ fn build_read_plan_with_cast_clipping(
767784
fields.push(Arc::new(field.clone()));
768785
}
769786
// `ProjectionMask::leaves` only flips flags in a `vec![false; num_columns]`,
770-
// so `leaf_indices` needs no sorting or deduplication here.
771-
ParquetReadPlan {
772-
projection_mask: ProjectionMask::leaves(
773-
schema_descr,
774-
leaf_indices.iter().copied(),
775-
),
776-
projected_schema: Arc::new(Schema::new_with_metadata(
777-
fields,
778-
file_schema.metadata().clone(),
779-
)),
780-
}
787+
// so the mask itself needs neither ordering nor deduplication. Callers that
788+
// size the read do care, so normalize before handing the indices back:
789+
// `size_of_columns` sums per index and would double-count a repeat.
790+
leaf_indices.sort_unstable();
791+
leaf_indices.dedup();
792+
(
793+
ParquetReadPlan {
794+
projection_mask: ProjectionMask::leaves(
795+
schema_descr,
796+
leaf_indices.iter().copied(),
797+
),
798+
projected_schema: Arc::new(Schema::new_with_metadata(
799+
fields,
800+
file_schema.metadata().clone(),
801+
)),
802+
},
803+
leaf_indices,
804+
)
781805
}
782806

783807
/// Groups every Parquet leaf index by its root (Arrow) column index, in one
@@ -1757,7 +1781,7 @@ mod test {
17571781
vec![Arc::new(Field::new("p", DataType::Int32, true))].into(),
17581782
),
17591783
};
1760-
let read_plan = build_read_plan_with_cast_clipping(
1784+
let (read_plan, _leaf_indices) = build_read_plan_with_cast_clipping(
17611785
&file_schema,
17621786
schema_descr,
17631787
&[],
@@ -1927,7 +1951,7 @@ mod test {
19271951
Schema::new(vec![divergent("a", "p", "q"), divergent("b", "m", "n")]);
19281952

19291953
// `a` is reached by a narrowing cast, `b` only by `get_field`.
1930-
let read_plan = build_read_plan_with_cast_clipping(
1954+
let (read_plan, _leaf_indices) = build_read_plan_with_cast_clipping(
19311955
&file_schema,
19321956
schema_descr,
19331957
&[],

datafusion/datasource-parquet/src/row_filter.rs

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ use super::ParquetFileMetrics;
8787
use super::supported_predicates::supports_list_predicates;
8888
use crate::projection_read_plan::{
8989
ParquetReadPlan, PushdownChecker, PushdownColumns, assemble_read_plan,
90+
build_read_plan_with_cast_clipping,
9091
};
9192

9293
/// A "compiled" predicate passed to `ParquetRecordBatchStream` to perform
@@ -281,12 +282,26 @@ pub(crate) fn build_parquet_read_plan(
281282
return Ok(None);
282283
};
283284

284-
let (read_plan, leaf_indices) = assemble_read_plan(
285-
&required_columns.required_columns,
286-
&required_columns.struct_field_accesses,
287-
file_schema,
288-
schema_descr,
289-
);
285+
// A retained Struct cast names the fields its conversion touches, so the
286+
// read is clipped to those leaves rather than decoding the whole root.
287+
// A cast whose target covers every leaf, or that cannot be clipped safely,
288+
// falls back to a full read of that root inside the helper.
289+
let (read_plan, leaf_indices) = if required_columns.cast_accesses.is_empty() {
290+
assemble_read_plan(
291+
&required_columns.required_columns,
292+
&required_columns.struct_field_accesses,
293+
file_schema,
294+
schema_descr,
295+
)
296+
} else {
297+
build_read_plan_with_cast_clipping(
298+
file_schema,
299+
schema_descr,
300+
&required_columns.required_columns,
301+
&required_columns.struct_field_accesses,
302+
&required_columns.cast_accesses,
303+
)
304+
};
290305

291306
let required_bytes = size_of_columns(&leaf_indices, metadata)?;
292307

@@ -1362,6 +1377,54 @@ mod test {
13621377
.unwrap();
13631378
let error = row_filter.evaluate(batch).unwrap_err().to_string();
13641379
datafusion_common::assert_contains!(error, "While casting struct field 'label'");
1380+
1381+
// A retained cast whose target names only the selected field — the
1382+
// shape `retain_field_path` produces for an evolved decimal — clips the
1383+
// read to that field's leaf instead of decoding the whole root.
1384+
let narrow_cast_type =
1385+
DataType::Struct(vec![Field::new("value", DataType::Int32, false)].into());
1386+
let narrow_field = get_field().call(vec![
1387+
datafusion_expr::cast(col("s"), narrow_cast_type.clone()),
1388+
lit("value"),
1389+
]);
1390+
let narrow_predicate = logical2physical(&narrow_field.gt(lit(5)), &file_schema);
1391+
let candidate =
1392+
FilterCandidateBuilder::new(narrow_predicate, Arc::clone(&file_schema))
1393+
.build(&metadata)
1394+
.expect("building narrow cast candidate")
1395+
.expect("a clipped struct cast must remain evaluable");
1396+
assert_eq!(
1397+
candidate.read_plan.projection_mask,
1398+
ProjectionMask::leaves(metadata.file_metadata().schema_descr(), [1]),
1399+
"the read must be clipped to the leaf the cast target names"
1400+
);
1401+
assert_eq!(candidate.read_plan.projected_schema.fields().len(), 1);
1402+
assert_eq!(
1403+
candidate.read_plan.projected_schema.field(0).data_type(),
1404+
&narrow_cast_type,
1405+
"sibling leaves must be pruned from the filter schema"
1406+
);
1407+
1408+
// The clipped schema must still evaluate: every row has value > 5.
1409+
let mut row_filter = DatafusionArrowPredicate::try_new(
1410+
candidate,
1411+
Count::new(),
1412+
Count::new(),
1413+
Time::new(),
1414+
)
1415+
.unwrap();
1416+
let batch = ParquetRecordBatchReaderBuilder::try_new(file.reopen().unwrap())
1417+
.unwrap()
1418+
.with_projection(row_filter.projection().clone())
1419+
.build()
1420+
.unwrap()
1421+
.next()
1422+
.unwrap()
1423+
.unwrap();
1424+
assert_eq!(
1425+
row_filter.evaluate(batch).unwrap(),
1426+
BooleanArray::from(vec![true, true, true])
1427+
);
13651428
}
13661429

13671430
/// Deeply nested get_field: get_field(struct_col, 'outer', 'inner') where the

datafusion/physical-expr-adapter/src/schema_rewriter.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,14 @@ impl DefaultPhysicalExprAdapterRewriter {
530530
// narrowable by a parent get_field.
531531
// Container casts involving Struct values must keep their existing
532532
// dispatch: Arrow can unwrap a container into a Struct where cast_column
533-
// cannot.
533+
// cannot. That leaves one shape uncovered: unwrapping hands the Struct
534+
// to Arrow's own cast, which has no all-null shortcut, so a decimal
535+
// below it can still fail on an all-null input (for example
536+
// `Dictionary(Int8, Struct<b: Utf8>)` to `Struct<b: Decimal128(10, -1)>`).
537+
// The Parquet reader does not produce dictionary-encoded Struct columns,
538+
// so this is not reachable through a Parquet scan; closing it would mean
539+
// telling "Arrow must unwrap this" apart from "Arrow will convert a
540+
// decimal while unwrapping" rather than dropping the carve-out.
534541
let source_type = physical_struct_field.data_type();
535542
let target_type = logical_struct_field.data_type();
536543
let is_struct = |data_type: &DataType| matches!(data_type, DataType::Struct(_));

datafusion/sqllogictest/test_files/schema_evolution_nested.slt

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,3 +122,58 @@ FROM large_list_messages;
122122
----
123123
1 10 NULL
124124
2 30 eth
125+
126+
##########
127+
# An all-null Struct must stay null when a child evolves to a decimal whose
128+
# conversion is rejected while it is being set up, before any value is read.
129+
# A whole-Struct conversion never reaches the child; extracting the field first
130+
# would. This holds whether the decimal is the selected field itself or sits
131+
# inside a container below it.
132+
##########
133+
134+
statement ok
135+
COPY (
136+
SELECT 1 AS row_id,
137+
CASE WHEN false THEN named_struct('a', arrow_cast('x', 'Utf8')) END AS s
138+
) TO 'test_files/scratch/schema_evolution_nested/null_struct_decimal/data.parquet'
139+
STORED AS PARQUET;
140+
141+
statement ok
142+
CREATE EXTERNAL TABLE null_struct_decimal (
143+
row_id INT,
144+
s STRUCT<a DECIMAL(10, -1)>
145+
)
146+
STORED AS PARQUET
147+
LOCATION 'test_files/scratch/schema_evolution_nested/null_struct_decimal/';
148+
149+
query IR
150+
SELECT row_id, get_field(s, 'a') FROM null_struct_decimal;
151+
----
152+
1 NULL
153+
154+
statement ok
155+
COPY (
156+
SELECT 1 AS row_id,
157+
CASE WHEN false
158+
THEN named_struct('a', arrow_cast(['x'], 'List(Utf8)'))
159+
END AS s
160+
) TO 'test_files/scratch/schema_evolution_nested/null_struct_list_decimal/data.parquet'
161+
STORED AS PARQUET;
162+
163+
statement ok
164+
CREATE EXTERNAL TABLE null_struct_list_decimal (
165+
row_id INT,
166+
s STRUCT<a ARRAY<DECIMAL(10, -1)>>
167+
)
168+
STORED AS PARQUET
169+
LOCATION 'test_files/scratch/schema_evolution_nested/null_struct_list_decimal/';
170+
171+
query I?
172+
SELECT row_id, s FROM null_struct_list_decimal;
173+
----
174+
1 NULL
175+
176+
query I?
177+
SELECT row_id, get_field(s, 'a') FROM null_struct_list_decimal;
178+
----
179+
1 NULL

0 commit comments

Comments
 (0)