voonhous commented on code in PR #19836:
URL: https://github.com/apache/hudi/pull/19836#discussion_r3948175606
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -496,11 +496,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 {
Review Comment:
Fixed: `widenNumericOperands` returns `None` when either operand is
unresolved, so the transform leaves the node alone.
This got sharper after #19850 merged: `bindAndResolveExpression` is now
called from `validateFilterExpression` too, so the throw also replaced the
`Invalid column references` message for a plain typo like `missing_col > 1`.
Two of that PR's tests fail without the guard.
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -496,11 +496,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) =>
+ TypeCoercion.findWiderTypeForTwo(left.dataType, right.dataType)
Review Comment:
Fixed: the object is picked from `SQLConf.get.ansiEnabled`.
Correction to my own comment, which the reply below inherited: I had the
direction backwards. Probed on catalyst 3.5.5:
```
TypeCoercion.findWiderTypeForTwo(LONG, FLOAT) = Some(FloatType)
AnsiTypeCoercion.findWiderTypeForTwo(LONG, FLOAT) = Some(DoubleType)
```
ANSI is the precise one, not the lossy one. Test pins both modes.
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -496,11 +496,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) =>
+ TypeCoercion.findWiderTypeForTwo(left.dataType, right.dataType)
+ .map { widerType =>
+ val coercedLeft = if (left.dataType == widerType) left else
Cast(left, widerType)
Review Comment:
Fixed: all-decimal comparisons return `original` before widening. Test
covers `DECIMAL(38,0)` against `DECIMAL(38,18)`, which drops the row without 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]
