cloud-fan commented on code in PR #57956:
URL: https://github.com/apache/spark/pull/57956#discussion_r3784612400
##########
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:
I disagree with the characterization that `throwable` is not this rule's
flag to use.
`Expression.throwable` is the standard expression-level contract for
optimizer transformations
that may change whether an expression is evaluated and therefore whether an
exception is
observable. The fact that `CombineFilters` and `PushPredicateThroughJoin`
also respect it is
intentional: if an expression is classified as throwable, all rules that can
change its evaluation
semantics should react consistently rather than maintaining rule-specific
classifications.
It is true that the check does not change behavior for today's hoistable
`VariantGet` and `Cast`
expressions, but that makes it forward-looking rather than conceptually
inert. I updated the class
documentation to state the current classification and the behavior if an
extraction becomes
throwable. I also addressed the separate implementation concerns in
82ad6dbcfcf: join crossing now
mirrors actual alias placement through pass-through Projects, and the unused
hoister predicate in
the direct Join path is gone.
--
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]
