-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spark: Rewrite V2 deletes to V3 DVs #12250
base: main
Are you sure you want to change the base?
Conversation
48f9fb6
to
d2910f8
Compare
...rk/src/main/java/org/apache/iceberg/spark/actions/RewritePositionDeleteFilesSparkAction.java
Outdated
Show resolved
Hide resolved
@@ -213,45 +217,61 @@ static class PositionDeletesWriterFactory implements DataWriterFactory { | |||
this.writeProperties = writeProperties; | |||
} | |||
|
|||
private Table underlyingTable(Table table) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little concerned here because it appears this has two negative impacts:
- This results in a distributed load of the table metadata
- This relies on loading from the metadata file location directly, which we're trying to remove from other actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm a bit confused why we couldn't just leverage the existing table broadcast? If it's a different type of Table
, does broadcasting the underlying table work instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this so that we fetch the format version not on the executors but rather earlier
37eb0d2
to
95ff7d4
Compare
95ff7d4
to
4b0819a
Compare
No description provided.