[Spark]Refactor Spark project structure to combine both Dsv1 connector and kernel backed Dsv2 connector #5320

huan233usc · 2025-10-09T19:59:10Z

Which Delta project/connector is this regarding?

Description

This PR refactors the Delta Spark build system to support a modular architecture that separates V1 and V2 implementations while maintaining a single published delta-spark jar that could use connector for both V1 and V2 and with public entry point DeltaCatalog and DeltaSparkSessionExtension unchanged.

Architecture Changes

Module Structure:

sparkV1 (internal): Delta Spark V1 implementation (production code only, no tests)
sparkV1Shaded (internal): V1 without DeltaLog/Snapshot/OptimisticTransaction classes, used to avoid V2 connector accidentally depend on legacy V1 representation of delta log.
sparkV2 (internal): Kernel-based Delta Spark implementation (formerly kernelSpark)
spark (combined): Final published module that merges V1 + V2 + storage into delta-spark.jar

Details Changes

Rename old catalog plugin:

Introduced Abstract* base classes in sparkV1 and Legacy* subclasses for backward compatibility
New unified classes in spark-combined:
- DeltaCatalog extends AbstractDeltaCatalog
- DeltaSparkSessionExtension extends AbstractDeltaSparkSessionExtension
  This makes public entry point DeltaCatalog and DeltaSparkSessionExtension unchanged.

Single Jar rules all connectors:

Internal modules (sparkV1, sparkV2, sparkV1Shaded) marked with skipReleaseSettings - not published to Maven
spark (combined) uses packageBin / mappings to merge classes from internal modules
Duplicate class detection added to prevent overlapping code
POM post-processing excludes internal module dependencies

Test Configuration:

Tests run in spark (combined) module with Test / baseDirectory pointing to spark/ directory
I could run tests for both V1/V2 connector

How was this patch tested?

All existing unit tests pass

python3 run-integration-tests.py --unity-catalog-commit-coordinator-integration-tests matches master branch

Does this PR introduce any user-facing changes?

No user-facing changes. This is an internal build system refactoring:

No

tdas · 2025-10-10T08:01:03Z

build.sbt

+// Module 3: delta-spark-v2 (kernel-spark based, depends on v1-shaded)
+// ============================================================
+lazy val `delta-spark-v2` = (project in file("kernel-spark"))
+  .dependsOn(`delta-spark-v1-shaded`) // Only depends on shaded v1 (no DeltaLog)


exactly what i had in mind.

tdas · 2025-10-10T08:05:21Z

build.sbt

+    // Test sources point to original spark/src/test/ (no file movement)
+    Test / unmanagedSourceDirectories ++= Seq(
+      baseDirectory.value.getParentFile / "spark" / "src" / "test" / "scala",
+      baseDirectory.value.getParentFile / "spark" / "src" / "test" / "java"
+    ),
+    Test / unmanagedResourceDirectories += 
+      baseDirectory.value.getParentFile / "spark" / "src" / "test" / "resources",
+
+    // Include spark-version-specific test sources
+    Test / unmanagedSourceDirectories ++= {
+      val sparkVer = sparkVersion.value
+      if (sparkVer.startsWith("3.5")) {
+        Seq(baseDirectory.value.getParentFile / "spark" / "src" / "test" / "scala-spark-3.5")
+      } else if (sparkVer.startsWith("4.0")) {
+        Seq(baseDirectory.value.getParentFile / "spark" / "src" / "test" / "scala-spark-master")
+      } else {
+        Seq.empty
+      }
+    },


it might be simpler to actually move the files, but this is okay for the first cut.

tdas · 2025-10-10T08:10:59Z

build.sbt

+// ============================================================
+// Module 1: delta-spark-v1 (prod code only, no tests)
+// ============================================================
+lazy val `delta-spark-v1` = (project in file("spark"))


why are you using "`" (backticks) ?
why not just name it sparkV1 (it all already delta)?

and fundamentally.. this is scala... and scala variables. nobody uses - in variable names :)

Ah I see, will rename

tdas · 2025-10-10T08:12:58Z

build.sbt

+// ============================================================
+lazy val spark = (project in file("spark-combined"))
+  .dependsOn(`delta-spark-shaded`)  // Direct dependency on shaded (for delegation classes)
+  .dependsOn(`delta-spark-v1` % "test->test")  // Test utilities from v1


why do you need this? v1 has no tests.

tdas · 2025-10-10T08:16:50Z

build.sbt

+    // This module contains delegation code like:
+    // - DeltaCatalog (delegates to V1 or V2)
+    // - DeltaSparkSessionExtension (registers both)


do we really have to shade anything?

These comments and many of those are stale, I will fix them (after fixing tests) and let you know then

kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkBatch.java

huan233usc · 2025-10-25T00:57:46Z

Hi @tdas @gengliangwang @scottsand-db -- please take another look when you have a chance, thanks

tdas · 2025-10-27T19:22:18Z

spark/src/main/scala/io/delta/sql/DeltaSparkSessionExtension.scala

 */
