wombatu-kun commented on code in PR #20513:
URL: https://github.com/apache/hudi/pull/20513#discussion_r4465648008
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/HoodieSparkTypeUtils.scala:
##########
@@ -69,12 +63,31 @@ object HoodieSparkTypeUtils {
// This is conservative (for ex, double to float rounds
monotonically but is
// rejected), which only costs pruning opportunity, never
correctness
case (fromNum: NumericType, toNum: NumericType) =>
Cast.canUpCast(fromNum, toNum)
+ // NOTE: false < true maps to 0 < 1. Decimal targets are excluded since,
for ex,
+ // DECIMAL(1, 1) cannot hold 1 and overflows into NULL
+ case (BooleanType, ByteType | ShortType | IntegerType | LongType |
FloatType | DoubleType) => true
+
+ // Date and time
// NOTE: Timestamps cast to numbers through their epoch seconds, which
only fit bigint and
// floating-point targets; narrower integral targets wrap around
and decimals overflow
// into NULL in non-ANSI mode
case (TimestampType, LongType | FloatType | DoubleType) => true
- case (TimestampType, _: NumericType) => false
+ // NOTE: A date maps to the start of that day, which increases with the
date
+ case (DateType, TimestampType | TimestampNTZType) => true
+ // NOTE: Truncating to the (local) date is non-decreasing, also across a
DST fall-back since
+ // the date does not repeat
+ case (TimestampType | TimestampNTZType, DateType) => true
+ // NOTE: Not order-preserving in general (for ex, "2022-1-5" >
"2022-01-10" as strings, but parses
+ // to an earlier date), but kept so that expression indexes on
to_date/to_timestamp, which the
+ // optimizer rewrites into this cast, keep pruning. TODO #19481
+ case (StringType, DateType | TimestampType | TimestampNTZType) => true
Review Comment:
Expression indexes never reach this arm:
`ExpressionIndexSupport.extractQueryAndLiterals` unwraps the cast (any
`UnaryExpression`) and `filterQueryWithRecordKey` rebuilds the query over the
bare attribute, so only plain column-stats pruning of a string column depends
on it. Is keeping it for that path intended (the NOTE and the
`TestHoodieCatalystExpressionUtils` comment would then need a different
reason), or can it go in this PR rather than in #19481?
--
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]
