haohuaijin commented on code in PR #25227:
URL: https://github.com/apache/datafusion/pull/25227#discussion_r3996341542
##########
datafusion/physical-expr/src/expressions/cast.rs:
##########
@@ -249,8 +249,11 @@ impl CastExpr {
})
}
- /// Check if casting from the specified source type to the target type is a
- /// widening cast (e.g. from `Int8` to `Int16`).
+ /// Check if casting from the source type to the target type is known to be
+ /// lossless and strictly order-preserving for all source values,
preserving nulls.
+ /// This includes widening casts (e.g. `Int8` to `Int16`) and
representation
+ /// conversions such as `Int32` to `Date32`, which interprets the same
integer
+ /// as days since the epoch.
pub fn check_bigger_cast(cast_type: &DataType, src: &DataType) -> bool {
if cast_type.eq(src) {
return true;
Review Comment:
`check_bigger_cast` is already used for [ordering
inference](https://github.com/apache/datafusion/blob/b94929a87f74dbe194db8ccd528592491d96606f/datafusion/physical-expr/src/expressions/cast.rs#L302).
I added `Int32 ↔ Date32` because Arrow reinterprets the same underlying
values, preserving ordering and nulls. This also keeps ClickBench's `MIN`/`MAX`
statistics optimization working after this PR restricts cast statistics
propagation. Should we rename the helper to reflect its broader contract than
widening?
--
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]