-class DeltaSparkSessionExtension extends (SparkSessionExtensions => Unit) {
+// V1 entry point class for backwards compatibility with Spark V1 APIs.
+class V1DeltaSparkSessionExtension extends AbstractDeltaSparkSessionExtension


why V1ClassName, and not ClassNameV1?
we have been following the pattern of version at the end... isnt it? DeltaTableV2?

tdas · 2025-10-27T19:23:36Z

spark/src/main/scala/io/delta/sql/AbstractDeltaSparkSessionExtension.scala

@@ -80,7 +80,11 @@ import org.apache.spark.sql.internal.SQLConf
 *
 * @since 0.4.0
 */
-class DeltaSparkSessionExtension extends (SparkSessionExtensions => Unit) {
+// V1 entry point class for backwards compatibility with Spark V1 APIs.


the docs above all refer to DeltaSparkSessionExtension with very nice CUJs where as this class is a different name.

i think the right thing to do is move this entire doc to the new DeltaSparkSessionExtension class. Along with the explanation of about delegation logic.

And in this V1DeltaSparkSessionExtension class has docs that ask users to see the docs on DeltaSparkSessionExtension.

With that there is a central place that documents this whole things and other places points to that one source of truth.

tdas · 2025-10-27T19:24:49Z

spark/src/main/scala/io/delta/sql/AbstractDeltaSparkSessionExtension.scala

+class V1DeltaSparkSessionExtension extends AbstractDeltaSparkSessionExtension
+
+// Abstract base class that contains the core Delta Spark Session extension logic.
+class AbstractDeltaSparkSessionExtension extends (SparkSessionExtensions => Unit) {
  override def apply(extensions: SparkSessionExtensions): Unit = {


also .. the file name should be changed to match the class name

tdas · 2025-10-27T19:25:13Z

spark/src/main/scala/org/apache/spark/sql/delta/catalog/DeltaCatalog.scala

 */
-class DeltaCatalog extends DelegatingCatalogExtension
+// V1 entry point class for backwards compatibility with Spark V1 APIs.
+class V1DeltaCatalog extends AbstractDeltaCatalog


same comment about ClassNameVx as i did for the extension. https://github.com/delta-io/delta/pull/5320/files#r2466822707

also same comment about docs and file name as i did here -
https://github.com/delta-io/delta/pull/5320/files#r2466825844

tdas · 2025-10-27T19:27:01Z

spark-unified/README.md

+# Delta Spark Unified Module
+
+This module contains the final, published `delta-spark` JAR that unifies both:
+- **V1 (DSv1)**: The traditional Delta Lake connector with full `DeltaLog` support


Suggested change

- **V1 (DSv1)**: The traditional Delta Lake connector with full `DeltaLog` support

- **V1 (hybrid DSv1 and DSv2)**: The traditional Delta Lake connector with full `DeltaLog` support

isnt that true? since the catalog is already v2 .. right?

tdas · 2025-10-27T19:27:27Z

spark-unified/README.md

+
+This module contains the final, published `delta-spark` JAR that unifies both:
+- **V1 (DSv1)**: The traditional Delta Lake connector with full `DeltaLog` support
+- **V2 (DSv2)**: The new Kernel-backed connector for improved performance


Suggested change

- **V2 (DSv2)**: The new Kernel-backed connector for improved performance

- **V2 (pure DSv2)**: The new Kernel-backed connector for improved performance

tdas · 2025-10-27T19:29:05Z

spark-unified/README.md

+**Published module**:
+- `delta-spark` (this module) - contains merged classes from all internal modules
+
+## Build


this page is great.

in a follow PR as i said offline. we should make all spark stuff in a one top level dir

spark/ - v1/ - v2/ - unified/ - README.md (move this read me at this top level dir)

tdas · 2025-10-27T19:31:15Z

spark/src/main/scala/org/apache/spark/sql/delta/DeltaErrors.scala

-        catalogImplConfig, classOf[DeltaCatalog].getName,
-        classOf[DeltaSparkSessionExtension].getName,
-        catalogImplConfig, classOf[DeltaCatalog].getName),
+      messageParameters = Array(classOf[AbstractDeltaSparkSessionExtension].getName,


What will this produce? earlier it produced the name DeltaSparkSessionExtension and DeltaCatalog. but now wont this produce Abstract***?
if so, we dont want to expose that in the error message since the user CUJ must not change.. right?

tdas · 2025-10-27T19:33:14Z

spark/src/test/scala/org/apache/spark/sql/delta/DeltaErrorsSuite.scala


      checkError(e, "DELTA_CONFIGURE_SPARK_SESSION_WITH_EXTENSION_AND_CATALOG", "56038", Map(
-        "sparkSessionExtensionName" -> classOf[DeltaSparkSessionExtension].getName,
+        "sparkSessionExtensionName" -> classOf[AbstractDeltaSparkSessionExtension].getName,


actually.. i think this test is wrong. if the goal of this test is to verify that the error message is as expected.. it should hardcode the exact class name string that is expected so that the test catches

please find out from delta-master what this string should be (ideally same as the documented full class name in the CUJ) and hard code that here. only then we will be sure that we are never breaking any error message tied to CUJ

tdas · 2025-10-27T19:35:04Z

kernel-spark/src/test/java/io/delta/kernel/spark/SparkDsv2TestBase.java

            .master("local[*]")
            .appName("SparkKernelDsv2Tests")
-            .config("spark.sql.extensions", "io.delta.sql.DeltaSparkSessionExtension")
+            .config("spark.sql.extensions", "io.delta.sql.V1DeltaSparkSessionExtension")


why does this need to change? this is in kernel-spark, so it should have access to the new DeltaSparkSessionSessionExtension.. isnt it?

same for the DeltaCatalog below

kernel-spark/ is location for SparkV2, DeltaSparkSessionSessionExtension is in Spark-unified/ (for accessing both V1 + V2 code) so kernel-spark/ cannot access DeltaSparkSessionSessionExtension.

We will need to use DeltaSparkSessionExtensionV1 here for test

tdas · 2025-10-27T19:35:31Z

kernel-spark/src/test/java/io/delta/kernel/spark/Dsv2BasicTest.java

            .set("spark.sql.catalog.dsv2", "io.delta.kernel.spark.catalog.TestCatalog")
            .set("spark.sql.catalog.dsv2.base_path", tempDir.getAbsolutePath())
-            .set("spark.sql.extensions", "io.delta.sql.DeltaSparkSessionExtension")
+            .set("spark.sql.extensions", "io.delta.sql.V1DeltaSparkSessionExtension")


same comment as below.

tdas · 2025-10-27T19:35:38Z

kernel-spark/src/test/java/io/delta/kernel/spark/read/SparkGoldenTableTest.java

            .set("spark.sql.catalog.dsv2", "io.delta.kernel.spark.catalog.TestCatalog")
            .set("spark.sql.catalog.dsv2.base_path", tempDir.getAbsolutePath())
-            .set("spark.sql.extensions", "io.delta.sql.DeltaSparkSessionExtension")
+            .set("spark.sql.extensions", "io.delta.sql.V1DeltaSparkSessionExtension")


same comment as below.

huan233usc added 12 commits October 9, 2025 12:57

save

e6e374f

save

9560950

save

8b9fda1

save

4f45983

fix

d6dc7ef

fix

3a6472b

fix

b55bfaa

fix

29a9dbe

fix

5cbf64f

fix

9a6acdd

fix

f8d4862

fix

6c7c972

tdas reviewed Oct 10, 2025

View reviewed changes

huan233usc added 2 commits October 10, 2025 10:36

fix

565f9cb

fix

74a1f5c

huan233usc changed the title ~~[POC]New spark structure~~ [WIP][POC]New spark structure Oct 10, 2025

huan233usc added 4 commits October 10, 2025 11:01

fix

c6c306d

fix

dbf686b

fix

2929d72

fix

642a42a

huan233usc force-pushed the new branch from 278d77b to 642a42a Compare October 11, 2025 03:51

huan233usc added 5 commits October 10, 2025 21:04

fix

e485cda

fix

f6ef579

fix

5f0dbc8

fix

812ba1d

fix

7dd8d18

huan233usc requested review from gengliangwang, scottsand-db and zikangh October 21, 2025 18:40

Merge branch 'master' into new

8729f60

tdas reviewed Oct 23, 2025

View reviewed changes

kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkBatch.java Outdated Show resolved Hide resolved

huan233usc added 2 commits October 23, 2025 16:03

revert unnecessary

5fb0ec9

rename

6caed50

huan233usc requested a review from tdas October 24, 2025 06:56

huan233usc self-assigned this Oct 24, 2025

huan233usc added 2 commits October 24, 2025 17:53

merge from master

bc84356

revert

2ce6de9

tdas reviewed Oct 27, 2025

View reviewed changes

huan233usc added 6 commits October 27, 2025 15:03

address comments

9c6d8aa

merge master

51bcb1c

fix

429244f

no storage

52e0c3a

add

8558ceb

add

022d1fe

	- V1 (DSv1): The traditional Delta Lake connector with full `DeltaLog` support
	- V1 (hybrid DSv1 and DSv2): The traditional Delta Lake connector with full `DeltaLog` support

	- V2 (DSv2): The new Kernel-backed connector for improved performance
	- V2 (pure DSv2): The new Kernel-backed connector for improved performance

Uh oh!

[Spark]Refactor Spark project structure to combine both Dsv1 connector and kernel backed Dsv2 connector #5320

Are you sure you want to change the base?

[Spark]Refactor Spark project structure to combine both Dsv1 connector and kernel backed Dsv2 connector #5320

Conversation

huan233usc commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which Delta project/connector is this regarding?

Description

Architecture Changes

Details Changes

How was this patch tested?

Does this PR introduce any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdas Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

huan233usc commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdas Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdas Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

huan233usc commented Oct 9, 2025 •

edited

Loading

tdas Oct 10, 2025 •

edited

Loading

huan233usc commented Oct 25, 2025 •

edited

Loading

tdas Oct 27, 2025 •

edited

Loading

tdas Oct 27, 2025 •

edited

Loading