voonhous commented on code in PR #19836:
URL: https://github.com/apache/hudi/pull/19836#discussion_r3949948669
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -521,11 +521,15 @@ object HoodieProcedureFilterUtils {
right: org.apache.spark.sql.catalyst.expressions.Expression,
constructor:
(org.apache.spark.sql.catalyst.expressions.Expression,
org.apache.spark.sql.catalyst.expressions.Expression) => T,
original: T): T = {
- (left, right) match {
- case (boundRef:
org.apache.spark.sql.catalyst.expressions.BoundReference, literal:
org.apache.spark.sql.catalyst.expressions.Literal)
- if boundRef.dataType == org.apache.spark.sql.types.LongType &&
literal.dataType == org.apache.spark.sql.types.IntegerType =>
- val castExpr =
org.apache.spark.sql.catalyst.expressions.Cast(boundRef,
org.apache.spark.sql.types.IntegerType)
- constructor(castExpr, literal)
+ (left.dataType, right.dataType) match {
+ case (_: NumericType, _: NumericType) =>
Review Comment:
Addressed: `EqualNullSafe` now uses the shared numeric coercion path. Tests
cover both validation and the expected matching row for `ts <=> 1000` on a Long
column, plus `ts <=> null`.
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -516,17 +528,91 @@ object HoodieProcedureFilterUtils {
}
}
- private def applyTypeCoercion[T <:
org.apache.spark.sql.catalyst.expressions.Expression](
-
left: org.apache.spark.sql.catalyst.expressions.Expression,
-
right: org.apache.spark.sql.catalyst.expressions.Expression,
-
constructor:
(org.apache.spark.sql.catalyst.expressions.Expression,
org.apache.spark.sql.catalyst.expressions.Expression) => T,
-
original: T): T = {
- (left, right) match {
- case (boundRef:
org.apache.spark.sql.catalyst.expressions.BoundReference, literal:
org.apache.spark.sql.catalyst.expressions.Literal)
- if boundRef.dataType == org.apache.spark.sql.types.LongType &&
literal.dataType == org.apache.spark.sql.types.IntegerType =>
- val castExpr =
org.apache.spark.sql.catalyst.expressions.Cast(boundRef,
org.apache.spark.sql.types.IntegerType)
- constructor(castExpr, literal)
+ private def applyTypeCoercion[T <: Expression](left: Expression,
+ right: Expression,
+ constructor: (Expression,
Expression) => T,
+ original: T): T = {
+ widenNumericOperands(Seq(left, right)) match {
+ case Some(Seq(widenedLeft, widenedRight)) => constructor(widenedLeft,
widenedRight)
case _ => original
}
}
+
+ private def applyInTypeCoercion(in: In): Expression = {
+ widenNumericOperands(in.value +: in.list) match {
+ case Some(widened) => In(widened.head, widened.tail)
+ case _ => in
+ }
+ }
+
+ private def applyArithmeticTypeCoercion(arith: BinaryArithmetic): Expression
= {
+ widenNumericOperands(Seq(arith.left, arith.right)) match {
Review Comment:
Good catch on the gap, though the framing is off: this is not new.
`Divide(Long, Int)` was already unresolved before the arithmetic widening,
because `BinaryOperator.checkInputDataTypes` rejects differing operand types,
so `ts / 2 > 500` failed validation on master too. The widening makes it
`Divide(Long, Long)`, which is still outside `TypeCollection(DoubleType,
DecimalType)` -- same rejection, different reason.
Fixed anyway, since the inconsistency with `price / 2 > 5` is real: `Divide`
is now matched ahead of the general `BinaryArithmetic` case and an integral
pair is promoted to `DoubleType`, mirroring the analyzer's `Division` rule. A
pair that already involves a decimal keeps widening the way the other
arithmetic does. The rebuild goes through `withNewChildren` so the
`failOnError`/`evalMode` constructor difference between 3.3 and 3.4+ stays out
of it.
--
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]
