adriangb commented on code in PR #21545:
URL: https://github.com/apache/datafusion/pull/21545#discussion_r3068322818
##########
datafusion/pruning/src/pruning_predicate.rs:
##########
@@ -1124,40 +1123,30 @@ fn rewrite_expr_to_prunable(
Ok((Arc::clone(column_expr), op, Arc::clone(scalar_expr)))
} else if let Some(cast) =
column_expr_any.downcast_ref::<phys_expr::CastExpr>() {
// `cast(col) op lit()`
- let arrow_schema = schema.as_arrow();
- let from_type = cast.expr().data_type(arrow_schema)?;
- verify_support_type_for_prune(&from_type, cast.cast_type())?;
- let (left, op, right) =
- rewrite_expr_to_prunable(cast.expr(), op, scalar_expr, schema)?;
- let left = Arc::new(phys_expr::CastExpr::new(
+ let (left, op, right) = rewrite_cast_child_to_prunable(
+ cast.expr(),
+ cast.cast_type(),
+ op,
+ scalar_expr,
+ schema,
+ )?;
+ let left = Arc::new(phys_expr::CastExpr::new_with_target_field(
left,
- cast.cast_type().clone(),
+ Arc::clone(cast.target_field()),
None,
));
Ok((left, op, right))
- } else if let Some(cast_col) =
column_expr_any.downcast_ref::<CastColumnExpr>() {
- // `cast_column(col) op lit()` - same as CastExpr but uses
CastColumnExpr
- let arrow_schema = schema.as_arrow();
- let from_type = cast_col.expr().data_type(arrow_schema)?;
- let to_type = cast_col.target_field().data_type();
- verify_support_type_for_prune(&from_type, to_type)?;
- let (left, op, right) =
- rewrite_expr_to_prunable(cast_col.expr(), op, scalar_expr,
schema)?;
- // Predicate pruning / statistics generally don't support struct
columns yet.
- // In the future we may want to support pruning on nested fields, in
which case we probably need to
- // do something more sophisticated here.
- // But for now since we don't support pruning on nested fields, we can
just cast to the target type directly.
- let left = Arc::new(phys_expr::CastExpr::new(left, to_type.clone(),
None));
Review Comment:
Since this comment was removed I'm curious what the current state of this is.
AFAIK:
- PruningPredicate doesn't support nested fields (did that change in this
PR? if so we should have tests)
- Statistics extraction for Parquet doesn't support nested fields (hence
even if PruningPredicate did e2e would still not work); since PruningPredicate
ultimately produces a stats expression to evaluate against a RecordBatch of
stats we'd need some agreement on the column names for nested expressions
(`"get_field("col", 'field')_min"`?).
If this comment is still true lets preserve it.
--
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]
