-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Push down DELETE for enforceable filters on Delta Lake #18332
Push down DELETE for enforceable filters on Delta Lake #18332
Conversation
@@ -361,7 +361,7 @@ private int[] getWriterIndexes(Page page) | |||
partitionName = Optional.of(partName); | |||
} | |||
|
|||
String fileName = session.getQueryId() + "-" + randomUUID(); | |||
String fileName = session.getQueryId() + "_" + randomUUID(); |
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.
This change is in sync with
trino/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMergeSink.java
Line 333 in 20e5779
Location targetLocation = sourceLocation.parentDirectory().appendPath(session.getQueryId() + "_" + randomUUID()); |
I'd benefit from a suggestion in which utility class could i extract the method for creating the file name.
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.
You can always add a new one ;)
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
6261a7b
to
cb8a04f
Compare
As agreed with @findepi , it is rather unusual to deal with |
cb8a04f
to
958e634
Compare
...no-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeExecuteDeleteTableHandle.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
958e634
to
b791cd1
Compare
de468d5
to
31f088f
Compare
31f088f
to
fd8e62a
Compare
throw new TransactionConflictException(format("Conflicting concurrent writes found. Expected transaction log version: %s, actual version: %s", tableHandle.getReadVersion(), currentVersion)); | ||
} | ||
long commitVersion = currentVersion + 1; | ||
transactionLogWriter.appendCommitInfoEntry(getCommitInfoEntry(session, commitVersion, createdTime, MERGE_OPERATION, tableHandle.getReadVersion())); |
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.
delete operation?
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 just following the same pattern as the one used for UPDATE
/DELETE
/MERGE
statements in Delta Lake. See #15763
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java
Show resolved
Hide resolved
...in/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeFileOperations.java
Outdated
Show resolved
Hide resolved
...t-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDeleteCompatibility.java
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
e11f841
to
36b1b96
Compare
...in/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeFileOperations.java
Show resolved
Hide resolved
Rebasing on |
7c22b5b
to
12e8d36
Compare
@pajaks & @alexjo2144 AC. Please review again the PR. |
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java
Outdated
Show resolved
Hide resolved
...t-tests/src/main/java/io/trino/tests/product/deltalake/TestDeltaLakeDeleteCompatibility.java
Outdated
Show resolved
Hide resolved
12e8d36
to
c2128d4
Compare
Thank you everybody for the valuable feedback. |
adaf4e7
to
4dc908c
Compare
Perform DELETE only on the metadata of the Delta Lake table when the delete filter does not touch any data columns. Sample queries affected by this enhancement are: ``` DELETE FROM table DELETE FROM table WHERE true DELETE FROM partitioned_table WHERE part1=value1 AND part2=value2 DELETE FROM partitioned_table WHERE part2=value2 ``` Given that this operation is performed only on the metadata layer, when there are `add` file entries in the transaction log of the table without statistics containing number of records, then number of deleted records will not be returned.
4dc908c
to
29ba6d3
Compare
Pushed small changes to fix CI failure. |
Description
Perform DELETE only on the metadata of the Delta Lake
table when the delete filter does not touch any data
columns.
Sample queries affected by this enhancement are:
Fixes #18331
The operation will not be performed on the metadata layer when
there are
add
file entries in the transaction log of the tablewithout statistics containing number of records.
Additional context and related issues
Without
DELETE
pushdownWith
DELETE
pushdowntldr; before 103 s after 1.8 s
Release notes
(x) Release notes are required, with the following suggested text: