vranes commented on code in PR #59267:
URL: https://github.com/apache/spark/pull/59267#discussion_r4206208667
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/RetainsOriginalJoinOutput.scala:
##########
@@ -112,7 +112,9 @@ trait RetainsOriginalJoinOutput {
* [[Project]] on top of the outer [[Join]] even though its output has
changed.
*
* The above example also holds true for cases when there is a [[Project]],
[[Aggregate]] or
- * [[Filter]] node on top of a [[Join]].
+ * [[Filter]] node on top of a [[Join]]. An [[AsOfJoin]] is handled the same
way, because
Review Comment:
nit: Simplify this comment addition.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveAsOfJoin.scala:
##########
@@ -74,10 +74,11 @@ object ResolveAsOfJoin extends Rule[LogicalPlan] with
SQLConfHelper {
}
val resolvedJoin = (matchLeft, matchOp, matchRight) match {
case (Some(leftExpr), Some(operator), Some(rightExpr)) =>
+ // Include hidden columns, such as the dropped key of a nested USING
join.
Review Comment:
nit: Unnecessary comment
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveAsOfJoin.scala:
##########
@@ -74,10 +74,11 @@ object ResolveAsOfJoin extends Rule[LogicalPlan] with
SQLConfHelper {
}
val resolvedJoin = (matchLeft, matchOp, matchRight) match {
case (Some(leftExpr), Some(operator), Some(rightExpr)) =>
+ // Include hidden columns, such as the dropped key of a nested USING
join.
Review Comment:
The one in
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/AsOfJoinResolver.scala
too
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/RetainsOriginalJoinOutput.scala:
##########
@@ -127,10 +129,30 @@ trait RetainsOriginalJoinOutput {
referencedAttributes = childReferencedAttributes
) =>
Project(scopes.current.output, join)
+ case asOfJoin: AsOfJoin
+ if childHasMissingAttributesNotInOutput(
+ scopes = scopes,
+ outputExpressions = outputExpressions,
+ referencedAttributes = referencedAttributesOf(asOfJoin)
+ ) =>
+ Project(scopes.current.output, asOfJoin)
Review Comment:
LLM finding:
> Parent Expressions Lose The Hidden Key
>
> The new Project in RetainsOriginalJoinOutput.scala:138 restores only
visible output. Its guard checks top-level output ExprIds, not references
inside expressions (RetainsOriginalJoinOutput.scala:164).
>
> Consequently, selecting d.k AS dk or sum(d.k) does not protect the
underlying d.k. The parent still references it, but the inserted projection
removes it. Strict single-pass analysis then encounters missing input
(ProjectResolver.scala:76, Resolver.scala:810).
>
> Missing regression: take the PR’s example and change the SELECT list to
d.k AS dk. Also test sum(d.k).
>
> Required fix: preserve hidden keys needed by parent expressions, including
aggregate/grouping and filter references, while retaining the intended
output-schema behavior.
Since this is a preexisting issue for JOIN in single-pass analyzer, IMO it
can be fixed in a follow-up. I created a JIRA
https://issues.apache.org/jira/browse/SPARK-60035
--
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]
