[Kernel-spark]Add an config and test trait to force connector to use Kernel based Dsv2 classes #5501
Conversation
This file contains hidden or 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
Signed-off-by: Xin Huang <[email protected]>
huan233usc
force-pushed
the
new-config
branch
from
b4df80a to
bc0631b
Compare
Signed-off-by: Xin Huang <[email protected]>
tdas
reviewed
Nov 17, 2025
| * io.delta.kernel.spark}) to access Spark's internal config API ({@link SQLConf#buildConf}), which | ||
| * is only accessible from {@code org.apache.spark.*} packages. | ||
| */ | ||
| public class DeltaDsv2EnableConf { |
Contributor
There was a problem hiding this comment.
Lets start calling everything just v2 instead of DSv2, just like we have defined sparkv1 and sparkv2.
also even the V1 connector already uses a lot of the DSv2 APIs.. so its confusing.
Contributor
There was a problem hiding this comment.
and is this class meant to be for all future confs supported by v2 connector, whether or not they are related to v2->v1 fallback?
if yes, then it should DeltaSQLConfV2. if no, then why not.
Collaborator
Author
There was a problem hiding this comment.
Renamed to DeltaSQLConfV2, this config should only for v2 connector related path
tdas
reviewed
Nov 17, 2025
| */ | ||
| public class DeltaDsv2EnableConf { | ||
|
|
||
| private static final String SQL_CONF_PREFIX = "spark.databricks.delta"; |
Contributor
There was a problem hiding this comment.
why are we defining this again? isnt this defined the DeltaSQLConf?
Collaborator
Author
There was a problem hiding this comment.
Was because of the difficulties of Java classes extending Scala classes. Changed this file to scala, as it mainly reuses the scala file's logic
tdas
reviewed
Nov 17, 2025
| SQLConf.buildConf(SQL_CONF_PREFIX + ".datasourcev2.enableMode") | ||
| .doc( | ||
| "Controls the DataSourceV2 enable mode. " | ||
| + "Valid values: NONE (disabled), STRICT (always enabled), AUTO (automatic determination).") |
Contributor
There was a problem hiding this comment.
can we define them as spark.databricks.delta.v2.enableMode
tdas
reviewed
Nov 18, 2025
spark-unified/src/main/java/org/apache/spark/sql/delta/catalog/DeltaCatalog.java
Show resolved
Hide resolved
tdas
reviewed
Nov 18, 2025
|
|
||
| @Override | ||
| public Table newDeltaCatalogBasedTable(Identifier ident, CatalogTable catalogTable) { | ||
| return createBasedOnDsv2Mode( |
tdas
reviewed
Nov 18, 2025
| } | ||
|
|
||
| /** | ||
| * Create table based on DataSourceV2 enable mode configuration. |
Contributor
There was a problem hiding this comment.
Delta v2 connector mode. link to a single location that define v1 vs v2. maybe the conf?
tdas
reviewed
Nov 18, 2025
| * @return Table instance from the appropriate supplier | ||
| */ | ||
| private Table createBasedOnDsv2Mode( | ||
| Supplier<Table> dsv2ConnectorSupplier, |
Contributor
There was a problem hiding this comment.
Suggested change
| Supplier<Table> dsv2ConnectorSupplier, | |
| Supplier<Table> v2ConnectorSupplier, |
tdas
reviewed
Nov 18, 2025
spark-unified/src/main/java/org/apache/spark/sql/delta/catalog/DeltaCatalog.java
Show resolved
Hide resolved
tdas
reviewed
Nov 18, 2025
Comment on lines
41
to
51
| public static void setUpSpark() { | ||
| spark = | ||
| SparkSession.builder() | ||
| .master("local[*]") | ||
| .appName("DeltaCatalogTest") | ||
| .config("spark.sql.extensions", "io.delta.sql.DeltaSparkSessionExtension") | ||
| .config( | ||
| "spark.sql.catalog.spark_catalog", | ||
| "org.apache.spark.sql.delta.catalog.DeltaCatalog") | ||
| .getOrCreate(); | ||
| } |
Contributor
There was a problem hiding this comment.
arent their existing traits for this? DeltaSQLCommandTest?
Collaborator
|
@huan233usc I think this PR overlaps with #5475. To test V2 as default, we can add another configuration and change the V1 batch read as V2. |
raveeram-db
reviewed
Nov 18, 2025
Collaborator
raveeram-db
left a comment
raveeram-db
left a comment
There was a problem hiding this comment.
Overall, looks great, just a nit
| * <ul> | ||
| * <li>"NONE": DataSourceV2 is disabled, always use V1 (DeltaTableV2) | ||
| * <li>"STRICT": DataSourceV2 is strictly enforced, always use V2 (Kernel SparkTable) | ||
| * <li>"AUTO": Automatically determine based on query (default) |
Collaborator
There was a problem hiding this comment.
Is AUTO implemented anywhere now (or rather is it really the default?). Perhaps we could update it when it's made as such, or just remove this clause entirely to remove confusion
Collaborator
Author
There was a problem hiding this comment.
I just remove it now and will add it back when we make this implementation
raveeram-db
approved these changes
Nov 21, 2025
tdas
reviewed
Nov 24, 2025
| "Controls the Delta V2 connector enable mode. " + | ||
| "Valid values: NONE (disabled, default), STRICT (should ONLY be enabled for testing).") | ||
| .stringConf | ||
| .checkValues(Set("NONE", "STRICT")) |
Contributor
There was a problem hiding this comment.
where did the AUTO mode go?
tdas
reviewed
Nov 24, 2025
spark/src/main/scala/org/apache/spark/sql/delta/catalog/AbstractDeltaCatalog.scala
Show resolved
Hide resolved
tdas
approved these changes
Nov 25, 2025
5 tasks
zikangh
pushed a commit
to zikangh/delta
that referenced
this pull request
Nov 26, 2025
…Kernel based Dsv2 classes (delta-io#5501) <!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md 2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'. 3. Be sure to keep the PR description updated to reflect all changes. 4. Please write your PR title to summarize what this PR proposes. 5. If possible, provide a concise example to reproduce the issue for a faster review. 6. If applicable, include the corresponding issue number in the PR title and link it in the body. --> #### Which Delta project/connector is this regarding? <!-- Please add the component selected below to the beginning of the pull request title For example: [Spark] Title of my pull request --> - [x] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description <!-- - Describe what this PR changes. - Describe why we need the change. If this PR resolves an issue be sure to include "Resolves #XXX" to correctly link and close the issue upon merge. --> This PR introduces a configuration-based mechanism to enable Kernel-backed DataSourceV2 reads in Delta Spark, with test trait to force run test with new connector. Key Changes 1. Configuration System(supposed only to use in test) Added DeltaDsv2EnableConf with spark.databricks.delta.datasourcev2.enableMode: - NONE: V1 only (DeltaTableV2) - STRICT: V2 only (Kernel SparkTable) 2. Catalog Routing based on the config 3. Test Trait and sample test case - Added Dsv2ForceTest trait: forces STRICT mode + selective test skipping - Added DeltaDataFrameWriterV2Dsv2Suite: validates Kernel's read-after-V1-write capability ## How was this patch tested? <!-- If tests were added, say they were added here. Please make sure to test the changes thoroughly including negative and positive cases if possible. If the changes were tested in any way other than unit tests, please clarify how you tested step by step (ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future). If the changes were not tested, please explain why. --> Unit ## Does this PR introduce _any_ user-facing changes? <!-- If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible. If possible, please also clarify if this is a user-facing change compared to the released Delta Lake versions or within the unreleased branches such as master. If no, write 'No'. --> No --------- Signed-off-by: Xin Huang <[email protected]> Signed-off-by: Xin Huang <[email protected]>
gengliangwang
added a commit
that referenced
this pull request
Nov 26, 2025
<!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md 2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'. 3. Be sure to keep the PR description updated to reflect all changes. 4. Please write your PR title to summarize what this PR proposes. 5. If possible, provide a concise example to reproduce the issue for a faster review. 6. If applicable, include the corresponding issue number in the PR title and link it in the body. --> #### Which Delta project/connector is this regarding? <!-- Please add the component selected below to the beginning of the pull request title For example: [Spark] Title of my pull request --> - [x] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description Follow-up of #5501. Rename table loading methods to better reflect their purpose: - `newDeltaCatalogBasedTable` → `loadCatalogTable` - `newDeltaPathTable` → `loadPathTable` - `createBasedOnV2Mode` → `loadTableInternal` Updated corresponding Javadoc comments from "Creates" to "Loads" to match the method semantics. ## How was this patch tested? Existing tests ## Does this PR introduce _any_ user-facing changes? <!-- If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible. If possible, please also clarify if this is a user-facing change compared to the released Delta Lake versions or within the unreleased branches such as master. If no, write 'No'. --> No
TimothyW553
pushed a commit
to TimothyW553/delta
that referenced
this pull request
Dec 2, 2025
…Kernel based Dsv2 classes (delta-io#5501) <!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md 2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'. 3. Be sure to keep the PR description updated to reflect all changes. 4. Please write your PR title to summarize what this PR proposes. 5. If possible, provide a concise example to reproduce the issue for a faster review. 6. If applicable, include the corresponding issue number in the PR title and link it in the body. --> #### Which Delta project/connector is this regarding? <!-- Please add the component selected below to the beginning of the pull request title For example: [Spark] Title of my pull request --> - [x] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description <!-- - Describe what this PR changes. - Describe why we need the change. If this PR resolves an issue be sure to include "Resolves #XXX" to correctly link and close the issue upon merge. --> This PR introduces a configuration-based mechanism to enable Kernel-backed DataSourceV2 reads in Delta Spark, with test trait to force run test with new connector. Key Changes 1. Configuration System(supposed only to use in test) Added DeltaDsv2EnableConf with spark.databricks.delta.datasourcev2.enableMode: - NONE: V1 only (DeltaTableV2) - STRICT: V2 only (Kernel SparkTable) 2. Catalog Routing based on the config 3. Test Trait and sample test case - Added Dsv2ForceTest trait: forces STRICT mode + selective test skipping - Added DeltaDataFrameWriterV2Dsv2Suite: validates Kernel's read-after-V1-write capability ## How was this patch tested? <!-- If tests were added, say they were added here. Please make sure to test the changes thoroughly including negative and positive cases if possible. If the changes were tested in any way other than unit tests, please clarify how you tested step by step (ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future). If the changes were not tested, please explain why. --> Unit ## Does this PR introduce _any_ user-facing changes? <!-- If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible. If possible, please also clarify if this is a user-facing change compared to the released Delta Lake versions or within the unreleased branches such as master. If no, write 'No'. --> No --------- Signed-off-by: Xin Huang <[email protected]> Signed-off-by: Xin Huang <[email protected]>
TimothyW553
pushed a commit
to TimothyW553/delta
that referenced
this pull request
Dec 2, 2025
<!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md 2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'. 3. Be sure to keep the PR description updated to reflect all changes. 4. Please write your PR title to summarize what this PR proposes. 5. If possible, provide a concise example to reproduce the issue for a faster review. 6. If applicable, include the corresponding issue number in the PR title and link it in the body. --> #### Which Delta project/connector is this regarding? <!-- Please add the component selected below to the beginning of the pull request title For example: [Spark] Title of my pull request --> - [x] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description Follow-up of delta-io#5501. Rename table loading methods to better reflect their purpose: - `newDeltaCatalogBasedTable` → `loadCatalogTable` - `newDeltaPathTable` → `loadPathTable` - `createBasedOnV2Mode` → `loadTableInternal` Updated corresponding Javadoc comments from "Creates" to "Loads" to match the method semantics. ## How was this patch tested? Existing tests ## Does this PR introduce _any_ user-facing changes? <!-- If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible. If possible, please also clarify if this is a user-facing change compared to the released Delta Lake versions or within the unreleased branches such as master. If no, write 'No'. --> No
zikangh
pushed a commit
to zikangh/delta
that referenced
this pull request
Dec 2, 2025
<!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md 2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'. 3. Be sure to keep the PR description updated to reflect all changes. 4. Please write your PR title to summarize what this PR proposes. 5. If possible, provide a concise example to reproduce the issue for a faster review. 6. If applicable, include the corresponding issue number in the PR title and link it in the body. --> #### Which Delta project/connector is this regarding? <!-- Please add the component selected below to the beginning of the pull request title For example: [Spark] Title of my pull request --> - [x] Spark - [ ] Standalone - [ ] Flink - [ ] Kernel - [ ] Other (fill in here) ## Description Follow-up of delta-io#5501. Rename table loading methods to better reflect their purpose: - `newDeltaCatalogBasedTable` → `loadCatalogTable` - `newDeltaPathTable` → `loadPathTable` - `createBasedOnV2Mode` → `loadTableInternal` Updated corresponding Javadoc comments from "Creates" to "Loads" to match the method semantics. ## How was this patch tested? Existing tests ## Does this PR introduce _any_ user-facing changes? <!-- If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible. If possible, please also clarify if this is a user-facing change compared to the released Delta Lake versions or within the unreleased branches such as master. If no, write 'No'. --> No
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Which Delta project/connector is this regarding?
Description
This PR introduces a configuration-based mechanism to enable Kernel-backed DataSourceV2 reads in Delta Spark, with test trait to force run test with new connector.
Key Changes
Configuration System(supposed only to use in test)
Added DeltaDsv2EnableConf with spark.databricks.delta.datasourcev2.enableMode:
Catalog Routing based on the config
Test Trait and sample test case
How was this patch tested?
Unit
Does this PR introduce any user-facing changes?
No