-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Core, Spark: Avoid manifest copies when importing data to V2 tables #8962
Core, Spark: Avoid manifest copies when importing data to V2 tables #8962
Conversation
a9ca63f
to
5780b82
Compare
* <p>By default, the manifest will be rewritten to assign all entries this update's snapshot ID. | ||
* In that case, it is always the responsibility of the caller to manage the lifecycle of the | ||
* original manifest. | ||
* <p>The manifest will be used directly if snapshot ID inheritance is enabled (all tables with |
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.
Simply clarifies the notion of snapshot ID inheritance and that it is always on in V2 tables.
5780b82
to
6d9d78b
Compare
fea275d
to
9b2b93a
Compare
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.
looks good to me, minor comment
* the commit fails, the manifest will never be deleted and it is up to the caller whether to | ||
* delete or reuse it. | ||
* <p>If the manifest is rewritten, it is always the responsibility of the caller to manage the | ||
* lifecycle of the original manifest. If manifest entries are allowed to inherit the snapshot ID |
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.
What do you think to simplify 'if manifest entries are allowed to inherit the snapshot ID...' to 'if the manifest is used directly'..?
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.
Yep, I liked it. Fixed.
2b5eed9
to
7f6a798
Compare
Thanks, @szehon-ho! |
This PR extends the idea from #8928 to
FastAppend
andMergeAppend
, which are used in data imports and migration.