Skip to content

[DO NOT MERGE] getFileChanges part 1 - #2

Open
zikangh wants to merge 1 commit into
masterfrom
zikang-han_data/initial2
Open

[DO NOT MERGE] getFileChanges part 1#2
zikangh wants to merge 1 commit into
masterfrom
zikang-han_data/initial2

Conversation

@zikangh

@zikangh zikangh commented Oct 6, 2025

Copy link
Copy Markdown
Owner

Which Delta project/connector is this regarding?

  • Spark
  • Standalone
  • Flink
  • Kernel
  • Other (fill in here)

Description

How was this patch tested?

Does this PR introduce any user-facing changes?

huan233usc
huan233usc reviewed Oct 7, 2025
View reviewed changes
Comment thread kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkMicroBatchStream.java

public SparkMicroBatchStream(String tablePath) {
this.tablePath = tablePath;
Configuration hadoopConf = new Configuration();

@huan233usc huan233usc Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to plumb it from outside.

@gengliangwang gengliangwang Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It have to be propagated from outside. Otherwise, it won't be able to access some filesystems.

gengliangwang
gengliangwang reviewed Oct 7, 2025
View reviewed changes
Comment thread kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkMicroBatchStream.java
ColumnarBatch batch,
long version,
Option<DeltaSourceOffset> endOffsetOpt,
boolean verifyMetadataAction) {

@gengliangwang gengliangwang Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verifyMetadataAction is not used

gengliangwang
gengliangwang reviewed Oct 7, 2025
View reviewed changes
Comment thread kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkMicroBatchStream.java
if (metadataOpt.isPresent()) {
// RULE 2: If commit has a read-incompatible Metadata action, fail this stream.
// TODO(M1): Implement checkReadIncompatibleSchemaChanges
metadataAction = metadataOpt;

@gengliangwang gengliangwang Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems metadataAction is not used besides the assignment here

gengliangwang
gengliangwang reviewed Oct 7, 2025
View reviewed changes
Comment thread kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkMicroBatchStream.java
}

/**
* Validate a commit and decide if it should be skipped. This mimics

@gengliangwang gengliangwang Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decide if it should be skipped

This is confusing. I thought it returns a boolean instead of void.

gengliangwang
gengliangwang reviewed Oct 7, 2025
View reviewed changes
Comment thread kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkMicroBatchStream.java
allIndexedFiles.addAll(versionFilesWithSentinels);

// Reset for the new version
currentVersionFiles = new ArrayList<>();

@gengliangwang gengliangwang Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reset here is confusing. So it will only keep the last version from the batches?

huan233usc
huan233usc reviewed Oct 7, 2025
View reviewed changes
Comment thread kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkMicroBatchStream.java

// Load the start snapshot for protocol validation
Snapshot startSnapshot =
TableManager.loadSnapshot(tablePath).atVersion(startVersion).build(engine);

@huan233usc huan233usc Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a TODO: this is not working with ccv2 table

huan233usc
huan233usc reviewed Oct 7, 2025
View reviewed changes
Comment thread kernel-spark/src/main/java/io/delta/kernel/spark/read/SparkMicroBatchStream.java
if (batch.getSize() == 0) {
continue;
}
long version = batch.getColumnVector(0).getLong(0);

@huan233usc huan233usc Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use indexOf to find the index of the version? hard coding with magic number 0 is a bit hard to understand

huan233usc
huan233usc reviewed Oct 7, 2025
View reviewed changes
Comment thread kernel-spark/src/main/java/io/delta/kernel/spark/read/KernelIndexedFile.java
* @param remove The Kernel RemoveFile, or null for AddFile or sentinel values
* @param shouldSkip A flag to indicate whether this file should be skipped
*/
public KernelIndexedFile(

@huan233usc huan233usc Oct 7, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define a Builder so that we don't really need to pass nullptr in

@zikangh

zikangh commented Oct 7, 2025

Copy link
Copy Markdown
Owner Author

Thank you! @gengliangwang @huan233usc, I created delta-io#5313 with the comments addressed. In the future I'll mostly be creating PRs against the main repo to make reviews easier.

@zikangh zikangh changed the title getFileChanges part 1 [DO NOT MERGE] getFileChanges part 1 Nov 17, 2025
zikangh pushed a commit that referenced this pull request Feb 9, 2026
@yili-db
[Spark][UC] Convert @ParameterizedTest to dynamic tests so that SBT c…
b132c27 
…an show the parameters. (delta-io#5817)

#### Which Delta project/connector is this regarding?
- [X] Spark
- [ ] Standalone
- [ ] Flink
- [ ] Kernel
- [ ] Other (fill in here)

## Description
Convert @ParameterizedTest to dynamic tests so that SBT can show the
parameters.
Also upgrade the JUnit related packages so that the display name can
actually show up in SBT output.
This is to address comment:
delta-io#5772 (comment)

Before this PR it shows:
```
[info] Test io.sparkuctest.UCDeltaTableDMLTest#testBasicInsertOperations(io.sparkuctest.UCDeltaTableIntegrationBaseTest$TableType):#1 started
[info] Test io.sparkuctest.UCDeltaTableDMLTest#testBasicInsertOperations(io.sparkuctest.UCDeltaTableIntegrationBaseTest$TableType):#2 started
[info] Test io.sparkuctest.UCDeltaTableDMLTest#testMergeWithDeleteAction(io.sparkuctest.UCDeltaTableIntegrationBaseTest$TableType):#1 started
[info] Test io.sparkuctest.UCDeltaTableDMLTest#testMergeWithDeleteAction(io.sparkuctest.UCDeltaTableIntegrationBaseTest$TableType):#2 started
...
[info] Test io.sparkuctest.UCDeltaTableCreationTest#testCreateTable(io.sparkuctest.UCDeltaTableIntegrationBaseTest$TableType, boolean, boolean, boolean, boolean):#1 started
[info] Test io.sparkuctest.UCDeltaTableCreationTest#testCreateTable(io.sparkuctest.UCDeltaTableIntegrationBaseTest$TableType, boolean, boolean, boolean, boolean):#2 started
```

After this PR it shows:
```
[info] Test io.sparkuctest.UCDeltaTableDMLTest#allTableTypesTestsFactory() #1:testUpdateOperations(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaTableDMLTest#allTableTypesTestsFactory() #1:testUpdateOperations(MANAGED) started
[info] Test io.sparkuctest.UCDeltaTableDMLTest#allTableTypesTestsFactory() #2:testDeleteOperations(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaTableDMLTest#allTableTypesTestsFactory() #2:testDeleteOperations(MANAGED) started
...
[info] Test io.sparkuctest.UCDeltaTableCreationTest#testCreateTable():tableType=EXTERNAL, withPartition=true, withCluster=false, withAsSelect=true, replaceTable=true started
[info] Test io.sparkuctest.UCDeltaTableCreationTest#testCreateTable():tableType=EXTERNAL, withPartition=true, withCluster=false, withAsSelect=true, replaceTable=false started
```

## How was this patch tested?

## Does this PR introduce _any_ user-facing changes?
No.

---------

Signed-off-by: Yi Li <yi.li@databricks.com>
zikangh pushed a commit that referenced this pull request Feb 9, 2026
@timothyw553 @tdas
Add E2E Test for Streaming Reads using UC Test Framework (delta-io#5833)
caf82cc 
#### 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
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [ ] Kernel
- [ ] Other (fill in here)

## Description
Added a new UC E2E streaming test suite to cover Delta structured
streaming read/write behavior across managed/external tables, including
UC commit version validation for managed tables. The changes live in
spark/unitycatalog/src/test/java/io/sparkuctest/
UCDeltaStreamingTest.java to exercise MemoryStream writes, streaming
reads in V2 strict mode, and checkpointed queries.
<!--
⚠️⚠️ **Note to reviewers** ⚠️⚠️ this branch is based on
delta-io#5651; therefore there are a
number of changes that are not directly related to the E2E test
integration for streaming reads (those changes are to enable these
streaming reads). Please just review
[spark/unitycatalog/src/test/java/io/sparkuctest/UCDeltaStreamingTest.java](https://github.com/delta-io/delta/pull/5833/changes#diff-04879565d272ba02d9c1e47707ec9bfdb1044460e957ec769d1914178554383b)
-->
## How was this patch tested?
Locally:
```
export UC_REMOTE=false
$ build/sbt "sparkUnityCatalog/testOnly io.sparkuctest.UCDeltaStreamingTest"

...
[info] Test run started (JUnit Jupiter)
[info] Test io.sparkuctest.UCDeltaStreamingTest#allTableTypesTestsFactory() #1:testStreamingWriteToManagedTable(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaStreamingTest#allTableTypesTestsFactory() #1:testStreamingWriteToManagedTable(MANAGED) started
[info] Test io.sparkuctest.UCDeltaStreamingTest#allTableTypesTestsFactory() #2:testStreamingReadFromTable(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaStreamingTest#allTableTypesTestsFactory() #2:testStreamingReadFromTable(MANAGED) started
[info] Test run finished: 0 failed, 0 ignored, 4 total, 24.65s
[info] Passed: Total 4, Failed 0, Errors 0, Passed 4
[success] Total time: 44 s, completed Jan 16, 2026, 12:17:45 AM
```

and tested against remote UC server
```
export UC_REMOTE=true
export UC_URI=$UC_URI
export UC_TOKEN=$UC_TOKEN
export UC_CATALOG_NAME=main
export UC_SCHEMA_NAME=demo_zh
export UC_BASE_TABLE_LOCATION=$S3_BASE_LOCATION

$ build/sbt "sparkUnityCatalog/testOnly io.sparkuctest.UCDeltaStreamingTest"

...
[info] Test run started (JUnit Jupiter)
[info] Test io.sparkuctest.UCDeltaStreamingTest#allTableTypesTestsFactory() #1:testStreamingWriteToManagedTable(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaStreamingTest#allTableTypesTestsFactory() #1:testStreamingWriteToManagedTable(MANAGED) started
[info] Test io.sparkuctest.UCDeltaStreamingTest#allTableTypesTestsFactory() #2:testStreamingReadFromTable(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaStreamingTest#allTableTypesTestsFactory() #2:testStreamingReadFromTable(MANAGED) started
[info] Test run finished: 0 failed, 0 ignored, 4 total, 250.336s
[info] Passed: Total 4, Failed 0, Errors 0, Passed 4
[success] Total time: 274 s (04:34), completed Jan 16, 2026, 12:10:14 AM
```

## Does this PR introduce _any_ user-facing changes?
No.

---------

Co-authored-by: Tathagata Das <tathagata.das1565@gmail.com>
zikangh pushed a commit that referenced this pull request Mar 9, 2026
@timothyw553
[INFRA] Fix integration tests not running due to reporting (delta-io#…
bff692e 
…6145)

<!--
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
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [ ] Kernel
- [ ] Other (fill in here)

## Description
Fix integration tests not running due to reporting. Remove common
setting to generate xml since

https://www.scala-sbt.org/1.x/docs/Testing.html
> By default, sbt will generate JUnit XML test reports for all tests in
the build, located in the target/test-reports directory for a project.
This can be disabled by disabling the JUnitXmlReportPlugin
<!--
- 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.
-->

## How was this patch tested?
Locally
```sh
$ build/sbt -mem 4096 "sparkUnityCatalog/testOnly io.sparkuctest.UCDelta*"
...
[info] Test io.sparkuctest.UCDeltaTableReadTest#allTableTypesTestsFactory() #1:testDeltaTableForPath(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaTableReadTest#allTableTypesTestsFactory() #1:testDeltaTableForPath(MANAGED) started
[info] Test io.sparkuctest.UCDeltaTableReadTest#allTableTypesTestsFactory() #2:testChangeDataFeed(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaTableReadTest#allTableTypesTestsFactory() #2:testChangeDataFeed(MANAGED) started
[info] Test io.sparkuctest.UCDeltaTableReadTest#allTableTypesTestsFactory() #3:testTimeTravelRead(EXTERNAL) started
[info] Test io.sparkuctest.UCDeltaTableReadTest#allTableTypesTestsFactory() #3:testTimeTravelRead(MANAGED) started
[info] Test run finished: 0 failed, 0 ignored, 6 total, 9.253s
[info] Passed: Total 63, Failed 0, Errors 0, Passed 63
[success] Total time: 157 s (02:37), completed Feb 26, 2026, 6:54:12 PM
```
<!--
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.
-->

## Does this PR introduce _any_ user-facing changes?
No
<!--
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'.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@gengliangwang gengliangwang gengliangwang left review comments
@huan233usc huan233usc huan233usc left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zikangh @gengliangwang @huan233usc