voonhous commented on code in PR #19475:
URL: https://github.com/apache/hudi/pull/19475#discussion_r4452790255
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/HoodieSparkTypeUtils.scala:
##########
@@ -35,10 +36,24 @@ object HoodieSparkTypeUtils {
*/
def isCastPreservingOrdering(from: DataType, to: DataType): Boolean =
(from, to) match {
- // NOTE: In the casting rules defined by Spark, only casting from String
to Numeric
- // (and vice versa) are the only casts that might break the ordering of
the elements after casting
+ // NOTE: Casting between String and Numeric types re-orders elements
(for ex, "10" < "9"
+ // lexicographically)
case (StringType, _: NumericType) => false
case (_: NumericType, StringType) => false
+ // NOTE: Cast.canUpCast rejects decimal to float/double as lossy, but
rounding is monotonic
+ // and every decimal fits within float range. Spark's type
coercion inserts this cast
+ // when a decimal column is compared with a float/double (for ex,
`price > 99.5D`)
+ case (_: DecimalType, FloatType | DoubleType) => true
+ // NOTE: Narrowing numeric casts (for ex, bigint to int) overflow and
wrap around in
+ // non-ANSI mode, breaking ordering; only up-casts are guaranteed
order-preserving.
+ // 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: 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
case _ => true
Review Comment:
Both are rejected now in e3265df: `(TimestampType, StringType)`, and every
non-boolean cast to boolean, since string to boolean breaks the same way.
Repros in `TestDataSkippingUtils`: your `CAST(C AS STRING) > '2026-11-01
01:20:00'` case under America/New_York, and `CAST(A AS BOOLEAN) = false` over
min -5 / max 5. Both pruned the matching file without the fix.
Deny-by-default is the better shape. #20458 rewrites the same match, so I'd
rather flip it once that lands than stack a third conflicting change here. I'll
track it on the #19445 checklist.
--
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]
