cloud-fan commented on code in PR #56023:
URL: https://github.com/apache/spark/pull/56023#discussion_r3279180557
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/RewriteNearestByJoinSuite.scala:
##########
@@ -66,7 +66,7 @@ class RewriteNearestByJoinSuite extends PlanTest {
val generate = Generate(
Inline(matchesAlias.toAttribute),
unrequiredChildIndex =
Seq(aggregate.output.indexOf(matchesAlias.toAttribute)),
- outer = outer,
+ outer = joinType == LeftOuter,
Review Comment:
This now mirrors the production derivation exactly, using the same
`joinType` parameter. Combined with the existing `comparePlans`-based tests,
that mapping is no longer being independently asserted: if production changed
to e.g. `outer = !joinType.isInstanceOf[InnerLike]`, every existing test would
still pass, and the new `synthetic Join uses the user's joinType` test only
checks `synJoin.joinType`, not `generate.outer`. Two cheap ways to close this
-- either is fine:
- Keep `outer: Boolean` as an independent helper parameter (the pre-PR
signature) and pass it explicitly per call site; or
- Extend the new structural test to also assert `generate.outer == (joinType
== LeftOuter)` (or just `outer == false` for `Inner`, `outer == true` for
`LeftOuter`).
--
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]
