Support reading deletion vectors in Delta Lake #17477

ebyhr · 2023-05-12T10:29:22Z

Description

Release notes

(x) Release notes are required, with the following suggested text:

# Delta Lake
* Support reading tables with deletion vectors. ({issue}`16903`)

...o-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/DeletionVectorEntry.java

...duct-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksDelete.java

findinpath

LGTM

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/util/DeletionVectors.java

...in/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/AddFileEntry.java

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/delete/DeletionVectors.java

...duct-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksDelete.java

alexjo2144

The changes relating to row_id are a little confusing to me here. Row id is used for write operations, specifically merge, but we're looking at read-only support here so I wouldn't expect the two to interact.

alexjo2144 · 2023-06-13T17:19:03Z

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/delete/DeletionVectors.java

+        int actualSize = inputStream.readInt();
+        if (actualSize != expectedSize) {
+            // TODO: Investigate why these size differ
+            log.warn("The size of deletion vector %s expects %s but got %s", inputFile.location(), expectedSize, actualSize);


Do you need to resize the array to the real size?

Unfortunately, resize doesn't help.

i don't think we can just ignore this.
it something to investigate. we should rather throw here, than risk correctness (if eg we read from wrong offset, or wrong number of bytes)

The cause was a misuse of TrinoDataInputStream. Switching to DataInputStream resolved the size difference.

this is very interesting. Can you elaborate on that @ebyhr?
do you know what are the situations where TrinoDataInputStream should be used and where it mustn't?

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/delete/DeletionVectors.java

ebyhr · 2023-07-04T05:08:57Z

Going to resolve confilcts.

...c/main/java/io/trino/plugin/deltalake/transactionlog/checkpoint/CheckpointEntryIterator.java

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeSplit.java

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/delete/DeletionVectors.java

...in/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakePageSourceProvider.java

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/delete/DeletionVectors.java

findepi · 2023-07-05T11:45:09Z

@ebyhr please split base85codec and roaringbitmap stuff to own prep PRs
this is the part i focused on first and want to merge it & remove from view

findepi · 2023-07-07T06:07:03Z

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/delete/DeletionVectors.java

+            return new UUID(highBits, lowBits);
+        }
+
+        // This method will be used when supporting https://github.com/trinodb/trino/issues/17063


ebyhr · 2023-07-12T10:22:19Z

Addressed comments partially. Let me take another look tomorrow.

findepi · 2023-07-12T11:08:28Z

Addressed comments partially. Let me take another look tomorrow.

are you planning on splitting this (per #17477 (comment)), or should i be reviewing this PR?

findepi · 2023-08-09T10:39:09Z

...n/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/TableSnapshot.java

@@ -158,6 +159,11 @@ public List<DeltaLakeTransactionLogEntry> getJsonTransactionLogEntries()
        return logTail.getFileEntries();
    }

+    public Map<Long, List<DeltaLakeTransactionLogEntry>> getJsonTransactionLogVersionAndEntries()


Do we rely on map ordering?

Updated activeAddEntries to use sorted(comparingByKey()).

i think we don't need to sort this, since it comes sorted. maybe we just keep it as a list of something
List<Transaction>
where "Transaction" has long transactionId and List<DeltaLakeTransactionLogEntry>?

findepi · 2023-08-09T10:39:42Z

...-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/TransactionLogAccess.java

-            }
+        for (Map.Entry<Long, List<DeltaLakeTransactionLogEntry>> deltaLakeTransactionLogEntries : jsonEntries.entrySet()) {
+            // Deletion vector registers both 'add' & 'remove' entries in any order. The 'add' entry should be kept.
+            Set<String> dependOnDeletionVector = new HashSet<>();


if we process removals before additions, can we remove dependOnDeletionVector set?

Updated to process removals first.

ebyhr · 2023-08-18T06:18:22Z

Rebased on master to resolve conflicts.

ebyhr · 2023-08-25T06:42:20Z

Rebased on master to resolve conflicts.

findinpath · 2023-08-30T08:07:11Z

suite-delta-lake-databricks122 timed out -> #18805

ebyhr · 2023-08-31T06:16:49Z

Rebased on master to resolve conflicts.

findinpath · 2023-09-03T18:18:04Z

I was curious whether the changes for dealing with on the Databricks product tests timeouts are effective and stumbled over this failure:

tests               | 2023-09-02 17:21:00 INFO: FAILURE     /    io.trino.tests.product.deltalake.TestDeltaLakeDeleteCompatibility.testDeletionVectors (Groups: profile_specific_tests, delta-lake-exclude-91, delta-lake-databricks, delta-lake-exclude-104, delta-lake-exclude-113, delta-lake-oss) took 18.2 seconds
tests               | 2023-09-02 17:21:00 SEVERE: Failure cause:
tests               | java.lang.AssertionError: Expected row count to be <3>, but was <4>; rows=[[0, CREATE TABLE], [1, WRITE], [2, DELETE], [3, OPTIMIZE]]

https://github.com/trinodb/trino/actions/runs/6057878380/job/16439570813

Apparently the OPTIMIZE is done in the background by Databricks.

ebyhr · 2023-09-05T23:21:47Z

@findepi @alexjo2144 Could you take another look when you have time?

findepi · 2023-09-12T13:49:41Z

cc @radek-starburst

ebyhr · 2023-09-12T13:56:27Z

(Just squashed commits into one)

cla-bot bot added the cla-signed label May 12, 2023

github-actions bot added delta-lake Delta Lake connector tests:hive labels May 12, 2023

ebyhr self-assigned this May 14, 2023

ebyhr force-pushed the ebi/delta-deletion-vectors branch 3 times, most recently from 57ed69a to 104ab4c Compare May 16, 2023 04:23

electrum reviewed May 16, 2023

View reviewed changes

...o-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/DeletionVectorEntry.java Outdated Show resolved Hide resolved

ebyhr marked this pull request as ready for review May 16, 2023 12:29

ebyhr force-pushed the ebi/delta-deletion-vectors branch 2 times, most recently from 7cb1ee1 to f65a908 Compare May 16, 2023 22:16

ebyhr requested review from alexjo2144 and findinpath May 16, 2023 22:18

ebyhr commented May 22, 2023

View reviewed changes

...duct-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksDelete.java Outdated Show resolved Hide resolved

ebyhr force-pushed the ebi/delta-deletion-vectors branch from f65a908 to b87932b Compare June 6, 2023 04:46

findinpath approved these changes Jun 8, 2023

View reviewed changes

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/util/DeletionVectors.java Outdated Show resolved Hide resolved

...in/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/transactionlog/AddFileEntry.java Show resolved Hide resolved

ebyhr force-pushed the ebi/delta-deletion-vectors branch from b87932b to 7cd6d32 Compare June 9, 2023 04:31

ebyhr requested a review from findepi June 9, 2023 04:32

findinpath reviewed Jun 9, 2023

View reviewed changes

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/delete/DeletionVectors.java Outdated Show resolved Hide resolved

findinpath reviewed Jun 9, 2023

View reviewed changes

...duct-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksDelete.java Outdated Show resolved Hide resolved

ebyhr force-pushed the ebi/delta-deletion-vectors branch from 7cd6d32 to 9de92c9 Compare June 13, 2023 08:04

alexjo2144 reviewed Jun 13, 2023

View reviewed changes

ebyhr force-pushed the ebi/delta-deletion-vectors branch from 9de92c9 to 865c973 Compare July 4, 2023 07:46

findepi reviewed Jul 5, 2023

View reviewed changes

findepi reviewed Jul 7, 2023

View reviewed changes

ebyhr force-pushed the ebi/delta-deletion-vectors branch from 865c973 to 0c6cff5 Compare July 12, 2023 10:21

findepi reviewed Aug 9, 2023

View reviewed changes

ebyhr force-pushed the ebi/delta-deletion-vectors branch 2 times, most recently from 41b37bb to a069fed Compare August 10, 2023 12:26

ebyhr force-pushed the ebi/delta-deletion-vectors branch from a069fed to 3291116 Compare August 18, 2023 06:18

ebyhr force-pushed the ebi/delta-deletion-vectors branch from 3291116 to 2f200a2 Compare August 25, 2023 06:42

ebyhr force-pushed the ebi/delta-deletion-vectors branch from 2f200a2 to a527e66 Compare August 25, 2023 06:53

ebyhr force-pushed the ebi/delta-deletion-vectors branch from a527e66 to 3e79fa1 Compare August 31, 2023 06:16

ebyhr force-pushed the ebi/delta-deletion-vectors branch 3 times, most recently from fb7d8e5 to 6f309df Compare September 1, 2023 03:09

ebyhr force-pushed the ebi/delta-deletion-vectors branch 2 times, most recently from 6295280 to f0f59bf Compare September 5, 2023 23:21

ebyhr force-pushed the ebi/delta-deletion-vectors branch 2 times, most recently from 08567c1 to 9637cd7 Compare September 6, 2023 09:16

findepi approved these changes Sep 12, 2023

View reviewed changes

Support reading deletion vectors in Delta Lake

0693b94

ebyhr force-pushed the ebi/delta-deletion-vectors branch from 9637cd7 to 0693b94 Compare September 12, 2023 13:55

ebyhr merged commit eb26565 into master Sep 12, 2023
3 of 12 checks passed

ebyhr deleted the ebi/delta-deletion-vectors branch September 12, 2023 13:56

github-actions bot added this to the 427 milestone Sep 12, 2023

colebow mentioned this pull request Sep 12, 2023

Add Trino 427 release notes #19023

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support reading deletion vectors in Delta Lake #17477

Support reading deletion vectors in Delta Lake #17477

ebyhr commented May 12, 2023

findinpath left a comment

alexjo2144 left a comment

alexjo2144 Jun 13, 2023

ebyhr Jun 15, 2023

findepi Jul 5, 2023

ebyhr Jul 12, 2023

findepi Jul 12, 2023

ebyhr commented Jul 4, 2023

findepi commented Jul 5, 2023

findepi Jul 7, 2023

ebyhr commented Jul 12, 2023

findepi commented Jul 12, 2023

findepi Aug 9, 2023

ebyhr Aug 10, 2023

findepi Aug 10, 2023

findepi Aug 9, 2023

ebyhr Aug 10, 2023

ebyhr commented Aug 18, 2023

ebyhr commented Aug 25, 2023

findinpath commented Aug 30, 2023

ebyhr commented Aug 31, 2023

findinpath commented Sep 3, 2023

ebyhr commented Sep 5, 2023

findepi commented Sep 12, 2023

ebyhr commented Sep 12, 2023

Support reading deletion vectors in Delta Lake #17477

Support reading deletion vectors in Delta Lake #17477

Conversation

ebyhr commented May 12, 2023

Description

Release notes

findinpath left a comment

Choose a reason for hiding this comment

alexjo2144 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebyhr commented Jul 4, 2023

findepi commented Jul 5, 2023

Choose a reason for hiding this comment

ebyhr commented Jul 12, 2023

findepi commented Jul 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebyhr commented Aug 18, 2023

ebyhr commented Aug 25, 2023

findinpath commented Aug 30, 2023

ebyhr commented Aug 31, 2023

findinpath commented Sep 3, 2023

ebyhr commented Sep 5, 2023

findepi commented Sep 12, 2023

ebyhr commented Sep 12, 2023