Skip to content

Revert "Improve Iceberg deletes when an entire file can be removed"#13485

Merged
findepi merged 2 commits intotrinodb:masterfrom
alexjo2144:iceberg/revert-whole-file-deletion
Aug 11, 2022
Merged

Revert "Improve Iceberg deletes when an entire file can be removed"#13485
findepi merged 2 commits intotrinodb:masterfrom
alexjo2144:iceberg/revert-whole-file-deletion

Conversation

@alexjo2144
Copy link
Copy Markdown
Member

@alexjo2144 alexjo2144 commented Aug 3, 2022

Description

This reverts commit 9ddaa60.

Including both a RowDelta and a DeleteFiles commit in the same Iceberg transaction
results in the table history being cleared. Instead only write RowDeltas,
even in the case where a file can be fully removed.

Is this change a fix, improvement, new feature, refactoring, or other?

Fix

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

Iceberg connector

How would you describe this change to a non-technical end user or system administrator?

Ensures Trino does not clear Iceberg table history when it's not expected.

Related issues, pull requests, and links

Quick fix for: #12843

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# Iceberg
* Fix delete transactions to ensure table history remain intact.

Fixes #12843

@cla-bot cla-bot bot added the cla-signed label Aug 3, 2022
@alexjo2144 alexjo2144 requested a review from findepi August 3, 2022 17:04
@alexjo2144
Copy link
Copy Markdown
Member Author

@findepi added a regression test

@findepi
Copy link
Copy Markdown
Member

findepi commented Aug 4, 2022

@alexjo2144 please see build failures

@alexjo2144 alexjo2144 force-pushed the iceberg/revert-whole-file-deletion branch 3 times, most recently from 6dc9cc9 to b9bf619 Compare August 9, 2022 19:32
This reverts commit 9ddaa60.

Including both a RowDelta and a DeleteFiles commit in the same Iceberg transaction
results in the table history being cleared. Instead only write RowDeltas,
even in the case where a file can be fully removed.
@alexjo2144 alexjo2144 force-pushed the iceberg/revert-whole-file-deletion branch from b9bf619 to cc786ae Compare August 9, 2022 21:31
@alexjo2144
Copy link
Copy Markdown
Member Author

The changes introduced by Merge made this more complex than a simple revert, but should be passing now.

@alexjo2144 alexjo2144 force-pushed the iceberg/revert-whole-file-deletion branch from cc786ae to 7ad6caf Compare August 10, 2022 16:29
@findepi
Copy link
Copy Markdown
Member

findepi commented Aug 10, 2022

Fixes #12843

FYI, added this in the description.

@findepi findepi merged commit 2815522 into trinodb:master Aug 11, 2022
@findepi findepi mentioned this pull request Aug 11, 2022
@github-actions github-actions bot added this to the 393 milestone Aug 11, 2022
@alexjo2144 alexjo2144 deleted the iceberg/revert-whole-file-deletion branch September 7, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Iceberg: DELETE erases table history

2 participants