cshuo commented on code in PR #19475:
URL: https://github.com/apache/hudi/pull/19475#discussion_r3710350258
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/BaseHoodieCatalystExpressionUtils.scala:
##########
@@ -82,9 +82,9 @@ abstract class BaseHoodieCatalystExpressionUtils extends
HoodieCatalystExpressio
// Binary
case Add(OrderPreservingTransformation(attrRef), _, _) => Some(attrRef)
case Add(_, OrderPreservingTransformation(attrRef), _) => Some(attrRef)
- case Multiply(OrderPreservingTransformation(attrRef), _, _) =>
Some(attrRef)
- case Multiply(_, OrderPreservingTransformation(attrRef), _) =>
Some(attrRef)
- case Divide(OrderPreservingTransformation(attrRef), _, _) =>
Some(attrRef)
+ case Multiply(OrderPreservingTransformation(attrRef), factor, _) if
isPositiveNumericLiteral(factor) => Some(attrRef)
Review Comment:
A strictly positive factor is not sufficient to make multiplication
order-preserving because integral arithmetic wraps in non-ANSI mode. For a
bigint file containing `{51, Long.MaxValue}`, `A * 2L > 100` is accepted here
and translated using the transformed max. Spark evaluates `Long.MaxValue * 2L`
as `-2`, so the file is pruned, although `51 * 2L = 102` matches the original
predicate.
Please restrict multiplication to analyzed types/factors for which overflow
cannot break monotonicity, or conservatively reject the unsafe integral/decimal
cases. This exact file-stats scenario should be added as a regression test.
--
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]
