You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/11/10 06:23:58 UTC
[GitHub] [iceberg] SreeramGarlapati opened a new pull request #3517: Skip processing snapshots of type Overwrite during readStream
SreeramGarlapati opened a new pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517
Delete operations on iceberg table can translate to Snapshots of type `DataOperations.DELETE` or `DataOperations.OVERWRITE`.
`DataOperations.DELETE` - when the delete operation on the table translates to full file deletes (for ex: deletes wiping out partitions using partitionKey level predicates)
`DataOperations.OVERWRITE` - is the most common case of Delete.
@rdblue / @aokolnychyi / @RussellSpitzer / @kbendick - pl. lemme know what you folks think & once we achieve principal level alignment - I will add unittests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794698122
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -200,13 +202,25 @@ public void stop() {
private boolean shouldProcess(Snapshot snapshot) {
String op = snapshot.operation();
- Preconditions.checkState(!op.equals(DataOperations.DELETE) || skipDelete,
- "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
- snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
- Preconditions.checkState(
- op.equals(DataOperations.DELETE) || op.equals(DataOperations.APPEND) || op.equals(DataOperations.REPLACE),
- "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId());
- return op.equals(DataOperations.APPEND);
+ switch (op) {
+ case DataOperations.APPEND:
+ return true;
+ case DataOperations.REPLACE:
+ return false;
+ case DataOperations.DELETE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
+ return false;
+ case DataOperations.OVERWRITE:
+ Preconditions.checkState(skipOverwrite,
+ "Cannot process overwrite snapshot : %s. Set read option %s to allow skipping snapshots of type overwrite",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_OVERWRITE_SNAPSHOTS);
+ return false;
+ default:
+ throw new IllegalStateException(String.format(
+ "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId()));
Review comment:
Thanks, this message seems better.
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -200,13 +202,25 @@ public void stop() {
private boolean shouldProcess(Snapshot snapshot) {
String op = snapshot.operation();
- Preconditions.checkState(!op.equals(DataOperations.DELETE) || skipDelete,
- "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
- snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
- Preconditions.checkState(
- op.equals(DataOperations.DELETE) || op.equals(DataOperations.APPEND) || op.equals(DataOperations.REPLACE),
- "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId());
- return op.equals(DataOperations.APPEND);
+ switch (op) {
+ case DataOperations.APPEND:
+ return true;
+ case DataOperations.REPLACE:
+ return false;
+ case DataOperations.DELETE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
+ return false;
+ case DataOperations.OVERWRITE:
+ Preconditions.checkState(skipOverwrite,
+ "Cannot process overwrite snapshot : %s. Set read option %s to allow skipping snapshots of type overwrite",
Review comment:
Sure.
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -200,13 +202,25 @@ public void stop() {
private boolean shouldProcess(Snapshot snapshot) {
String op = snapshot.operation();
- Preconditions.checkState(!op.equals(DataOperations.DELETE) || skipDelete,
- "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
- snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
- Preconditions.checkState(
- op.equals(DataOperations.DELETE) || op.equals(DataOperations.APPEND) || op.equals(DataOperations.REPLACE),
- "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId());
- return op.equals(DataOperations.APPEND);
+ switch (op) {
+ case DataOperations.APPEND:
+ return true;
+ case DataOperations.REPLACE:
+ return false;
+ case DataOperations.DELETE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
Review comment:
Sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r764674380
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
I connected with @SreeramGarlapati to contribute on this PR.
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
I have added a separate config to skip overwrites. I will discuss with @SreeramGarlapati and will update on the plan to eventually handle upserts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794820073
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -200,13 +202,25 @@ public void stop() {
private boolean shouldProcess(Snapshot snapshot) {
String op = snapshot.operation();
- Preconditions.checkState(!op.equals(DataOperations.DELETE) || skipDelete,
- "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
- snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
- Preconditions.checkState(
- op.equals(DataOperations.DELETE) || op.equals(DataOperations.APPEND) || op.equals(DataOperations.REPLACE),
- "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId());
- return op.equals(DataOperations.APPEND);
+ switch (op) {
+ case DataOperations.APPEND:
+ return true;
+ case DataOperations.REPLACE:
+ return false;
+ case DataOperations.DELETE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
Review comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794680044
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -200,13 +202,25 @@ public void stop() {
private boolean shouldProcess(Snapshot snapshot) {
String op = snapshot.operation();
- Preconditions.checkState(!op.equals(DataOperations.DELETE) || skipDelete,
- "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
- snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
- Preconditions.checkState(
- op.equals(DataOperations.DELETE) || op.equals(DataOperations.APPEND) || op.equals(DataOperations.REPLACE),
- "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId());
- return op.equals(DataOperations.APPEND);
+ switch (op) {
+ case DataOperations.APPEND:
+ return true;
+ case DataOperations.REPLACE:
+ return false;
+ case DataOperations.DELETE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
+ return false;
+ case DataOperations.OVERWRITE:
+ Preconditions.checkState(skipOverwrite,
+ "Cannot process overwrite snapshot : %s. Set read option %s to allow skipping snapshots of type overwrite",
Review comment:
Can you copy the delete error message, but change "delete" to "overwrite"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r763897554
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
I will discuss with @SreeramGarlapati and will update on the plan to eventually handle upserts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r749765488
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
+1. I would prefer they be two separate configs, but also that we have a plan for the longer term to handle sending out these row deltas.
I'd be ok with getting a PR in to ignore `OVERWRITE`, but this isn't something we should ignore in the longer term (or even really the near-to-medium term) as others have mentioned.
Personally I would consider using a schema similar to the delta.io change capture feed that has a dataframe with the before image / after image (row before and after update) and then the type of operation for each row (insert, delete, update_before, update_after).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] SreeramGarlapati commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
SreeramGarlapati commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r778497242
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
all in all, there are 2 options for reading upserts:
1. for updates which are written with - `copy on write` -- a new data file is created which has a combination of both old rows and these new updated rows. So, in this case - we can take a spark option from the user to take consent - that they are okay with data replay.
2. for updates which are written with - `merge on read` - we will expose an option to read change data feed - where we will include a metadata column - which indicates whether a record is an INSERT vs DELETE.
did this make sense - @rdblue & @kbendick
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] SreeramGarlapati commented on pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
SreeramGarlapati commented on pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#issuecomment-965502105
I stumbled on the exact same change made by @kbendick : https://github.com/apache/iceberg/pull/3267
@kbendick - truly apologize - I did not get a chance to look at this (I had to turn off my git notifications at some point in the past). Truly appreciate - if you can reopen your branch and get that going. Or - I can merge your changes into this PR. Either way works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r763695130
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
I have added a separate config to skip overwrites.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r750715460
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process overwrite snapshot : %s. Set read option %s to allow skipping snapshots of type overwrite",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
+ return false;
default:
Review comment:
Should add a test for this new conf option
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r763695130
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
I connected with @SreeramGarlapati to contribute on this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r763897554
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
I have added a separate config to skip overwrites. I will discuss with @SreeramGarlapati and will update on the plan to eventually handle upserts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] SreeramGarlapati commented on pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
SreeramGarlapati commented on pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#issuecomment-964845832
https://github.com/apache/iceberg/projects/2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794681406
##########
File path: spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java
##########
@@ -481,6 +481,35 @@ public void testReadStreamWithSnapshotTypeDeleteAndSkipDeleteOption() throws Exc
.containsExactlyInAnyOrderElementsOf(Iterables.concat(dataAcrossSnapshots));
}
+ @Test
+ public void testReadStreamWithSnapshotTypeDeleteAndSkipOverwriteOption() throws Exception {
+ table.updateSpec()
+ .removeField("id_bucket")
+ .addField(ref("id"))
+ .commit();
+
+ table.refresh();
Review comment:
No need for refresh calls after altering a table. The table instance that was just committed will be up to date afterwards.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] SreeramGarlapati edited a comment on pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
SreeramGarlapati edited a comment on pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#issuecomment-964845832
https://github.com/apache/iceberg/projects/2
continuation of #2660
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] SreeramGarlapati commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
SreeramGarlapati commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r778497242
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
I agree with @kbendick - we will invest in emitting change feed - similar to delta format.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#issuecomment-1020815246
Hi @rdblue @kbendick @RussellSpitzer Request you to please review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794681674
##########
File path: spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java
##########
@@ -481,6 +481,35 @@ public void testReadStreamWithSnapshotTypeDeleteAndSkipDeleteOption() throws Exc
.containsExactlyInAnyOrderElementsOf(Iterables.concat(dataAcrossSnapshots));
}
+ @Test
+ public void testReadStreamWithSnapshotTypeDeleteAndSkipOverwriteOption() throws Exception {
+ table.updateSpec()
+ .removeField("id_bucket")
+ .addField(ref("id"))
+ .commit();
+
+ table.refresh();
+
+ // fill table with some data
+ List<List<SimpleRecord>> dataAcrossSnapshots = TEST_DATA_MULTIPLE_SNAPSHOTS;
+ appendDataAsMultipleSnapshots(dataAcrossSnapshots);
+
+ table.refresh();
Review comment:
Also unnecessary. The table will be refreshed as part of the overwrite commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794680738
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -200,13 +202,25 @@ public void stop() {
private boolean shouldProcess(Snapshot snapshot) {
String op = snapshot.operation();
- Preconditions.checkState(!op.equals(DataOperations.DELETE) || skipDelete,
- "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
- snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
- Preconditions.checkState(
- op.equals(DataOperations.DELETE) || op.equals(DataOperations.APPEND) || op.equals(DataOperations.REPLACE),
- "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId());
- return op.equals(DataOperations.APPEND);
+ switch (op) {
+ case DataOperations.APPEND:
+ return true;
+ case DataOperations.REPLACE:
+ return false;
+ case DataOperations.DELETE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
+ return false;
+ case DataOperations.OVERWRITE:
+ Preconditions.checkState(skipOverwrite,
+ "Cannot process overwrite snapshot : %s. Set read option %s to allow skipping snapshots of type overwrite",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_OVERWRITE_SNAPSHOTS);
+ return false;
+ default:
+ throw new IllegalStateException(String.format(
+ "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId()));
Review comment:
How about "Cannot process unknown snapshot operation: %s (snapshot id %s)"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794679811
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -200,13 +202,25 @@ public void stop() {
private boolean shouldProcess(Snapshot snapshot) {
String op = snapshot.operation();
- Preconditions.checkState(!op.equals(DataOperations.DELETE) || skipDelete,
- "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
- snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
- Preconditions.checkState(
- op.equals(DataOperations.DELETE) || op.equals(DataOperations.APPEND) || op.equals(DataOperations.REPLACE),
- "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId());
- return op.equals(DataOperations.APPEND);
+ switch (op) {
+ case DataOperations.APPEND:
+ return true;
+ case DataOperations.REPLACE:
+ return false;
+ case DataOperations.DELETE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
Review comment:
I don't see a need to change the error message here. It was short and to the point before. Can you revert the change to it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] tprelle commented on pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
tprelle commented on pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#issuecomment-967132303
Hi @SreeramGarlapati @kbendick,
I have some concern to link Overwrite and Delete because in table v1 so Copy on Write mode. A merge with an update will create a overwrite snapshot. So if you skip overwrite snapshot you will miss all theses changes.
For example delta.io allow user to choose to resend everything if it's copy on write mode.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794813758
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -200,13 +202,25 @@ public void stop() {
private boolean shouldProcess(Snapshot snapshot) {
String op = snapshot.operation();
- Preconditions.checkState(!op.equals(DataOperations.DELETE) || skipDelete,
- "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
- snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
- Preconditions.checkState(
- op.equals(DataOperations.DELETE) || op.equals(DataOperations.APPEND) || op.equals(DataOperations.REPLACE),
- "Cannot process %s snapshot: %s", op.toLowerCase(Locale.ROOT), snapshot.snapshotId());
- return op.equals(DataOperations.APPEND);
+ switch (op) {
+ case DataOperations.APPEND:
+ return true;
+ case DataOperations.REPLACE:
+ return false;
+ case DataOperations.DELETE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process delete snapshot: %s, to ignore deletes, set %s=true.",
Review comment:
Indentation isn't correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#issuecomment-1024572860
Merged. Thanks @SreeramGarlapati and @rajarshisarkar!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#issuecomment-1020815246
Hi @rdblue @kbendick @RussellSpitzer Request you to please review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r748908276
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
Review comment:
I don't think that this should use the same configuration to skip deletes and overwrites. Overwrites are different and I think that we should at a minimum have a different property. I would also prefer to have some additional clarity on how we plan to eventually handle this. We could skip overwrites, but what about use cases where they are probably upserts? What about when they're created by copy-on-write MERGE operations?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r763695262
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process overwrite snapshot : %s. Set read option %s to allow skipping snapshots of type overwrite",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
+ return false;
default:
Review comment:
I have added the Unit Test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r764674578
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process overwrite snapshot : %s. Set read option %s to allow skipping snapshots of type overwrite",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
+ return false;
default:
Review comment:
I have added the Unit Test.
##########
File path: spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java
##########
@@ -205,6 +205,11 @@ private boolean shouldProcess(Snapshot snapshot) {
"Cannot process delete snapshot : %s. Set read option %s to allow skipping snapshots of type delete",
snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
return false;
+ case DataOperations.OVERWRITE:
+ Preconditions.checkState(skipDelete,
+ "Cannot process overwrite snapshot : %s. Set read option %s to allow skipping snapshots of type overwrite",
+ snapshot.snapshotId(), SparkReadOptions.STREAMING_SKIP_DELETE_SNAPSHOTS);
+ return false;
default:
Review comment:
I have added the Unit Test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rajarshisarkar commented on a change in pull request #3517: Skip processing snapshots of type Overwrite during readStream
Posted by GitBox <gi...@apache.org>.
rajarshisarkar commented on a change in pull request #3517:
URL: https://github.com/apache/iceberg/pull/3517#discussion_r794697737
##########
File path: spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java
##########
@@ -481,6 +481,35 @@ public void testReadStreamWithSnapshotTypeDeleteAndSkipDeleteOption() throws Exc
.containsExactlyInAnyOrderElementsOf(Iterables.concat(dataAcrossSnapshots));
}
+ @Test
+ public void testReadStreamWithSnapshotTypeDeleteAndSkipOverwriteOption() throws Exception {
+ table.updateSpec()
+ .removeField("id_bucket")
+ .addField(ref("id"))
+ .commit();
+
+ table.refresh();
Review comment:
I have removed the refresh call.
##########
File path: spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java
##########
@@ -481,6 +481,35 @@ public void testReadStreamWithSnapshotTypeDeleteAndSkipDeleteOption() throws Exc
.containsExactlyInAnyOrderElementsOf(Iterables.concat(dataAcrossSnapshots));
}
+ @Test
+ public void testReadStreamWithSnapshotTypeDeleteAndSkipOverwriteOption() throws Exception {
+ table.updateSpec()
+ .removeField("id_bucket")
+ .addField(ref("id"))
+ .commit();
+
+ table.refresh();
+
+ // fill table with some data
+ List<List<SimpleRecord>> dataAcrossSnapshots = TEST_DATA_MULTIPLE_SNAPSHOTS;
+ appendDataAsMultipleSnapshots(dataAcrossSnapshots);
+
+ table.refresh();
Review comment:
I have removed the refresh call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org