[1105] Delta Lake Change Data Feed support - streaming reads #1154
Conversation
This file contains 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
- update DeltaSource to use the above trait - add option in DeltaOption - add DeltaCDCStreamSuite GitOrigin-RevId: a02b7c9f66e1c6659c39855ad26bc4bfd35e33b3
scottsand-db
commented
May 26, 2022
| } | ||
| } | ||
| } | ||
|
|
||
|
|
vkorukanti
approved these changes
May 31, 2022
| override val schema: StructType = | ||
| ColumnWithDefaultExprUtils.removeDefaultExpressions(deltaLog.snapshot.metadata.schema) | ||
| override val schema: StructType = { | ||
| val schemaWithoutCDC = |
There was a problem hiding this comment.
nit: add a comment on why we removing some fields from the schema
| deltaTable.delete("value = 1") // version 4 | ||
| }, | ||
| ProcessAllAvailable(), | ||
| CheckAnswer((1, "insert", 3), (2, "insert", 3), (1, "delete", 4)) |
There was a problem hiding this comment.
Is CheckNewAnswer better here to just check what the new values in the output are?
There was a problem hiding this comment.
All of our streaming tests use CheckAnswer, not CheckNewAnswer, so i'm perfectly fine sticking with CheckAnswer.
There was a problem hiding this comment.
flyby comment: CheckNewAnswer is often easier to read than the ever growing list in CheckAnswer, so I slightly prefer that. but its okay, not blocking this PR for that. got ahead and merge it.
| deltaTable.delete("id = 3") // version 2 | ||
| }, | ||
| ProcessAllAvailable(), | ||
| CheckAnswer((4, "insert", 1), (5, "insert", 1), (3, "delete", 2)) |
jbguerraz
pushed a commit
to jbguerraz/delta
that referenced
this pull request
Jul 6, 2022
This PR adds CDF + Streaming functionality, as part of the ongoing CDF project. The bulk of this PR is a) adding `DeltaSourceCDCSupport`. This PR adds on the necessary CDF functionality to DeltaSource. b) updating DeltaSource to use the various `DeltaSourceCDCSupport` APIs when CDF is enabled c) adding a new test suite Closes delta-io#1154 GitOrigin-RevId: e7e8f6d48f99a63e7c5d35a5d0173a9cc26cf274
jbguerraz
pushed a commit
to jbguerraz/delta
that referenced
this pull request
Jul 6, 2022
This PR adds CDF + Streaming functionality, as part of the ongoing CDF project. The bulk of this PR is a) adding `DeltaSourceCDCSupport`. This PR adds on the necessary CDF functionality to DeltaSource. b) updating DeltaSource to use the various `DeltaSourceCDCSupport` APIs when CDF is enabled c) adding a new test suite Closes delta-io#1154 GitOrigin-RevId: e7e8f6d48f99a63e7c5d35a5d0173a9cc26cf274
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR adds CDF + Streaming functionality, as part of the ongoing CDF project #1105.
The bulk of this PR is
a) adding
DeltaSourceCDCSupport. This PR adds on the necessary CDF functionality to DeltaSource.b) updating
DeltaSourceto use the variousDeltaSourceCDCSupportAPIs when CDF is enabledc) adding a new test suite
How was this patch tested?
New UTs.
Does this PR introduce any user-facing changes?
No.