wangyum commented on code in PR #59344:
URL: https://github.com/apache/spark/pull/59344#discussion_r4296406514
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ExprUtils.scala:
##########
@@ -262,6 +264,9 @@ object ExprUtils extends EvalHelper with QueryErrorsBase {
private def canEvaluateUnconditionallyInternal(e: Expression): Boolean = e
match {
case _: AttributeReference | _: Literal => true
+ case Cast(child, toType, _, _) =>
+ child.dataType.isInstanceOf[NumericType] &&
toType.isInstanceOf[NumericType] &&
+ Cast.canUpCast(child.dataType, toType) &&
canEvaluateUnconditionallyInternal(child)
Review Comment:
Fixed in 7afd4fde9bc: a decimal source now widens only to a decimal target,
and `Cast(<DECIMAL(19,0) attr>, LongType)` is asserted rejected at both the
unit and the split level.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/joins/SplitStreamedSideJoinConditionSuite.scala:
##########
@@ -151,6 +163,15 @@ class SplitStreamedSideJoinConditionSuite extends
QueryTest with SharedSparkSess
|ON s.id = b.bid AND s.a < 5
""".stripMargin))
+ testWithCodegenOnAndOff(
+ "left outer join with lossless widening cast in streamed-only condition")(
+ spark.sql("""
+ |SELECT /*+ BROADCAST(b) */ s.id, s.a, s.pad, b.bid
+ |FROM (SELECT id, CAST(id AS int) AS a, pad FROM streamed_t WHERE pad %
2 = 0) s
+ |LEFT OUTER JOIN (SELECT DISTINCT bid FROM build_t) b
+ |ON s.id = b.bid AND CAST(s.a AS bigint) < 5L
Review Comment:
Fixed in 7afd4fde9bc: the test now uses `CAST(s.id AS double) < 5.0D`, which
survives UnwrapCastInBinaryComparison (SPARK-39476), and asserts from the
executed `BroadcastHashJoinExec` that the Cast conjunct lands in the
streamed-only part.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/ExprUtils.scala:
##########
@@ -243,14 +243,16 @@ object ExprUtils extends EvalHelper with QueryErrorsBase {
*
* Only a whitelist of total, deterministic expressions qualifies:
* - leaves: attribute references and literals;
+ * - lossless widening numeric casts: Cast where both types are numeric
and Cast.canUpCast
Review Comment:
Fixed in 7afd4fde9bc: reworded to "non-throwing widening numeric up-casts
(Cast.canUpCast)" since Int/Long -> Float/Double can round, and the "lossless"
test names were renamed accordingly.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExprUtilsSuite.scala:
##########
@@ -44,6 +46,10 @@ class ExprUtilsSuite extends SparkFunSuite {
assert(!arithmetic.throwable)
assert(!ExprUtils.canEvaluateUnconditionally(arithmetic))
assert(!ExprUtils.canEvaluateUnconditionally(EqualTo(Remainder(a,
Literal(3L)), Literal(0L))))
+ // Narrowing cast (Long -> Int), parsing cast (String -> Int), and
non-numeric cast.
Review Comment:
Fixed in 7afd4fde9bc: added
`assert(!ExprUtils.canEvaluateUnconditionally(Cast(Add(i, Literal(1)),
LongType)))` to pin the child-recursion conjunct.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
