[KYUUBI #6577] Add spark sql engine plugin module and define a sql stringify plugin #6578

wForget · 2024-08-02T03:49:39Z

🔍 Description

Issue References 🔗

This pull request fixes #6577

Describe Your Solution 🔧

Define a PlanOnlyExecutor interface to extend plan only mode of spark engine using SPI

Types of changes 🔖

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklist 📝

This patch was not authored or co-authored using Generative Tooling

Be nice. Be informative.

…an only mode of spark engine using SPI

wForget · 2024-08-02T03:58:15Z

cc @pan3793 @iodone @yaooqinn Could you take a quick look at this? If this is feasible, I will try to replace the current lineage plan only mode implementation.

codecov-commenter · 2024-08-02T04:56:14Z

Codecov Report

Attention: Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (c94f0d7) to head (5f5494e).
Report is 60 commits behind head on master.

Files with missing lines	Patch %	Lines
...ubi/engine/spark/operation/PlanOnlyStatement.scala	0.00%	7 Missing ⚠️
...spark/operation/planonly/SQLStringifyPlugins.scala	0.00%	6 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #6578    +/-   ##
=======================================
  Coverage    0.00%   0.00%            
=======================================
  Files         677     684     +7     
  Lines       41806   42222   +416     
  Branches     5711    5758    +47     
=======================================
- Misses      41806   42222   +416

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cxzl25 · 2024-08-12T10:10:39Z

...park-engine-plugin/src/main/java/org/apache/kyuubi/engine/spark/plugin/PlanOnlyExecutor.java

+import org.apache.spark.sql.SparkSession;
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan;
+
+public interface PlanOnlyExecutor {


Just a curious question, what functions can be used to extend this SPI?

I hope to only expand the functionality of explaining the execution plan.

extensions/spark/kyuubi-spark-engine-plugin/pom.xml

yaooqinn · 2024-08-16T03:18:11Z

...park-engine-plugin/src/main/java/org/apache/kyuubi/engine/spark/plugin/PlanOnlyExecutor.java

+
+  String mode();
+
+  default String execute(SparkPlans plans) {


where is this called?

in PlanOnlyStatement

yaooqinn · 2024-08-16T03:46:18Z

...-sql-engine-plugin/src/main/java/org/apache/kyuubi/engine/spark/plugin/PlanOnlyExecutor.java

-    LogicalPlan optimizedPlan();
-    SparkPlan sparkPlan();
-    SparkPlan executedPlan();
+  String mode();


can we move the mode to execute?

We will use SPI to load plugins, so we need to define a qualified name for the plugin to distinguish them.

yaooqinn · 2024-08-16T03:49:00Z

...-sql-engine-plugin/src/main/java/org/apache/kyuubi/engine/spark/plugin/PlanOnlyExecutor.java


-public interface SparkPlans {
+public interface PlanOnlyExecutor {


How about SQLStringifyPlugin? Since output does not have to be a plan

yaooqinn · 2024-08-16T03:49:22Z

...-sql-engine-plugin/src/main/java/org/apache/kyuubi/engine/spark/plugin/PlanOnlyExecutor.java


-public interface SparkPlans {
+public interface PlanOnlyExecutor {


Can we add a developer guide?

yaooqinn · 2024-08-16T03:49:51Z

...-sql-engine-plugin/src/main/java/org/apache/kyuubi/engine/spark/plugin/PlanOnlyExecutor.java


+  String execute(SparkSession spark, String statement);


maybe toString

yaooqinn · 2024-08-16T07:21:00Z

...k-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/PlanOnlyStatement.scala

@@ -28,6 +28,7 @@ import org.apache.spark.sql.types.StructType
 import org.apache.kyuubi.KyuubiSQLException
 import org.apache.kyuubi.config.KyuubiConf.{LINEAGE_PARSER_PLUGIN_PROVIDER, OPERATION_PLAN_ONLY_EXCLUDES, OPERATION_PLAN_ONLY_OUT_STYLE}
 import org.apache.kyuubi.engine.spark.KyuubiSparkUtil.getSessionConf
+import org.apache.kyuubi.engine.spark.operation.planonly.SQLStringifyPlugins


So the plugin is actually a must for spark sql engine？ why do we need to separate it from the engine module？

So the plugin is actually a must for spark sql engine？ why do we need to separate it from the engine module？

Because I don't want to introduce kyuubi engine dependency in kyuubi spark extensions

we have to bundle this when packaging

Yes, the kyuubi server plugin also works this way

This plugin will be lightweight enough to only retain some interface definitions

yaooqinn

LGTM, except for #6578 (comment)

wForget · 2024-08-16T08:39:46Z

@yaooqinn Thanks for your review, I will add developer guide later.

[KYUUBI apache#6577] Define a PlanOnlyExecutor interface to extend pl…

c3ce8dd

…an only mode of spark engine using SPI

github-actions bot added module:spark kind:build module:extensions labels Aug 2, 2024

newline

a4f9dd3

wForget force-pushed the KYUUBI-6577 branch from 3d50bc1 to a4f9dd3 Compare August 2, 2024 03:51

license

24f1f79

wForget self-assigned this Aug 2, 2024

fix style

5129317

cxzl25 reviewed Aug 12, 2024

View reviewed changes

narrow interface definition

8d65c7e

yaooqinn reviewed Aug 16, 2024

View reviewed changes

extensions/spark/kyuubi-spark-engine-plugin/pom.xml Outdated Show resolved Hide resolved

yaooqinn reviewed Aug 16, 2024

View reviewed changes

refactor

870a9b4

yaooqinn reviewed Aug 16, 2024

View reviewed changes

address comments

4aca8ea

yaooqinn reviewed Aug 16, 2024

View reviewed changes

yaooqinn approved these changes Aug 16, 2024

View reviewed changes

add readme

f8fdafe

wForget changed the title ~~[KYUUBI #6577] Define a PlanOnlyExecutor interface to extend plan only mode of spark engine using SPI~~ [KYUUBI #6577] Add spark sql engine plugin module and define a sql stringify plugin Aug 16, 2024

wForget marked this pull request as ready for review August 16, 2024 12:50

fix

567e185

yaooqinn approved these changes Sep 5, 2024

View reviewed changes

fix readme

5f5494e

wForget added this to the v1.10.0 milestone Sep 6, 2024

bowenliang123 modified the milestones: v1.10.0, v1.11.0 Oct 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #6577] Add spark sql engine plugin module and define a sql stringify plugin #6578

[KYUUBI #6577] Add spark sql engine plugin module and define a sql stringify plugin #6578

wForget commented Aug 2, 2024

wForget commented Aug 2, 2024

codecov-commenter commented Aug 2, 2024 •

edited

Loading

cxzl25 Aug 12, 2024

wForget Aug 12, 2024

yaooqinn Aug 16, 2024

wForget Aug 16, 2024

yaooqinn Aug 16, 2024

wForget Aug 16, 2024

yaooqinn Aug 16, 2024

yaooqinn Aug 16, 2024

yaooqinn Aug 16, 2024

yaooqinn Aug 16, 2024

wForget Aug 16, 2024

yaooqinn Aug 16, 2024

wForget Aug 16, 2024

wForget Aug 16, 2024

yaooqinn Aug 16, 2024

yaooqinn left a comment

wForget commented Aug 16, 2024


		public interface SparkPlans {
		public interface PlanOnlyExecutor {

[KYUUBI #6577] Add spark sql engine plugin module and define a sql stringify plugin #6578

Are you sure you want to change the base?

[KYUUBI #6577] Add spark sql engine plugin module and define a sql stringify plugin #6578

Conversation

wForget commented Aug 2, 2024

🔍 Description

Issue References 🔗

Describe Your Solution 🔧

Types of changes 🔖

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests

Checklist 📝

wForget commented Aug 2, 2024

codecov-commenter commented Aug 2, 2024 • edited Loading

Codecov Report

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaooqinn left a comment

Choose a reason for hiding this comment

wForget commented Aug 16, 2024

codecov-commenter commented Aug 2, 2024 •

edited

Loading