[SPARK-48782][SQL] Add support for executing procedures in catalogs #47943

aokolnychyi · 2024-08-30T22:38:22Z

What changes were proposed in this pull request?

This PR adds support for executing procedures in catalogs.

Why are the changes needed?

These changes are needed per discussed and voted SPIP tracked in SPARK-44167.

Does this PR introduce any user-facing change?

Yes. This PR adds CALL commands.

How was this patch tested?

This PR comes with tests.

Was this patch authored or co-authored using generative AI tooling?

No.

aokolnychyi · 2024-08-30T22:43:19Z

sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala

@@ -92,7 +93,38 @@ class QueryExecution(
      sparkSession.sessionState.analyzer.executeAndCheck(logical, tracker)
    }
    tracker.setAnalyzed(plan)
-    plan
+
+    mode match {


This part will have to evolve. There are 3 options for us to consider.

Add a special type of commands that must be executed during the analysis (i.e. generalize this PR).

Add a special mix-in interface for procedures that know the type of the last result set before the execution. All other procedures will not output anything if invoked via spark.sql.

Migrate to qe.commandExecuted instead of qe.analyzed everywhere as we will know the output only after executing the procedure.

@cloud-fan, any thoughts?

I'd prefer a special mix-in interface that indicates the command returns multiple result sets. It should have a method to return multiple LogicalPlan/DataFrame. SparkSession#sql will recognize this special interface and get the last DataFrame.

@cloud-fan, do you mean a special mix-in connector interface or logical plan interface?
When will the execution happen?

cloud-fan · 2024-09-02T12:54:22Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala

+/**
+ * The logical plan for the CALL command.
+ */
+case class Call(procedure: LogicalPlan, args: Seq[Expression]) extends UnaryCommand {


shall we extend UnaryRunnableCommand to avoid creating a corresponding physical node?

dongjoon-hyun · 2024-09-11T15:07:28Z

Gentle ping, @aokolnychyi .

Although we still have time for feature freeze, I'm wondering if you want to deliver this via Apache Spark 4.0.0-preview2 RC1 (next Monday).

aokolnychyi · 2024-09-13T00:06:54Z

Will update tomorrow. Thanks for pinging, @dongjoon-hyun!

dongjoon-hyun · 2024-09-13T00:07:39Z

Thank you, @aokolnychyi .

aokolnychyi · 2024-09-18T01:54:59Z

sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4

@@ -298,6 +298,10 @@ statement
        LEFT_PAREN columns=multipartIdentifierPropertyList RIGHT_PAREN
        (OPTIONS options=propertyList)?                                #createIndex
    | DROP INDEX (IF EXISTS)? identifier ON TABLE? identifierReference #dropIndex
+    | CALL identifierReference


I can split this into a separate PR, if needed.

cloud-fan · 2024-09-18T08:05:05Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

+
+    private def argMetadata(byName: Boolean): Metadata = {
+      new MetadataBuilder()
+        .putBoolean(ProcedureParameter.BY_NAME_METADATA_KEY, byName)


shall we omit this metadata if the arg is not by name?

We can, skipped.

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/InvokeProcedures.scala

cloud-fan · 2024-09-18T09:07:09Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/CallExec.scala

+/**
+ * The physical plan of the CALL statement used in EXPLAIN.
+ */
+case class CallExec(


How about this

case class ExplainOnlySparkPlan(toExplain: LogicalPlan)... { def simpleString = toExplain.simpleString }

I like that, updated.

cloud-fan · 2024-09-19T09:09:20Z

thanks, merging to master!

aokolnychyi · 2024-09-19T16:20:20Z

Thanks, @cloud-fan @dongjoon-hyun!

dongjoon-hyun · 2024-09-19T16:22:48Z

Nice. Thank you, @aokolnychyi and @cloud-fan .

allisonwang-db

This is awesome! Late LGTM

allisonwang-db · 2024-09-20T21:58:42Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/MultiResult.scala

+case class MultiResult(children: Seq[LogicalPlan]) extends LogicalPlan {
+
+  override def output: Seq[Attribute] = children.lastOption.map(_.output).getOrElse(Nil)


Can we add some comments for this class and the output here (which uses the last result set's schema)

allisonwang-db · 2024-09-20T22:08:29Z

sql/core/src/main/scala/org/apache/spark/sql/execution/MultiResultExec.scala

+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.Attribute
+
+case class MultiResultExec(children: Seq[SparkPlan]) extends SparkPlan {


ditto for docstring

Will follow up with some docs.

allisonwang-db · 2024-09-20T22:13:05Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

+    }
+
+   private def validateParameterModes(procedure: BoundProcedure): Unit = {
+     procedure.parameters.find(_.mode != ProcedureParameter.Mode.IN).foreach { param =>


Are we planning to support more parameter modes?

In the future, yes. There is no active work at the moment, as far as I know.

…test in ProcedureSuite ### What changes were proposed in this pull request? This PR is a followup of #47943 that enables ANSI for malformed input test in ProcedureSuite. ### Why are the changes needed? The specific test fails with ANSI mode disabled https://github.com/apache/spark/actions/runs/10951615244/job/30408963913 ``` - malformed input to implicit cast *** FAILED *** (4 milliseconds) Expected exception org.apache.spark.SparkNumberFormatException to be thrown, but no exception was thrown (ProcedureSuite.scala:264) org.scalatest.exceptions.TestFailedException: at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472) at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471) at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1564) ... ``` The test depends on `sum`'s failure so this PR simply enables ANSI mode for that specific test. ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? Manually ran with ANSI mode off. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #48193 from HyukjinKwon/SPARK-48782-followup. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Max Gekk <max.gekk@gmail.com>

### What changes were proposed in this pull request? This PR adds support for executing procedures in catalogs. ### Why are the changes needed? These changes are needed per [discussed and voted](https://lists.apache.org/thread/w586jr53fxwk4pt9m94b413xyjr1v25m) SPIP tracked in [SPARK-44167](https://issues.apache.org/jira/browse/SPARK-44167). ### Does this PR introduce _any_ user-facing change? Yes. This PR adds CALL commands. ### How was this patch tested? This PR comes with tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#47943 from aokolnychyi/spark-48782. Authored-by: Anton Okolnychyi <aokolnychyi@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…test in ProcedureSuite ### What changes were proposed in this pull request? This PR is a followup of apache#47943 that enables ANSI for malformed input test in ProcedureSuite. ### Why are the changes needed? The specific test fails with ANSI mode disabled https://github.com/apache/spark/actions/runs/10951615244/job/30408963913 ``` - malformed input to implicit cast *** FAILED *** (4 milliseconds) Expected exception org.apache.spark.SparkNumberFormatException to be thrown, but no exception was thrown (ProcedureSuite.scala:264) org.scalatest.exceptions.TestFailedException: at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472) at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471) at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1564) ... ``` The test depends on `sum`'s failure so this PR simply enables ANSI mode for that specific test. ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? Manually ran with ANSI mode off. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#48193 from HyukjinKwon/SPARK-48782-followup. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Max Gekk <max.gekk@gmail.com>

### What changes were proposed in this pull request? This PR adds support for executing procedures in catalogs. ### Why are the changes needed? These changes are needed per [discussed and voted](https://lists.apache.org/thread/w586jr53fxwk4pt9m94b413xyjr1v25m) SPIP tracked in [SPARK-44167](https://issues.apache.org/jira/browse/SPARK-44167). ### Does this PR introduce _any_ user-facing change? Yes. This PR adds CALL commands. ### How was this patch tested? This PR comes with tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#47943 from aokolnychyi/spark-48782. Authored-by: Anton Okolnychyi <aokolnychyi@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…test in ProcedureSuite ### What changes were proposed in this pull request? This PR is a followup of apache#47943 that enables ANSI for malformed input test in ProcedureSuite. ### Why are the changes needed? The specific test fails with ANSI mode disabled https://github.com/apache/spark/actions/runs/10951615244/job/30408963913 ``` - malformed input to implicit cast *** FAILED *** (4 milliseconds) Expected exception org.apache.spark.SparkNumberFormatException to be thrown, but no exception was thrown (ProcedureSuite.scala:264) org.scalatest.exceptions.TestFailedException: at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472) at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471) at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1564) ... ``` The test depends on `sum`'s failure so this PR simply enables ANSI mode for that specific test. ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? Manually ran with ANSI mode off. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#48193 from HyukjinKwon/SPARK-48782-followup. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Max Gekk <max.gekk@gmail.com>

github-actions bot added SQL DOCS labels Aug 30, 2024

aokolnychyi commented Aug 30, 2024

View reviewed changes

cloud-fan reviewed Sep 2, 2024

View reviewed changes

[SPARK-48782][SQL] Add support for procedures in catalogs

d449057

aokolnychyi force-pushed the spark-48782 branch from 8905e1e to d449057 Compare September 17, 2024 17:33

Attempt to fix EXPLAIN

3cb76c4

aokolnychyi commented Sep 18, 2024

View reviewed changes

cloud-fan reviewed Sep 18, 2024

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala Show resolved Hide resolved

cloud-fan reviewed Sep 18, 2024

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/InvokeProcedures.scala Show resolved Hide resolved

cloud-fan reviewed Sep 18, 2024

View reviewed changes

aokolnychyi added 2 commits September 18, 2024 13:57

Feedback

1e36de6

Minor update

fb820f8

cloud-fan approved these changes Sep 19, 2024

View reviewed changes

cloud-fan closed this in 492d1b1 Sep 19, 2024

allisonwang-db reviewed Sep 20, 2024

View reviewed changes

HyukjinKwon mentioned this pull request Sep 21, 2024

[SPARK-48782][SQL][TESTS][FOLLOW-UP] Enable ANSI for malformed input test in ProcedureSuite #48193

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-48782][SQL] Add support for executing procedures in catalogs #47943

[SPARK-48782][SQL] Add support for executing procedures in catalogs #47943

aokolnychyi commented Aug 30, 2024

aokolnychyi Aug 30, 2024 •

edited

Loading

aokolnychyi Aug 30, 2024

cloud-fan Sep 2, 2024 •

edited

Loading

aokolnychyi Sep 13, 2024 •

edited

Loading

cloud-fan Sep 2, 2024

dongjoon-hyun commented Sep 11, 2024

aokolnychyi commented Sep 13, 2024

dongjoon-hyun commented Sep 13, 2024

aokolnychyi Sep 18, 2024

cloud-fan Sep 18, 2024

aokolnychyi Sep 18, 2024

cloud-fan Sep 18, 2024

aokolnychyi Sep 18, 2024

cloud-fan commented Sep 19, 2024

aokolnychyi commented Sep 19, 2024

dongjoon-hyun commented Sep 19, 2024

allisonwang-db left a comment

allisonwang-db Sep 20, 2024

allisonwang-db Sep 20, 2024

aokolnychyi Sep 26, 2024

allisonwang-db Sep 20, 2024

aokolnychyi Sep 26, 2024

		case class MultiResult(children: Seq[LogicalPlan]) extends LogicalPlan {

		override def output: Seq[Attribute] = children.lastOption.map(_.output).getOrElse(Nil)

[SPARK-48782][SQL] Add support for executing procedures in catalogs #47943

[SPARK-48782][SQL] Add support for executing procedures in catalogs #47943

Conversation

aokolnychyi commented Aug 30, 2024

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

aokolnychyi Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan Sep 2, 2024 • edited Loading

Choose a reason for hiding this comment

aokolnychyi Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun commented Sep 11, 2024

aokolnychyi commented Sep 13, 2024

dongjoon-hyun commented Sep 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan commented Sep 19, 2024

aokolnychyi commented Sep 19, 2024

dongjoon-hyun commented Sep 19, 2024

allisonwang-db left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aokolnychyi Aug 30, 2024 •

edited

Loading

cloud-fan Sep 2, 2024 •

edited

Loading

aokolnychyi Sep 13, 2024 •

edited

Loading