You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "friendlymatthew (via GitHub)" <gi...@apache.org> on 2026/03/09 15:23:21 UTC
[PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
friendlymatthew opened a new pull request, #20822:
URL: https://github.com/apache/datafusion/pull/20822
## Which issue does this PR close?
- Related to #20603
## Rationale for this change
This PR enables Parquet row-level filter pushdown for struct field access expressions, which previously fell back to a full scan followed by a separate filtering pass, a significant perf penalty for queries filtering on struct fields in large Parquet files (like Variant types!)
Filters on struct fields like `WHERE s['foo'] > 67` were not being pushed into the Parquet decoder. This is because `PushdownChecker` sees the underlying `Column("s")` has a `Struct` type and unconditionally rejects it, without considering that `get_field` resolves to a primitive leaf. With this change, deeply nested access like `s['outer']['inner']` will also get pushed down because the logical simplifier flattens it before it reaches the physical plan
Note: this does not address the projection side and should not be blocked by it. `SELECT s['foo']` still reads the entire struct rather than just the needed leaf column. That requires separate changes to how the opener builds its projection mask.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #20822:
URL: https://github.com/apache/datafusion/pull/20822
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#issuecomment-4025448820
> > This looks great to me!
> > How do we generate the right ProjectionMask / translate into the right leaf column index in Parquet? I don't see that added anywhere but maybe the existing code already did that correctly?
>
> We do not. I added a note in the PR message:
>
> > Note: this does not address the projection side and should not be blocked by it. SELECT s['foo'] still reads the entire struct rather than just the needed leaf column. That requires separate changes to how the opener builds its projection mask.
>
> I have some ideas about this, and will push up a follow up PR
Wording It's a bit confusing here, because projection can mean one of two things: the select part of the query, or the projection of the columns that the filters need to be evaluated. I assume here you are referring to the latter, i.e. although we support these filters now, we still read the entire struct column and then apply the get field operation in memory?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#discussion_r2906480912
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -368,6 +399,31 @@ impl TreeNodeVisitor<'_> for PushdownChecker<'_> {
type Node = Arc<dyn PhysicalExpr>;
fn f_down(&mut self, node: &Self::Node) -> Result<TreeNodeRecursion> {
+ // check for get_field(Column("foo"), "bar", ...) accessing a struct field
+ // the resolved return type tells us the leaf type
+ // if it's non0nested, we can pushdown by recording the root column index and skip children
+ // this way, the visitor never sees the raw struct Column and rejects it
+ if let Some(func) =
+ ScalarFunctionExpr::try_downcast_func::<GetFieldFunc>(node.as_ref())
+ {
+ let args = func.args();
+
+ if let Some(column) = args
+ .first()
+ .and_then(|a| a.as_any().downcast_ref::<Column>())
+ {
+ let return_type = func.return_type();
+ if !DataType::is_nested(return_type) {
+ if let Some(recursion) = self.check_struct_field_column(column.name())
Review Comment:
Could you add some comments explaining how this traversal works? e.g. for `get_field(col, 'a', 'b')` how do we recurse, how does that relate to the `self.file_schema.index_of(column_name)` call in `check_struct_field_column` (maybe `check_struct_field_column` can include example inputs / outputs in it's docstring), etc.
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -60,9 +60,11 @@
//! still be sorted by size.
//!
//! List-aware predicates (for example, `array_has`, `array_has_all`, and
-//! `array_has_any`) can be evaluated directly during Parquet decoding. Struct
-//! columns and other nested projections that are not explicitly supported will
-//! continue to be evaluated after the batches are materialized.
+//! `array_has_any`) can be evaluated directly during Parquet decoding.
+//! Struct field access via `get_field` (e.g. `WHERE s['value'] > 5`) is also
+//! supported when the accessed leaf is a primitive type. Direct references to
+//! whole struct columns and other unsupported nested projections will continue
+//! to be evaluated after the batches are materialized.
Review Comment:
What would these unsupported examples look like?
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -368,6 +395,31 @@ impl TreeNodeVisitor<'_> for PushdownChecker<'_> {
type Node = Arc<dyn PhysicalExpr>;
fn f_down(&mut self, node: &Self::Node) -> Result<TreeNodeRecursion> {
+ // check for get_field(Column("foo"), "bar", ...) accessing a struct field
+ // the resolved return type tells us the leaf type
+ // if it's non0nested, we can pushdown by recording the root column index and skip children
Review Comment:
```suggestion
// if it's non-nested, we can push down by recording the root column index and skip children
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#issuecomment-4025483431
Let's leave this open for a day to collect feedback 😃
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "friendlymatthew (via GitHub)" <gi...@apache.org>.
friendlymatthew commented on code in PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#discussion_r2906168627
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -1146,6 +1198,228 @@ mod test {
);
}
+ /// get_field(struct_col, 'a') on a struct with a primitive leaf should allow pushdown.
+ #[test]
+ fn get_field_on_struct_allows_pushdown() {
+ let table_schema = Arc::new(Schema::new(vec![Field::new(
+ "struct_col",
+ DataType::Struct(
+ vec![Arc::new(Field::new("a", DataType::Int32, true))].into(),
+ ),
+ true,
+ )]));
+
+ // get_field(struct_col, 'a') > 5
+ let get_field_expr = datafusion_functions::core::get_field().call(vec![
+ col("struct_col"),
+ Expr::Literal(ScalarValue::Utf8(Some("a".to_string())), None),
+ ]);
+ let expr = get_field_expr.gt(Expr::Literal(ScalarValue::Int32(Some(5)), None));
+ let expr = logical2physical(&expr, &table_schema);
+
+ assert!(can_expr_be_pushed_down_with_schemas(&expr, &table_schema));
+ }
+
+ /// get_field on a struct field that resolves to a nested type should still block pushdown.
+ #[test]
+ fn get_field_on_nested_leaf_prevents_pushdown() {
+ let inner_struct = DataType::Struct(
+ vec![Arc::new(Field::new("x", DataType::Int32, true))].into(),
+ );
+ let table_schema = Arc::new(Schema::new(vec![Field::new(
+ "struct_col",
+ DataType::Struct(
+ vec![Arc::new(Field::new("nested", inner_struct, true))].into(),
+ ),
+ true,
+ )]));
+
+ // get_field(struct_col, 'nested') IS NOT NULL — the leaf is still a struct
+ let get_field_expr = datafusion_functions::core::get_field().call(vec![
+ col("struct_col"),
+ Expr::Literal(ScalarValue::Utf8(Some("nested".to_string())), None),
+ ]);
+ let expr = get_field_expr.is_not_null();
+ let expr = logical2physical(&expr, &table_schema);
+
+ assert!(!can_expr_be_pushed_down_with_schemas(&expr, &table_schema));
+ }
+
+ /// get_field on a struct produces correct Parquet leaf indices.
+ #[test]
+ fn get_field_filter_candidate_has_correct_leaf_indices() {
+ use arrow::array::{Int32Array, StringArray, StructArray};
+
+ // Schema: id (Int32), s (Struct{value: Int32, label: Utf8})
+ // Parquet leaves: id=0, s.value=1, s.label=2
+ let struct_fields: arrow::datatypes::Fields = vec![
+ Arc::new(Field::new("value", DataType::Int32, false)),
+ Arc::new(Field::new("label", DataType::Utf8, false)),
+ ]
+ .into();
+ let schema = Arc::new(Schema::new(vec![
+ Field::new("id", DataType::Int32, false),
+ Field::new("s", DataType::Struct(struct_fields.clone()), false),
+ ]));
+
+ let batch = RecordBatch::try_new(
+ Arc::clone(&schema),
+ vec![
+ Arc::new(Int32Array::from(vec![1, 2, 3])),
+ Arc::new(StructArray::new(
+ struct_fields,
+ vec![
+ Arc::new(Int32Array::from(vec![10, 20, 30])) as _,
+ Arc::new(StringArray::from(vec!["a", "b", "c"])) as _,
+ ],
+ None,
+ )),
+ ],
+ )
+ .unwrap();
+
+ let file = NamedTempFile::new().expect("temp file");
+ let mut writer =
+ ArrowWriter::try_new(file.reopen().unwrap(), Arc::clone(&schema), None)
+ .expect("writer");
+ writer.write(&batch).expect("write batch");
+ writer.close().expect("close writer");
+
+ let reader_file = file.reopen().expect("reopen file");
+ let builder = ParquetRecordBatchReaderBuilder::try_new(reader_file)
+ .expect("reader builder");
+ let metadata = builder.metadata().clone();
+ let file_schema = builder.schema().clone();
+
+ // get_field(s, 'value') > 5
+ let get_field_expr = datafusion_functions::core::get_field().call(vec![
+ col("s"),
+ Expr::Literal(ScalarValue::Utf8(Some("value".to_string())), None),
+ ]);
+ let expr = get_field_expr.gt(Expr::Literal(ScalarValue::Int32(Some(5)), None));
+ let expr = logical2physical(&expr, &file_schema);
+
+ let candidate = FilterCandidateBuilder::new(expr, file_schema)
+ .build(&metadata)
+ .expect("building candidate")
+ .expect("get_field filter on struct should be pushable");
+
+ // The root column is s (Arrow index 1), which expands to Parquet
+ // leaves 1 (s.value) and 2 (s.label).
+ assert_eq!(
+ candidate.projection.leaf_indices,
+ vec![1, 2],
+ "leaf_indices should contain both leaves of struct s"
+ );
+ }
+
+ /// Deeply nested get_field: get_field(struct_col, 'outer', 'inner') where the
+ /// leaf is primitive should allow pushdown. The logical simplifier flattens
+ /// nested get_field(get_field(col, 'a'), 'b') into get_field(col, 'a', 'b').
+ #[test]
+ fn get_field_deeply_nested_allows_pushdown() {
Review Comment:
Some proof that nested field access works
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -368,6 +394,31 @@ impl TreeNodeVisitor<'_> for PushdownChecker<'_> {
type Node = Arc<dyn PhysicalExpr>;
fn f_down(&mut self, node: &Self::Node) -> Result<TreeNodeRecursion> {
+ // check for get_field(Column("foo"), "bar", ...) accessing a struct field
+ // the resolved return type tells us the leaf type
+ // if it's non0nested, we can pushdown by recording the root column index and skip children
+ // this way, the visitor never sees the raw struct Column and rejects it
+ if let Some(func) = node.as_any().downcast_ref::<ScalarFunctionExpr>()
+ && func.name() == "get_field"
Review Comment:
We specifically target `get_field` expressions here because it's the only way to access struct fields in datafusion. Bracket notation, dot notation, and the function form all compile to it
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -294,6 +303,23 @@ impl<'schema> PushdownChecker<'schema> {
}
}
+ fn check_struct_field_column(
Review Comment:
like `check_single_column`, but for struct columns accessed via `get_field`
We record the root column index without blocking on the struct type, since the caller has already verified the accessed leaf is a primitive
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -1146,6 +1198,228 @@ mod test {
);
}
+ /// get_field(struct_col, 'a') on a struct with a primitive leaf should allow pushdown.
+ #[test]
+ fn get_field_on_struct_allows_pushdown() {
+ let table_schema = Arc::new(Schema::new(vec![Field::new(
+ "struct_col",
+ DataType::Struct(
+ vec![Arc::new(Field::new("a", DataType::Int32, true))].into(),
+ ),
+ true,
+ )]));
+
+ // get_field(struct_col, 'a') > 5
+ let get_field_expr = datafusion_functions::core::get_field().call(vec![
+ col("struct_col"),
+ Expr::Literal(ScalarValue::Utf8(Some("a".to_string())), None),
+ ]);
+ let expr = get_field_expr.gt(Expr::Literal(ScalarValue::Int32(Some(5)), None));
+ let expr = logical2physical(&expr, &table_schema);
+
+ assert!(can_expr_be_pushed_down_with_schemas(&expr, &table_schema));
+ }
+
+ /// get_field on a struct field that resolves to a nested type should still block pushdown.
+ #[test]
Review Comment:
we don't allow all struct columns through because Parquet doesn't store structs as a single column
i.e. we don't push `struct_col IS NOT NULL` down
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "cetra3 (via GitHub)" <gi...@apache.org>.
cetra3 commented on code in PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#discussion_r2908036318
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -294,6 +308,42 @@ impl<'schema> PushdownChecker<'schema> {
}
}
+ /// Checks whether a struct's root column exists in the file schema and, if so,
+ /// records its index so the entire struct is decoded for filter evaluation.
+ ///
+ /// This is called when we see a `get_field` expression that resolves to a
+ /// primitive leaf type. We only need the *root* column index because the
+ /// Parquet reader decodes all leaves of a struct together.
+ ///
+ /// # Example
+ ///
+ /// Given file schema `{a: Int32, s: Struct(foo: Utf8, bar: Int64)}` and the
+ /// expression `get_field(s, 'foo') = 'hello'`:
+ ///
+ /// - `column_name` = `"s"` (the root struct column)
+ /// - `file_schema.index_of("s")` returns `1`
+ /// - We push `1` into `required_columns`
+ /// - Return `None` (no issue — traversal continues in the caller)
+ ///
+ /// If `"s"` is not in the file schema (e.g. a projected-away column), we set
+ /// `projected_columns = true` and return `Jump` to skip the subtree.
+ fn check_struct_field_column(
Review Comment:
This feels a little weird to me, I'm wondering if there is a way to combine it with `check_single_column`
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "AdamGS (via GitHub)" <gi...@apache.org>.
AdamGS commented on code in PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#discussion_r2906200029
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -368,6 +394,31 @@ impl TreeNodeVisitor<'_> for PushdownChecker<'_> {
type Node = Arc<dyn PhysicalExpr>;
fn f_down(&mut self, node: &Self::Node) -> Result<TreeNodeRecursion> {
+ // check for get_field(Column("foo"), "bar", ...) accessing a struct field
+ // the resolved return type tells us the leaf type
+ // if it's non0nested, we can pushdown by recording the root column index and skip children
+ // this way, the visitor never sees the raw struct Column and rejects it
+ if let Some(func) = node.as_any().downcast_ref::<ScalarFunctionExpr>()
+ && func.name() == "get_field"
Review Comment:
you can do `ScalarFunctionExpr::try_downcast_func::<GetFieldFunc>(func)` here
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "friendlymatthew (via GitHub)" <gi...@apache.org>.
friendlymatthew commented on PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#issuecomment-4025466833
> > > This looks great to me!
> > > How do we generate the right ProjectionMask / translate into the right leaf column index in Parquet? I don't see that added anywhere but maybe the existing code already did that correctly?
> >
> >
> > We do not. I added a note in the PR message:
> > > Note: this does not address the projection side and should not be blocked by it. SELECT s['foo'] still reads the entire struct rather than just the needed leaf column. That requires separate changes to how the opener builds its projection mask.
> >
> >
> > I have some ideas about this, and will push up a follow up PR
>
> Wording It's a bit confusing here, because projection can mean one of two things: the select part of the query, or the projection of the columns that the filters need to be evaluated. I assume here you are referring to the latter, i.e. although we support these filters now, we still read the entire struct column and then apply the get field operation in memory?
Yes exactly. Filters on struct fields are now pushed down to the row-level filter, but the projection side still reads the entire struct column. I still need to teach Datafusion to project only the needed subcolumns (leaf columns) of the struct rather than materializing the whole thing
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "friendlymatthew (via GitHub)" <gi...@apache.org>.
friendlymatthew commented on PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#issuecomment-4025415489
> This looks great to me!
>
> How do we generate the right ProjectionMask / translate into the right leaf column index in Parquet? I don't see that added anywhere but maybe the existing code already did that correctly?
We do not. I added a note in the PR message:
> Note: this does not address the projection side and should not be blocked by it. SELECT s['foo'] still reads the entire struct rather than just the needed leaf column. That requires separate changes to how the opener builds its projection mask.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#issuecomment-4025479472
Great makes sense!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "cetra3 (via GitHub)" <gi...@apache.org>.
cetra3 commented on code in PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#discussion_r2908034845
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -368,6 +418,47 @@ impl TreeNodeVisitor<'_> for PushdownChecker<'_> {
type Node = Arc<dyn PhysicalExpr>;
fn f_down(&mut self, node: &Self::Node) -> Result<TreeNodeRecursion> {
+ // Handle struct field access like `s['foo']['bar'] > 10`.
+ //
+ // DataFusion represents nested field access as `get_field(Column("s"), "foo")`
+ // (or chained: `get_field(get_field(Column("s"), "foo"), "bar")`).
+ //
+ // We intercept the outermost `get_field` on the way *down* the tree so
+ // the visitor never reaches the raw `Column("s")` node. Without this,
+ // `check_single_column` would see that `s` is a Struct and reject it.
+ //
+ // The strategy:
+ // 1. Match `get_field` whose first arg is a `Column` (the struct root).
+ // 2. Check that the *resolved* return type is primitive — meaning we've
+ // drilled all the way to a leaf (e.g. `s['foo']` → Utf8).
+ // 3. Record the root column index via `check_struct_field_column` and
+ // return `Jump` to skip visiting the children (the Column and the
+ // literal field-name args), since we've already handled them.
+ //
+ // If the return type is still nested (e.g. `s['nested_struct']` → Struct),
+ // we fall through and let normal traversal continue, which will
+ // eventually reject the expression when it hits the struct Column.
+ if let Some(func) =
+ ScalarFunctionExpr::try_downcast_func::<GetFieldFunc>(node.as_ref())
Review Comment:
Is there any other functions that can enumerate into fields? Or is `get_field` the main one.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow filters on struct fields to be pushed down into Parquet scan [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #20822:
URL: https://github.com/apache/datafusion/pull/20822#discussion_r2911813110
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -368,6 +418,47 @@ impl TreeNodeVisitor<'_> for PushdownChecker<'_> {
type Node = Arc<dyn PhysicalExpr>;
fn f_down(&mut self, node: &Self::Node) -> Result<TreeNodeRecursion> {
+ // Handle struct field access like `s['foo']['bar'] > 10`.
+ //
+ // DataFusion represents nested field access as `get_field(Column("s"), "foo")`
+ // (or chained: `get_field(get_field(Column("s"), "foo"), "bar")`).
+ //
+ // We intercept the outermost `get_field` on the way *down* the tree so
+ // the visitor never reaches the raw `Column("s")` node. Without this,
+ // `check_single_column` would see that `s` is a Struct and reject it.
+ //
+ // The strategy:
+ // 1. Match `get_field` whose first arg is a `Column` (the struct root).
+ // 2. Check that the *resolved* return type is primitive — meaning we've
+ // drilled all the way to a leaf (e.g. `s['foo']` → Utf8).
+ // 3. Record the root column index via `check_struct_field_column` and
+ // return `Jump` to skip visiting the children (the Column and the
+ // literal field-name args), since we've already handled them.
+ //
+ // If the return type is still nested (e.g. `s['nested_struct']` → Struct),
+ // we fall through and let normal traversal continue, which will
+ // eventually reject the expression when it hits the struct Column.
+ if let Some(func) =
+ ScalarFunctionExpr::try_downcast_func::<GetFieldFunc>(node.as_ref())
Review Comment:
addressed offline (I spoke to @friendlymatthew and @cetra3 )
##########
datafusion/datasource-parquet/src/row_filter.rs:
##########
@@ -294,6 +308,42 @@ impl<'schema> PushdownChecker<'schema> {
}
}
+ /// Checks whether a struct's root column exists in the file schema and, if so,
+ /// records its index so the entire struct is decoded for filter evaluation.
+ ///
+ /// This is called when we see a `get_field` expression that resolves to a
+ /// primitive leaf type. We only need the *root* column index because the
+ /// Parquet reader decodes all leaves of a struct together.
+ ///
+ /// # Example
+ ///
+ /// Given file schema `{a: Int32, s: Struct(foo: Utf8, bar: Int64)}` and the
+ /// expression `get_field(s, 'foo') = 'hello'`:
+ ///
+ /// - `column_name` = `"s"` (the root struct column)
+ /// - `file_schema.index_of("s")` returns `1`
+ /// - We push `1` into `required_columns`
+ /// - Return `None` (no issue — traversal continues in the caller)
+ ///
+ /// If `"s"` is not in the file schema (e.g. a projected-away column), we set
+ /// `projected_columns = true` and return `Jump` to skip the subtree.
+ fn check_struct_field_column(
Review Comment:
addressed offline (I spoke to @friendlymatthew and @cetra3 )
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org