Re: [PR] feat: Integrate CastColumnExpr into PhysicalExprAdapter [datafusion]

["adriangb (via GitHub)" <gi...@apache.org>]

adriangb commented on code in PR #20269:
URL: https://github.com/apache/datafusion/pull/20269#discussion_r2888859227


##########
datafusion/physical-expr/src/expressions/cast_column.rs:
##########
@@ -177,6 +179,24 @@ impl PhysicalExpr for CastColumnExpr {
         )))
     }
 
+    /// A [`CastColumnExpr`] preserves the ordering of its child if the cast is done
+    /// under the same datatype family.

Review Comment:
   Do we have a high level SLT test for this? I'm afraid that as we refactor to e.g. fold CastColumnExpr into CastExpr we'll loose this property (which seems very valuable).



##########
datafusion/physical-expr/src/equivalence/properties/mod.rs:
##########
@@ -853,6 +853,18 @@ impl EquivalenceProperties {
                                     sort_expr.options,
                                 ));
                             }
+                        } else if let Some(cast_expr) =
+                            r_expr.as_any().downcast_ref::<CastColumnExpr>()

Review Comment:
   I think this is the unfortunate part of having two cast expressions: we have to handle both in multiple places.
   
   @paleolimbot this makes me think that even customizing casts with a cast registry that chooses a different cast expression will be problematic. It almost feels like we either need to (1) have a single `Cast` expression that internally dispatches (so code snippets like this can match on a single expression) or (2) have some way to determine "is this a cast expression?". To complicate matters further: do all cast expressions share the same properties e.g. preserving order ([see](https://github.com/apache/datafusion/pull/20269/changes#r2888859227)) 



-- 
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