[#3554] feat(spark-connector): support spark multi Version #3415

FANNG1 · 2024-05-16T08:06:22Z

What changes were proposed in this pull request?

split spark connector to spark common which contains common logic and v3.x which contains adaptor logic
add separate GitHub action to do spark IT
./gradlew :spark-connector:spark35-runtime:build to build corresponding spark connector jars

Why are the changes needed?

Fix: #3554

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests with corresponding spark version

spark-connector/spark3.3/build.gradle.kts

FANNG1 · 2024-05-24T07:40:57Z

...src/test/java/com/datastrato/gravitino/spark/connector/catalog/TestTransformTableChange.java

@@ -245,29 +240,4 @@ void testTransformUpdateColumnNullability() {
    Assertions.assertEquals(
        sparkUpdateColumnNullability.nullable(), gravitinoUpdateColumnNullability.nullable());
  }
-
-  @Test
-  void testUpdateColumnDefaultValue() {


move the test to spark3.4 module

...com/datastrato/gravitino/spark/connector/integration/test/iceberg/SparkIcebergCatalogIT.java

FANNG1 · 2024-05-24T10:52:38Z

@jerryshao @qqqttt123 @caican00 could you help to review when you are free, thanks

FANNG1 · 2024-05-27T04:03:27Z

...com/datastrato/gravitino/spark/connector/integration/test/iceberg/SparkIcebergCatalogIT.java

@@ -330,7 +330,7 @@ void testIcebergReservedProperties() throws NoSuchTableException {
    dropTableIfExists(tableName);
    sql(
        String.format(
-            "CREATE TABLE %s (id INT COMMENT 'id comment' NOT NULL, name STRING COMMENT '', age INT)",
+            "CREATE TABLE %s (id INT NOT NULL COMMENT 'id comment', name STRING COMMENT '', age INT)",


@caican00 , spark 3.3 will throw something like syntax error here

jerryshao · 2024-05-28T06:56:10Z

gradle/libs.versions.toml

 trino = '426'
-spark = "3.4.1" # 3.5.0 causes tests to fail
+spark = "3.4.1" # used for Gravitino catalog integration test


Why do we need this, can we reuse spark34? Also, I suggest to rename to spark-34

spark-34 seems not work because libs.versions.spark.34.get() is couldn't compile

Script compilation errors: Line 14: val sparkVersion: String = libs.versions.spark.34.get() ^ Property getter or setter expected

jerryshao · 2024-05-28T06:57:36Z

settings.gradle.kts

@@ -28,7 +28,13 @@ include(
  "clients:client-python"
 )
 include("trino-connector")
-include("spark-connector:spark-connector", "spark-connector:spark-connector-runtime")
+include("spark-connector:spark-common", "spark-connector:spark33", "spark-connector:spark33-runtime", "spark-connector:spark34", "spark-connector:spark34-runtime", "spark-connector:spark35", "spark-connector:spark35-runtime")


I think we can rename to spark-3.3, spark-3.4, like this.

How do you support different scala version?

scala version is specified by -PscalaVersion when build or test. like

./gradlew :spark-connector:spark35-runtime:build -PscalaVersion=2.13

multi scala version support will be in another PR

jerryshao · 2024-05-28T07:02:01Z

spark-connector/v3.3/spark-runtime/build.gradle.kts

+  when (sparkMajorVersion) {
+    "3.3" -> {
+      val kyuubiVersion: String = libs.versions.kyuubi4spark33.get()
+      println("Applying Spark 3.3 dependencies")


We can remove this line, this is useless.

jerryshao · 2024-05-28T07:06:31Z

spark-connector/v3.4/spark-runtime/build.gradle.kts

+  when (sparkMajorVersion) {
+    "3.4" -> {
+      val kyuubiVersion: String = libs.versions.kyuubi4spark34.get()
+      println("Applying Spark 3.4 dependencies")


FANNG1 · 2024-05-28T10:07:31Z

@jerryshao all comments are addressed, please help to review again.

### What changes were proposed in this pull request? 1. split spark connector to spark common which contains common logic and v3.x which contains adaptor logic 2. add separate GitHub action to do spark IT 3. ./gradlew :spark-connector:spark35-runtime:build to build corresponding spark connector jars ### Why are the changes needed? Fix: #3554 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? existing tests with corresponding spark version

1. split spark connector to spark common which contains common logic and v3.x which contains adaptor logic 2. add separate GitHub action to do spark IT 3. ./gradlew :spark-connector:spark35-runtime:build to build corresponding spark connector jars Fix: #3554 no existing tests with corresponding spark version

…3610) ### What changes were proposed in this pull request? 1. split spark connector to spark common which contains common logic and v3.x which contains adaptor logic 2. add separate GitHub action to do spark IT 3. ./gradlew :spark-connector:spark35-runtime:build to build corresponding spark connector jars ### Why are the changes needed? Fix: #3554 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? existing tests with corresponding spark version

…che#3415) ### What changes were proposed in this pull request? 1. split spark connector to spark common which contains common logic and v3.x which contains adaptor logic 2. add separate GitHub action to do spark IT 3. ./gradlew :spark-connector:spark35-runtime:build to build corresponding spark connector jars ### Why are the changes needed? Fix: apache#3554 ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? existing tests with corresponding spark version

FANNG1 marked this pull request as draft May 16, 2024 08:06

FANNG1 changed the title ~~support spark multi Version~~ [SIP] support spark multi Version May 16, 2024

FANNG1 force-pushed the version branch 2 times, most recently from 1f19672 to 69b6a46 Compare May 16, 2024 15:27

FANNG1 self-assigned this May 16, 2024

FANNG1 added the branch-0.5 label May 16, 2024

FANNG1 force-pushed the version branch 4 times, most recently from b71ee4f to 7771821 Compare May 23, 2024 00:17

caican00 reviewed May 23, 2024

View reviewed changes

spark-connector/spark3.3/build.gradle.kts Outdated Show resolved Hide resolved

FANNG1 force-pushed the version branch 3 times, most recently from 897a84d to 193033f Compare May 24, 2024 07:26

FANNG1 commented May 24, 2024

View reviewed changes

...com/datastrato/gravitino/spark/connector/integration/test/iceberg/SparkIcebergCatalogIT.java Outdated Show resolved Hide resolved

FANNG1 force-pushed the version branch from 193033f to cc625a7 Compare May 24, 2024 08:02

FANNG1 changed the title ~~[SIP] support spark multi Version~~ [#3554] feat(spark-connector): support spark multi Version May 24, 2024

FANNG1 force-pushed the version branch from 8b2396f to 4811ff5 Compare May 24, 2024 10:50

FANNG1 marked this pull request as ready for review May 24, 2024 10:51

FANNG1 requested review from jerryshao and qqqttt123 May 24, 2024 10:51

FANNG1 force-pushed the version branch from a619a8a to 21d37d7 Compare May 24, 2024 11:57

FANNG1 mentioned this pull request May 25, 2024

[Bug report] spark connector failed to start in real spark3.3 envriment #3564

Closed

FANNG1 commented May 27, 2024

View reviewed changes

FANNG1 force-pushed the version branch 3 times, most recently from f39ba94 to a2c9266 Compare May 28, 2024 03:03

support multi version

aafc039

move TestGravitinoDriverPlugin

76db69d

FANNG1 force-pushed the version branch from a2c9266 to a77d44c Compare May 28, 2024 03:15

support v3.x

0ff2167

FANNG1 force-pushed the version branch from c0b9cec to 0ff2167 Compare May 28, 2024 04:00

jerryshao reviewed May 28, 2024

View reviewed changes

FANNG1 added 3 commits May 28, 2024 16:15

xx

540f67b

reuse spark34

4c34793

fix github action

b577b70

jerryshao approved these changes May 28, 2024

View reviewed changes

jerryshao merged commit 4c61a64 into apache:main May 28, 2024
33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#3554] feat(spark-connector): support spark multi Version #3415

[#3554] feat(spark-connector): support spark multi Version #3415

FANNG1 commented May 16, 2024 •

edited

Loading

FANNG1 May 24, 2024

FANNG1 commented May 24, 2024

FANNG1 May 27, 2024

jerryshao May 28, 2024 •

edited

Loading

FANNG1 May 28, 2024 •

edited

Loading

jerryshao May 28, 2024

jerryshao May 28, 2024

FANNG1 May 28, 2024

FANNG1 May 28, 2024

FANNG1 May 28, 2024

jerryshao May 28, 2024

FANNG1 May 28, 2024

FANNG1 May 28, 2024

jerryshao May 28, 2024

FANNG1 May 28, 2024

FANNG1 commented May 28, 2024

[#3554] feat(spark-connector): support spark multi Version #3415

[#3554] feat(spark-connector): support spark multi Version #3415

Conversation

FANNG1 commented May 16, 2024 • edited Loading

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?

Choose a reason for hiding this comment

FANNG1 commented May 24, 2024

Choose a reason for hiding this comment

jerryshao May 28, 2024 • edited Loading

Choose a reason for hiding this comment

FANNG1 May 28, 2024 • edited Loading

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

FANNG1 commented May 28, 2024

FANNG1 commented May 16, 2024 •

edited

Loading

jerryshao May 28, 2024 •

edited

Loading

FANNG1 May 28, 2024 •

edited

Loading