peter-toth commented on code in PR #57956:
URL: https://github.com/apache/spark/pull/57956#discussion_r3784989041
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/PullOutVariantExtractions.scala:
##########
@@ -177,11 +177,20 @@ object PullOutVariantExtractions extends
Rule[LogicalPlan] {
case _ => false
}
+ private def isJoinHoistable(e: Expression): Boolean = {
Review Comment:
**Finding 4.** Fair enough, keeping `throwable` makes sense.
The one item left from this thread is @qlong's test. Both other `throwable`
callers pin their classification the same way — `assert(!arithmetic.throwable)`
at `ExprUtilsSuite.scala:44` and `assert(!unmarkedThrowable.throwable)` at
`SplitStreamedSideJoinConditionSuite.scala:115` — each beside a comment saying
what the assertion protects:
```scala
test("hoistable variant extractions are not classified as throwable") {
// `isJoinHoistable`'s join gate is inert while this holds. Classifying
either expression as
// throwable stops this rule hoisting extractions across a Join unless
// spark.sql.variant.pushVariantIntoScan.deferCastError is set.
val v = AttributeReference("v", VariantType)()
assert(!VariantGet(v, Literal("$.a"), IntegerType, failOnError = true,
Some("UTC")).throwable)
assert(!Cast(v, IntegerType, Some("UTC")).throwable)
}
```
--
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]
