voonhous commented on code in PR #20532:
URL: https://github.com/apache/hudi/pull/20532#discussion_r4488743481
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/dataskipping/rules/ConstantOperandRule.scala:
##########
@@ -17,23 +17,33 @@
package org.apache.spark.sql.hudi.dataskipping.rules
-import org.apache.spark.sql.catalyst.expressions.{Add, BitwiseOr, DateAdd,
DateDiff, DateSub, Divide, Expression, Literal, Multiply, ShiftLeft, ShiftRight}
-import org.apache.spark.sql.hudi.dataskipping.{Increasing, RangeRule, Step}
-import org.apache.spark.sql.types.Decimal
+import org.apache.spark.sql.catalyst.expressions.{Add, And, BitwiseOr,
DateAdd, DateDiff, DateSub, Divide, Expression, GreaterThanOrEqual,
LessThanOrEqual, Literal, Multiply, ShiftLeft, ShiftRight}
+import org.apache.spark.sql.hudi.dataskipping.{Increasing, RangeRule, Step,
TermRange}
+import org.apache.spark.sql.types.{ByteType, DataType, Decimal, DoubleType,
FloatType, IntegerType, LongType, ShortType}
+
+import scala.util.Try
/**
* Binary operators with the column on one side and a constant on the other:
* `Add`, `Multiply`, `Divide`, `DateAdd`, `DateSub`, `DateDiff`, `ShiftLeft`,
`ShiftRight`, `BitwiseOr`.
*
* Shared guard: the other operand is constant. Today that is enforced per
function:
- * - `Add`: the other side is foldable (either side)
- * - `Multiply`: the other side is a positive numeric literal (either side)
+ * - `Add`: float / double result and a finite constant, or integral result
and a constant (either side)
+ * - `Multiply`: float / double or integral result, and the other side is a
positive numeric literal
+ * (either side)
* - `Divide`: the divisor is a positive numeric literal (column as dividend
only)
- * - `DateAdd`, `DateSub`, `DateDiff` (end only), `ShiftLeft`, `ShiftRight`,
`BitwiseOr`: ANY second
- * operand is accepted today, constant or not
+ * - `ShiftLeft`: the shift amount is constant
+ * - `DateAdd`, `DateSub`, `DateDiff` (end only), `ShiftRight`, `BitwiseOr`:
ANY second operand is
+ * accepted today, constant or not
*
* NOTE: A column-referencing other operand could reverse the ordering (for
ex, A + A * -2 = -A), and a
* non-deterministic one is re-evaluated against the min/max bounds (for
ex, A + rand())
+ *
+ * NOTE: Integral `Add`, `Multiply` and `ShiftLeft` wrap around on overflow in
non-ANSI mode, so the
+ * max can map below the min (for ex, Long.MaxValue * 2 = -2), and throw
in ANSI mode. Their
Review Comment:
Fixed: the NOTE, the `IntegralLimits` scaladoc, the test comment and the
description now say only `Add` / `Multiply` throw under ANSI.
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/dataskipping/rules/ConstantOperandRule.scala:
##########
@@ -17,23 +17,33 @@
package org.apache.spark.sql.hudi.dataskipping.rules
-import org.apache.spark.sql.catalyst.expressions.{Add, BitwiseOr, DateAdd,
DateDiff, DateSub, Divide, Expression, Literal, Multiply, ShiftLeft, ShiftRight}
-import org.apache.spark.sql.hudi.dataskipping.{Increasing, RangeRule, Step}
-import org.apache.spark.sql.types.Decimal
+import org.apache.spark.sql.catalyst.expressions.{Add, And, BitwiseOr,
DateAdd, DateDiff, DateSub, Divide, Expression, GreaterThanOrEqual,
LessThanOrEqual, Literal, Multiply, ShiftLeft, ShiftRight}
+import org.apache.spark.sql.hudi.dataskipping.{Increasing, RangeRule, Step,
TermRange}
+import org.apache.spark.sql.types.{ByteType, DataType, Decimal, DoubleType,
FloatType, IntegerType, LongType, ShortType}
+
+import scala.util.Try
/**
* Binary operators with the column on one side and a constant on the other:
* `Add`, `Multiply`, `Divide`, `DateAdd`, `DateSub`, `DateDiff`, `ShiftLeft`,
`ShiftRight`, `BitwiseOr`.
*
* Shared guard: the other operand is constant. Today that is enforced per
function:
- * - `Add`: the other side is foldable (either side)
- * - `Multiply`: the other side is a positive numeric literal (either side)
+ * - `Add`: float / double result and a finite constant, or integral result
and a constant (either side)
+ * - `Multiply`: float / double or integral result, and the other side is a
positive numeric literal
+ * (either side)
* - `Divide`: the divisor is a positive numeric literal (column as dividend
only)
- * - `DateAdd`, `DateSub`, `DateDiff` (end only), `ShiftLeft`, `ShiftRight`,
`BitwiseOr`: ANY second
- * operand is accepted today, constant or not
+ * - `ShiftLeft`: the shift amount is constant
+ * - `DateAdd`, `DateSub`, `DateDiff` (end only), `ShiftRight`, `BitwiseOr`:
ANY second operand is
+ * accepted today, constant or not
*
* NOTE: A column-referencing other operand could reverse the ordering (for
ex, A + A * -2 = -A), and a
* non-deterministic one is re-evaluated against the min/max bounds (for
ex, A + rand())
+ *
+ * NOTE: Integral `Add`, `Multiply` and `ShiftLeft` wrap around on overflow in
non-ANSI mode, so the
+ * max can map below the min (for ex, Long.MaxValue * 2 = -2), and throw
in ANSI mode. Their
+ * steps carry a file guard that the file's range does not overflow (see
[[IntegralType]]).
+ * Float / double results round monotonically and saturate to infinity
instead. Decimal results
+ * overflow into NULL and are not matched (#20460)
Review Comment:
Done here: decimal `Add`, `Multiply` and `Divide` are now `partial` steps,
which also fixes `Divide` dropping files on master (for ex, `CAST(A AS
DECIMAL(38,0)) / 0.000000000000000001BD > 0` over `{1, Long.MaxValue}`).
--
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]
