kosiew commented on code in PR #21545:
URL: https://github.com/apache/datafusion/pull/21545#discussion_r3068731757
##########
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:
Good point. This PR does not add nested-field pruning support, and I agree
the old comment was still documenting a real limitation rather than a
CastColumnExpr-only detail. The branch was collapsed as part of the CastExpr
unification, but the nested-field/statistics naming question is still unchanged.
I will restore the note in the unified CastExpr handling path, adjusted to
avoid referencing CastColumnExpr, and keep it clear that this PR only preserves
existing behavior. I do not plan to add nested-field pruning tests here because
this PR intentionally does not change that behavior; those should come with the
future nested-field pruning/statistics work.
--
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]
