yadavay-amzn commented on code in PR #56101:
URL: https://github.com/apache/spark/pull/56101#discussion_r3338513151
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/RewriteNearestByJoin.scala:
##########
@@ -72,7 +73,15 @@ object RewriteNearestByJoin extends Rule[LogicalPlan] {
private lazy val random = new scala.util.Random()
def apply(plan: LogicalPlan): LogicalPlan = plan.transformUp {
- case j @ NearestByJoin(left, right, joinType, _, numResults,
rankingExpression, direction) =>
+ case j @ NearestByJoin(left, right, joinType, _, numResults,
rankingExpression, direction)
+ // The optimizer checks both the config flag AND the broadcast threshold
to decide
+ // whether to skip the rewrite. This mixes optimizer/planner concerns
but is necessary:
+ // if we only checked the config flag, a NearestByJoin node with right
side exceeding
+ // the broadcast threshold would reach the planner unrewritten, and no
strategy would
+ // handle it (NearestByJoinSelection returns Nil for large right sides),
causing a
+ // planning failure. The alternative (two-pass approach) is deferred to
future work.
+ if !(SQLConf.get.nearestByBroadcastEnabled &&
Review Comment:
Extracted `NearestByJoin.canBroadcastRight(j, conf)` as single source of
truth, used by both `RewriteNearestByJoin` and `NearestByJoinSelection`. Also
added `sizeInBytes >= 0` guard matching the standard broadcast eligibility
pattern.
--
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]
