cshuo commented on code in PR #20457:
URL: https://github.com/apache/hudi/pull/20457#discussion_r4409969972
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/BaseHoodieCatalystExpressionUtils.scala:
##########
@@ -83,9 +83,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:
Could we also guard the other operand of `Add` when it references the same
column? For `A + A * -2 < -5` with file values `{0, 10}`, the `Add` case above
accepts the left `A` without inspecting the negative multiplication, and the
expression still has only one distinct attribute. Data skipping then evaluates
the expression at the minimum (`0`) and prunes the file, even though `A = 10`
matches. Spark 3.5.4 retains this expression in the optimized plan. This is an
existing `Add` issue, but it allows the negative factor addressed here to
bypass the new check.
--
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]
