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. I rejected every non-boolean cast to
boolean, not just numeric ones, because string to boolean breaks the same way.
I added a test for each in `TestDataSkippingUtils`. Both fail without the
fix:
- `CAST(C AS STRING) > '2026-11-01 01:20:00'` in America/New_York, which is
your probe case. The file holds 01:30 EDT and 01:10 EST, and the 01:30 row
matches but the file was skipped.
- `CAST(A AS BOOLEAN) = false` on a file with min -5 and max 5. Both bounds
become `true`, so the file was skipped even though it holds a 0.
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]
