You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "vladimirg-db (via GitHub)" <gi...@apache.org> on 2025/03/26 16:38:59 UTC

[PR] [WIP][SPARK-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

vladimirg-db opened a new pull request, #50406:
URL: https://github.com/apache/spark/pull/50406

   ### What changes were proposed in this pull request?
   
   New single-pass Analyzer features:
   - GROUP BY
   - ORDER BY
   - JOIN
   - Correlated subqueries
   - Other small features and bugfixes
   
   ### Why are the changes needed?
   
   To replace the fixed-point Analyzer with the single-pass one.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No, single-pass Analyzer is not yet enabled by default.
   
   ### How was this patch tested?
   
   CI with `ANALYZER_DUAL_RUN_LEGACY_AND_SINGLE_PASS_RESOLVER`.
   
   ### 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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "iwanttobepowerful (via GitHub)" <gi...@apache.org>.
iwanttobepowerful commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2804008495

   @vladimirg-db hi, does new Analyzer's principle similar to calcite's validator?


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "vladimirg-db (via GitHub)" <gi...@apache.org>.
vladimirg-db commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2804077538

   Hello @iwanttobepowerful. I never worked with Calcite.
   
   The new Analyzer processes the logical plan in one* bottom-up traversal. For example, expression IDs get propagated bottom-up ([ExpressionIdAssigner](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ExpressionIdAssigner.scala)) or subqueries can find [outer references](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/NameScope.scala#L864) from already resolved subtrees. That means that sometimes we have to build additional data-structures as we descend to leaf nodes before starting the bottom-up analysis, e.g. for [CTE resolution](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/CteScope.scala) we build a stack of lookup scopes.
   
   * We have one pass for the main algebra (90% of the Analysis), but there is also one pre-pass to [resolve the metadata](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/MetadataResolver.scala) (blocking calls), and a couple of rules running after the Analysis before the Optimizer to stay compatible with the old Analyzer in [PlanRewriter](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/PlanRewriter.scala) (e.g. `CleanupAliases`).


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #50406: [SPARK-51611][SQL] New iteration of single-pass Analyzer functionality
URL: https://github.com/apache/spark/pull/50406


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2786657366

   thanks, merging to master!


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "vladimirg-db (via GitHub)" <gi...@apache.org>.
vladimirg-db commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2783938841

   Thanks @mihailom-db for EXCEPT/INTERSECT!


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "iwanttobepowerful (via GitHub)" <gi...@apache.org>.
iwanttobepowerful commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2804214109

   > Hello @iwanttobepowerful. I never worked with Calcite.
   > 
   > The new Analyzer processes the logical plan in one* bottom-up traversal. For example, expression IDs get propagated bottom-up ([ExpressionIdAssigner](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ExpressionIdAssigner.scala?rgh-link-date=2025-04-15T07%3A21%3A46Z)) or subqueries can find [outer references](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/NameScope.scala?rgh-link-date=2025-04-15T07%3A21%3A46Z#L864) from already resolved subtrees. That means that sometimes we have to build additional data-structures as we descend to leaf nodes before starting the bottom-up analysis, e.g. for [CTE resolution](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/CteScope.scala?rgh-link-date=2025-04-15T07%3A21%3A46Z) we build a stack of lookup scopes.
   > 
   > * We have one pass for the main algebra (95% of the Analysis), but there is also one pre-pass to [resolve the metadata](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/MetadataResolver.scala?rgh-link-date=2025-04-15T07%3A21%3A46Z) (blocking calls), and a couple of rules running after the Analysis before the Optimizer to stay compatible with the old Analyzer in [PlanRewriter](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/PlanRewriter.scala?rgh-link-date=2025-04-15T07%3A21%3A46Z) (e.g. `CleanupAliases`).
   
   thanks!!! very nice


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "vladimirg-db (via GitHub)" <gi...@apache.org>.
vladimirg-db commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2785753645

   Yeah, sorry... let me fix that real quick...


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "vladimirg-db (via GitHub)" <gi...@apache.org>.
vladimirg-db commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2755601257

   Thanks @mihailoale-db and @mihailotim-db for this work!


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "vladimirg-db (via GitHub)" <gi...@apache.org>.
vladimirg-db commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2786159709

   @cloud-fan tests passed!


-- 
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-51611][SQL] New iteration of single-pass Analyzer functionality [spark]

Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #50406:
URL: https://github.com/apache/spark/pull/50406#issuecomment-2785718607

   The test failures seem to be valid


-- 
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