Skip to content
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: Merge conflicting deletion vectors #11693

Closed

Conversation

amogh-jahagirdar
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar commented Dec 3, 2024

This PR adds the ability to merge conflicting deletion vectors for a given data file instead of failing the commit. In parallel, every conflicting DV will be merged with the committed DV, and a new Puffin with the merged DV will be output per datafile which had conflicting DVs.
This PR also takes care of any tables that wrote multiple DVs for a given data file (which is not spec compliant) by opportunistically merging those as well

@github-actions github-actions bot added the core label Dec 3, 2024
@amogh-jahagirdar amogh-jahagirdar marked this pull request as ready for review December 11, 2024 16:10
* @param spec the data file partition spec
* @param partition the data file partition
*/
default void delete(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also make this interface take in a List positions (kind of like a batch delete API on the writer) but it felt more natural to just pass in the index imo, cc @aokolnychyi @nastra thoughts?

Comment on lines +94 to +98
public static Builder builderFor(
TableOperations ops, PartitionSpec spec, FileFormat format, int partitionId, long taskId) {
return new Builder(ops, spec, format, partitionId, taskId);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed a way to create an output file factory from table operations since that's all that exists in MergingSnapshotProducer, this is public so we need to be careful.

Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jan 11, 2025
Copy link

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Jan 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant