You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "mihailoale-db (via GitHub)" <gi...@apache.org> on 2026/10/12 12:51:21 UTC
[PR] [SPARK-60193] [SQL] Single-pass resolver code health improvements [spark]
mihailoale-db opened a new pull request, #59399:
URL: https://github.com/apache/spark/pull/59399
### What changes were proposed in this pull request?
In this PR I propose to introduce some single-pass resolver code health improvements.
### Why are the changes needed?
To improve the code health of the single-pass resolver.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing tests.
### Was this patch authored or co-authored using generative AI tooling?
Yes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193][SQL] Single-pass resolver code health improvements [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b closed pull request #59399: [SPARK-60193][SQL] Single-pass resolver code health improvements
URL: https://github.com/apache/spark/pull/59399
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193] [SQL] Single-pass resolver code health improvements [spark]
Posted by "mihailoale-db (via GitHub)" <gi...@apache.org>.
mihailoale-db commented on PR #59399:
URL: https://github.com/apache/spark/pull/59399#issuecomment-6143415447
@uros-b Could you PTAL when you find time? Thanks!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193] [SQL] Single-pass resolver code health improvements [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #59399:
URL: https://github.com/apache/spark/pull/59399#discussion_r4257593202
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/SubqueryScope.scala:
##########
@@ -74,8 +74,7 @@ import org.apache.spark.sql.catalyst.plans.logical.{Aggregate, Filter, LogicalPl
* which indicates how many subquery levels we need to go up to find the attribute reference.
* We need the outerScopeLevel map when resolving the subquery expressions, where we collect
* the outer references and wrap the ones cannot be resolved in the current subquery scope or
- * its parent scopes with [[OuterScopeReference]]. More details can be found in
- * `SubqueryExpressionResolver.processOuterReferences`.
+ * its parent scopes with [[OuterScopeReference]].
Review Comment:
This still says unresolved outer refs are wrapped in OuterScopeReference. processOuterReferences is already just SubExprUtils.getOuterReferences. You cleaned the sibling comment and left this one.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/SubqueryExpressionResolver.scala:
##########
@@ -201,58 +201,8 @@ class SubqueryExpressionResolver(expressionResolver: ExpressionResolver, resolve
}
/**
- * Return `outerAttrs` for the resolved subquery plan.
- * If `spark.sql.optimizer.supportNestedCorrelatedSubqueries.enabled` is set to false, simply
- * collect the outer references within the subquery plan.
- *
- * Otherwise,
- * 1. Get the outer references within the resolved subquery plan and also get the outer
- * attributes from any nested subqueries.
- * 2. For each outer reference, using the `outerReferenceScopeLevel` map from current subquery
- * scope to check if it has only one level of correlation or beyond one level of correlation.
- * If it is beyond one level of correlation, wrap it with [[OuterScopeReference]] and add it to
- * the parent subquery scope's `outerReferenceScopeLevel` map with level - 1. E.g: For the
- * following query:
- *
- * {{{
- * SELECT *
- * FROM t1
- * WHERE t1.a = (
- * SELECT MAX(t2.a)
- * FROM t2
- * WHERE t2.a = (
- * SELECT MAX(t3.a)
- * FROM t3
- * WHERE t3.b > t2.b AND t3.c > t1.c
- * ) AND t2.b > t1.b
- * }}}
- *
- * After resolving:
- *
- * {{{
- * Aggregate (MAX(t3.a))
- * Filter (t3.b > OuterReference(t2.b) AND t3.c > OuterReference(t1.c))
- * Scan t3
- * }}}
- *
- * The innermost subquery has two outer references: `t2.b(outerScopeReferenceLevel = 1)` and
- * `t1.c(outerScopeReferenceLevel = 2)`. The innermost subquery expression is resolved as
- * `scalar-subquery(outerAttrs = [t2.b, OuterScopeReference(t1.c)])` and
- * `(t1.c.exprId -> outerScopeReferenceLevel = 1)` is added to the `outerReferenceScopeLevel`
- * map of the parent subquery scope.
- * After resolving the parent subquery plan:
- *
- * {{{
- * Aggregate (MAX(t2.a))
- * Filter (t2.a = scalar-subquery(outerAttrs = [t2.b, OuterScopeReference(t1.c)]) AND
- * t2.b > OuterReference(t1.b))
- * Scan t2
- * }}}
- *
- * The collected outer references are `t1.b` and the `OuterScopeReference(t1.c)` from the
- * innermost subquery. Both of them have `outerScopeReferenceLevel = 1`, they are all resolved
- * as one level outer references. The subquery expression is then resolved as
- * `scalar-subquery(outerAttrs = [t1.b, t1.c])`.
+ * Return `outerAttrs` for the resolved subquery plan by collecting the outer references within
+ * the subquery plan. Only one level of correlation is supported.
*/
private def processOuterReferences(
resolvedPlan: LogicalPlan,
Review Comment:
Is parentSubqueryScope used? Can we remove it?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193][SQL] Single-pass resolver code health improvements [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #59399:
URL: https://github.com/apache/spark/pull/59399#issuecomment-6155925420
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/2dad66372a4ee946a96e1249ec1e2cb79342f35b
- merged into branch-4.x https://github.com/apache/spark/commit/09f113d6252b3222f1c0e2896f39af1c9a53991c
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193][SQL] Single-pass resolver code health improvements [spark]
Posted by "mihailoale-db (via GitHub)" <gi...@apache.org>.
mihailoale-db commented on code in PR #59399:
URL: https://github.com/apache/spark/pull/59399#discussion_r4258551296
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/SubqueryExpressionResolver.scala:
##########
@@ -201,58 +201,8 @@ class SubqueryExpressionResolver(expressionResolver: ExpressionResolver, resolve
}
/**
- * Return `outerAttrs` for the resolved subquery plan.
- * If `spark.sql.optimizer.supportNestedCorrelatedSubqueries.enabled` is set to false, simply
- * collect the outer references within the subquery plan.
- *
- * Otherwise,
- * 1. Get the outer references within the resolved subquery plan and also get the outer
- * attributes from any nested subqueries.
- * 2. For each outer reference, using the `outerReferenceScopeLevel` map from current subquery
- * scope to check if it has only one level of correlation or beyond one level of correlation.
- * If it is beyond one level of correlation, wrap it with [[OuterScopeReference]] and add it to
- * the parent subquery scope's `outerReferenceScopeLevel` map with level - 1. E.g: For the
- * following query:
- *
- * {{{
- * SELECT *
- * FROM t1
- * WHERE t1.a = (
- * SELECT MAX(t2.a)
- * FROM t2
- * WHERE t2.a = (
- * SELECT MAX(t3.a)
- * FROM t3
- * WHERE t3.b > t2.b AND t3.c > t1.c
- * ) AND t2.b > t1.b
- * }}}
- *
- * After resolving:
- *
- * {{{
- * Aggregate (MAX(t3.a))
- * Filter (t3.b > OuterReference(t2.b) AND t3.c > OuterReference(t1.c))
- * Scan t3
- * }}}
- *
- * The innermost subquery has two outer references: `t2.b(outerScopeReferenceLevel = 1)` and
- * `t1.c(outerScopeReferenceLevel = 2)`. The innermost subquery expression is resolved as
- * `scalar-subquery(outerAttrs = [t2.b, OuterScopeReference(t1.c)])` and
- * `(t1.c.exprId -> outerScopeReferenceLevel = 1)` is added to the `outerReferenceScopeLevel`
- * map of the parent subquery scope.
- * After resolving the parent subquery plan:
- *
- * {{{
- * Aggregate (MAX(t2.a))
- * Filter (t2.a = scalar-subquery(outerAttrs = [t2.b, OuterScopeReference(t1.c)]) AND
- * t2.b > OuterReference(t1.b))
- * Scan t2
- * }}}
- *
- * The collected outer references are `t1.b` and the `OuterScopeReference(t1.c)` from the
- * innermost subquery. Both of them have `outerScopeReferenceLevel = 1`, they are all resolved
- * as one level outer references. The subquery expression is then resolved as
- * `scalar-subquery(outerAttrs = [t1.b, t1.c])`.
+ * Return `outerAttrs` for the resolved subquery plan by collecting the outer references within
+ * the subquery plan. Only one level of correlation is supported.
*/
private def processOuterReferences(
resolvedPlan: LogicalPlan,
Review Comment:
Done!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193][SQL] Single-pass resolver code health improvements [spark]
Posted by "mihailoale-db (via GitHub)" <gi...@apache.org>.
mihailoale-db commented on code in PR #59399:
URL: https://github.com/apache/spark/pull/59399#discussion_r4258559606
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ResolverGuard.scala:
##########
@@ -762,8 +755,6 @@ object ResolverGuard {
private val UNSUPPORTED_FUNCTION_NAMES = {
val map = new IdentifierMap[Unit]()
// User info functions are not supported.
- // [[InlineUserInfoExpressions]] cannot be invoked as a post-hoc rule as it produces
- // inconsistent aliases based on the table type (because of the rule ordering in fixed-point).
map += ("current_user", ())
map += ("session_user", ())
Review Comment:
Thanks for the suggestion, done!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193][SQL] Single-pass resolver code health improvements [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #59399:
URL: https://github.com/apache/spark/pull/59399#issuecomment-6155917796
Thank you @mihailoale-db!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193][SQL] Single-pass resolver code health improvements [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #59399:
URL: https://github.com/apache/spark/pull/59399#issuecomment-6143738901
I fixed the PR title just a bit, PTAL at a few nit comments above, and please try to make the CI green
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-60193][SQL] Single-pass resolver code health improvements [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #59399:
URL: https://github.com/apache/spark/pull/59399#discussion_r4258084585
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ResolverGuard.scala:
##########
@@ -762,8 +755,6 @@ object ResolverGuard {
private val UNSUPPORTED_FUNCTION_NAMES = {
val map = new IdentifierMap[Unit]()
// User info functions are not supported.
- // [[InlineUserInfoExpressions]] cannot be invoked as a post-hoc rule as it produces
- // inconsistent aliases based on the table type (because of the rule ordering in fixed-point).
map += ("current_user", ())
map += ("session_user", ())
Review Comment:
Is there a missed leftover below (783-784)?
The `HIGHER_ORDER_FUNCTIONS` scaladoc says these functions are "supported but guarded under the `ANALYZER_SINGLE_PASS_RESOLVER_ENABLE_HIGHER_ORDER_FUNCTIONS_RESOLUTION` flag". That flag is not in SQLConf, and `isUnsupportedFunction` (681-685) rejects every HOF unconditionally. Suggested text: "Higher order functions that are not yet supported by the single-pass resolver."
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org