[SPARK-48529][SQL] Introduction of Labels in SQL Scripting #47146
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
miland-db
force-pushed
the
sql_batch_labels
branch
from
6968d11
to
214c685
Compare
davidm-db
reviewed
Jul 2, 2024
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
Show resolved
Hide resolved
miland-db
changed the title
# Conflicts: # sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingExecutionNode.scala # sql/core/src/main/scala/org/apache/spark/sql/scripting/SqlScriptingInterpreter.scala # sql/core/src/test/scala/org/apache/spark/sql/scripting/SqlScriptingExecutionNodeSuite.scala # sql/core/src/test/scala/org/apache/spark/sql/scripting/SqlScriptingInterpreterSuite.scala
dbatomic
reviewed
Jul 9, 2024
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
Outdated
Show resolved
Hide resolved
dbatomic
reviewed
Jul 9, 2024
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
Show resolved
Hide resolved
dbatomic
reviewed
Jul 9, 2024
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
Show resolved
Hide resolved
dbatomic
reviewed
Jul 9, 2024
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/SqlScriptingParserSuite.scala
Show resolved
Hide resolved
davidm-db
reviewed
Jul 9, 2024
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
Outdated
Show resolved
Hide resolved
davidm-db
reviewed
Jul 9, 2024
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
Outdated
Show resolved
Hide resolved
dbatomic
approved these changes
Jul 9, 2024
cloud-fan
reviewed
Jul 9, 2024
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4
Show resolved
Hide resolved
davidm-db
approved these changes
Jul 9, 2024
cloud-fan
reviewed
Jul 10, 2024
| case class CompoundBody(collection: Seq[CompoundPlanStatement]) extends CompoundPlanStatement | ||
| case class CompoundBody( | ||
| collection: Seq[CompoundPlanStatement], | ||
| label: Option[String]) extends CompoundPlanStatement |
There was a problem hiding this comment.
when can it be None? Seems we always specify an UUID if label is not given.
There was a problem hiding this comment.
In this comment I gave 2 examples when it can be None. We can also add UUID there, but I don't think it makes sense to do it because surrounding loop will have label which is logically correct, and if user adds BEGIN and END block then the label will be appointed to that block automatically because it will pass through visitBeginEndCompoundBlock(ctx: BeginEndCompoundBlockContext).
There was a problem hiding this comment.
can we add a parameter doc for it in the classdoc above? We need to explain when Spark should generate the label if not specified by users.
|
In the follow up PR that is introducing some kind of |
dbatomic
approved these changes
Jul 10, 2024
|
thanks, merging to master! |
biruktesf-db
pushed a commit
to biruktesf-db/spark
that referenced
this pull request
Jul 11, 2024
### What changes were proposed in this pull request? Previous [PR1](apache#46665) and [PR2](apache#46665) introduced parser and interpreter changes for SQL Scripting. This PR is a follow-up to introduce the concept of labels for SQL Scripting language and proposes the following changes: - Changes grammar to support labels at start and end of the compound statements. - Updates visitor functions for compound nodes in the syntax tree in AstBuilder to check if labels are present and valid. More details can be found in [Jira item](https://issues.apache.org/jira/browse/SPARK-48529) for this task and its parent (where the design doc is uploaded as well). ### Why are the changes needed? The intent is to add support for various SQL scripting concepts like loops, leave & iterate statements. ### Does this PR introduce any user-facing change? No. This PR is among first PRs in series of PRs that will introduce changes to sql() API to add support for SQL scripting, but for now, the API remains unchanged. In the future, the API will remain the same as well, but it will have new possibility to execute SQL scripts. ### How was this patch tested? There are tests for newly introduced parser changes: SqlScriptingParserSuite - unit tests for execution nodes. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#47146 from miland-db/sql_batch_labels. Lead-authored-by: David Milicevic <david.milicevic@databricks.com> Co-authored-by: Milan Dankovic <milan.dankovic@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
jingz-db
pushed a commit
to jingz-db/spark
that referenced
this pull request
Jul 22, 2024
### What changes were proposed in this pull request? Previous [PR1](apache#46665) and [PR2](apache#46665) introduced parser and interpreter changes for SQL Scripting. This PR is a follow-up to introduce the concept of labels for SQL Scripting language and proposes the following changes: - Changes grammar to support labels at start and end of the compound statements. - Updates visitor functions for compound nodes in the syntax tree in AstBuilder to check if labels are present and valid. More details can be found in [Jira item](https://issues.apache.org/jira/browse/SPARK-48529) for this task and its parent (where the design doc is uploaded as well). ### Why are the changes needed? The intent is to add support for various SQL scripting concepts like loops, leave & iterate statements. ### Does this PR introduce any user-facing change? No. This PR is among first PRs in series of PRs that will introduce changes to sql() API to add support for SQL scripting, but for now, the API remains unchanged. In the future, the API will remain the same as well, but it will have new possibility to execute SQL scripts. ### How was this patch tested? There are tests for newly introduced parser changes: SqlScriptingParserSuite - unit tests for execution nodes. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#47146 from miland-db/sql_batch_labels. Lead-authored-by: David Milicevic <david.milicevic@databricks.com> Co-authored-by: Milan Dankovic <milan.dankovic@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
attilapiros
pushed a commit
to attilapiros/spark
that referenced
this pull request
Oct 4, 2024
### What changes were proposed in this pull request? Previous [PR1](apache#46665) and [PR2](apache#46665) introduced parser and interpreter changes for SQL Scripting. This PR is a follow-up to introduce the concept of labels for SQL Scripting language and proposes the following changes: - Changes grammar to support labels at start and end of the compound statements. - Updates visitor functions for compound nodes in the syntax tree in AstBuilder to check if labels are present and valid. More details can be found in [Jira item](https://issues.apache.org/jira/browse/SPARK-48529) for this task and its parent (where the design doc is uploaded as well). ### Why are the changes needed? The intent is to add support for various SQL scripting concepts like loops, leave & iterate statements. ### Does this PR introduce any user-facing change? No. This PR is among first PRs in series of PRs that will introduce changes to sql() API to add support for SQL scripting, but for now, the API remains unchanged. In the future, the API will remain the same as well, but it will have new possibility to execute SQL scripts. ### How was this patch tested? There are tests for newly introduced parser changes: SqlScriptingParserSuite - unit tests for execution nodes. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#47146 from miland-db/sql_batch_labels. Lead-authored-by: David Milicevic <david.milicevic@databricks.com> Co-authored-by: Milan Dankovic <milan.dankovic@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Previous PR1 and PR2 introduced parser and interpreter changes for SQL Scripting. This PR is a follow-up to introduce the concept of labels for SQL Scripting language and proposes the following changes:
More details can be found in Jira item for this task and its parent (where the design doc is uploaded as well).
Why are the changes needed?
The intent is to add support for various SQL scripting concepts like loops, leave & iterate statements.
Does this PR introduce any user-facing change?
No.
This PR is among first PRs in series of PRs that will introduce changes to sql() API to add support for SQL scripting, but for now, the API remains unchanged.
In the future, the API will remain the same as well, but it will have new possibility to execute SQL scripts.
How was this patch tested?
There are tests for newly introduced parser changes:
SqlScriptingParserSuite - unit tests for execution nodes.
Was this patch authored or co-authored using generative AI tooling?
No.