Implement table changes function for delta lake #16205

homar · 2023-02-21T15:08:11Z

Description

Adds table_changes function to delta lake.
It allows reading CDF entries from table for which CDF is enabled.

This PR requires #15575 to be merged first.
Commits 5th and 6th adds some additional things to TableFunction implementation that I found necessary while working on table_changes function.

Additional context and related issues

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# delta
* Add table_changes function. ({issue}`issuenumber`)

martint · 2023-02-21T16:42:21Z

This shouldn’t need that other table functions PR. That PR is only needed for table function that take subqueries or descriptors as input.

It should be possible to model this function in the same way as the query pass-through functions that already exist in some connectors.

homar · 2023-02-21T20:08:59Z

This shouldn’t need that other table functions PR. That PR is only needed for table function that take subqueries or descriptors as input.

It should be possible to model this function in the same way as the query pass-through functions that already exist in some connectors.

Yes I know it should be possible to use query pass-through. We considered that but our conclusion was that if we went that way we could have ended up with a more complex code. Using #15575 allows to nicely encapsulate the function.

...main/java/io/trino/plugin/deltalake/function/tablechanges/DeltaLakeTableChangesFunction.java

findinpath · 2023-02-23T12:26:17Z

...e/src/main/java/io/trino/plugin/deltalake/function/tablechanges/TableChangesSplitSource.java

+        if (processedVersion <= endVersion) {
+            Path transactionLogDir = getTransactionLogDir(new Path(tableLocation));
+            try {
+                Optional<List<DeltaLakeTransactionLogEntry>> entriesFromJson = getEntriesFromJson(processedVersion, transactionLogDir, fileSystem);


What happens in case that the transaction log entry file is missing?
This may very well happen when delta.logRetentionDuration kicks in and the transaction log files before the last checkpoints get removed.

So if we request the changes from 100 to 200, but we have transaction log entries starting from 180 (which is the last checkpoint of the table) should we get 180-200 range ?

See #16192 which deals with this kind of situations for retrieving the $history of the table.

So if we request the changes from 100 to 200, but we have transaction log entries starting from 180 (which is the last checkpoint of the table) should we get 180-200 range ?

No, it should fail

alexjo2144

Just an initial skim. @homar since you have the function taking in the schema and table name I don't think you need the initial commits from #15575 can you remove them from the branch?

docs/src/main/sphinx/connector/delta-lake.rst

...main/java/io/trino/plugin/deltalake/function/tablechanges/DeltaLakeTableChangesFunction.java

...in/java/io/trino/plugin/deltalake/function/tablechanges/TableChangesFunctionTableHandle.java

alexjo2144 · 2023-02-23T21:55:22Z

...e/src/main/java/io/trino/plugin/deltalake/function/tablechanges/TableChangesSplitSource.java

+        if (processedVersion <= endVersion) {
+            Path transactionLogDir = getTransactionLogDir(new Path(tableLocation));
+            try {
+                Optional<List<DeltaLakeTransactionLogEntry>> entriesFromJson = getEntriesFromJson(processedVersion, transactionLogDir, fileSystem);


So if we request the changes from 100 to 200, but we have transaction log entries starting from 180 (which is the last checkpoint of the table) should we get 180-200 range ?

No, it should fail

homar · 2023-02-25T10:49:26Z

Just an initial skim. @homar since you have the function taking in the schema and table name I don't think you need the initial commits from #15575 can you remove them from the branch?

Not entirely true, I still need at least some of them. For example my io.trino.plugin.deltalake.function.tablechanges.TableChangesFunctionProcessor relies on io.trino.spi.ptf.TableFunctionSplitProcessor that was introduced in 5ceef49
In general I am using this paradigm Execution by operator for table functions without sources.
Which was explicitly introduced in following commits 5ceef49 and 300eb25
But these commits relies on previous changes so I need all of them ;)

homar · 2023-04-29T11:44:46Z

just a rebase

homar · 2023-04-29T15:14:52Z

CI: #17295

mosabua

I reviewed the replacement docs PR that is linked now. Taking off the request changes here and leaving that up to other maintainers to discuss.

Manfred supposed to dismiss this per his last comment

findepi · 2023-05-02T13:10:50Z

@homar please make sure there are tests for table_changes for tables having row fields exercising reading whole row columns, but also dereferencing particular fields. see conversation: https://github.com/trinodb/trino/pull/17085/files#r1176399103
cc @krvikash

findepi · 2023-05-04T13:02:14Z

@homar please rebase to resolve conflict
also note that delta tests aren't green

martint

@mosabua there is no new syntax. this is a connector-level feature.

@findepi, any changes to the language or function library (syntax or semantics) qualifies.

@homar, please update per that discussion.

homar · 2023-05-05T14:22:56Z

@findepi test added, see io.trino.plugin.deltalake.TestDeltaLakeConnectorTest#testReadChangesOnTableWithRowColumn
@martint label added, what else do I need to do ?

martint · 2023-05-06T05:35:13Z

@homar, I meant, update the PR based on the discussion above to make the argument optional instead of giving -1 a special meaning.

homar · 2023-05-08T10:28:24Z

@homar, I meant, update the PR based on the discussion above to make the argument optional instead of giving -1 a special meaning.

@martint that is done, can we now proceed with merging this ?

Comments addressed

findepi · 2023-05-10T12:51:50Z

...ake/src/main/java/io/trino/plugin/deltalake/functions/tablechanges/TableChangesFunction.java

+                .filter(column -> column.getColumnType() != SYNTHESIZED)
+                .collect(toImmutableList());
+        accessControl.checkCanSelectFromColumns(null, schemaTableName, columnHandles.stream()
+                .map(DeltaLakeColumnHandle::getBaseColumnName)


getColumnName

findepi · 2023-05-10T12:51:53Z

...ake/src/main/java/io/trino/plugin/deltalake/functions/tablechanges/TableChangesFunction.java

+
+        ImmutableList.Builder<Descriptor.Field> outputFields = ImmutableList.builder();
+        columnHandles.stream()
+                .map(columnHandle -> new Descriptor.Field(columnHandle.getBaseColumnName(), Optional.of(columnHandle.getBaseType())))


getBaseColumnName -> getColumnName

getBaseType -> getType, new method

@JsonIgnore public Type getType() { return projectionInfo.map(DeltaLakeColumnProjectionInfo::getType) .orElse(baseType); }

findepi · 2023-05-10T12:57:52Z

force push https://github.com/trinodb/trino/compare/7665f6332438f2f8af1891a1ee30b94c87b72d73..946d31c37fb4f46b34371eb9b339d589fca5ddf4 combining rebase and changes made it hard to see what changed, but i tried to fish out the relevant part. i could have missed some things though

table_changes allows reading cdf entries stream. since_version is optional and exclusive. If since_version is not provided entire history of the table will be read.

homar · 2023-05-12T07:56:33Z

CI: #17158

findepi · 2023-05-12T08:24:30Z

CI: #17158

and #17472 ?

colebow · 2023-05-17T17:45:54Z

@homar @findepi we need docs for this - @homar, would you be willing to submit a follow-up PR for that?

homar · 2023-05-17T19:58:12Z

@homar @findepi we need docs for this - @homar, would you be willing to submit a follow-up PR for that?

@colebow The PR is there #17252

cla-bot bot added the cla-signed label Feb 21, 2023

homar requested review from kasiafi and findinpath February 21, 2023 15:08

homar assigned alexjo2144 and unassigned alexjo2144 Feb 21, 2023

homar requested review from alexjo2144 and findepi February 21, 2023 15:08

homar mentioned this pull request Feb 21, 2023

Support execution by operator for table functions #15575

Merged

github-actions bot added the docs label Feb 21, 2023

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch 2 times, most recently from 5af6d0f to 63e654b Compare February 22, 2023 08:51

findinpath reviewed Feb 23, 2023

View reviewed changes

alexjo2144 reviewed Feb 23, 2023

View reviewed changes

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch from 63e654b to d10f71c Compare February 25, 2023 12:51

github-actions bot added the tests:hive label Feb 25, 2023

homar requested review from findinpath and alexjo2144 February 25, 2023 13:02

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch 4 times, most recently from 18e1759 to eddba0d Compare February 27, 2023 10:47

homar mentioned this pull request Feb 27, 2023

Add test for vacuum with CDF files #15663

Closed

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch from eddba0d to 9f56faf Compare March 3, 2023 10:47

github-actions bot added bigquery BigQuery connector delta-lake Delta Lake connector mongodb MongoDB connector labels Mar 3, 2023

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch from 9f56faf to ab94a16 Compare March 3, 2023 13:33

mosabua reviewed May 1, 2023

View reviewed changes

martint previously requested changes May 4, 2023

View reviewed changes

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch from 6b1cc6c to 25e6998 Compare May 5, 2023 14:21

homar added the syntax-needs-review label May 5, 2023

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch 2 times, most recently from 44bd539 to 7665f63 Compare May 8, 2023 09:05

martint removed the syntax-needs-review label May 8, 2023

Extract canonicalizePartitionValues to TransactionLogUtil

a703569

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch from 7665f63 to 946d31c Compare May 9, 2023 14:11

findepi reviewed May 10, 2023

View reviewed changes

Add table_changes function for delta lake

80c52ad

table_changes allows reading cdf entries stream. since_version is optional and exclusive. If since_version is not provided entire history of the table will be read.

homar force-pushed the homar/implement_table_changes_function_for_delta_lake branch from 946d31c to 80c52ad Compare May 11, 2023 10:08

findepi merged commit 4cf66ff into trinodb:master May 12, 2023
90 of 92 checks passed

github-actions bot added this to the 418 milestone May 12, 2023

colebow mentioned this pull request May 17, 2023

Add Trino 418 release notes #17540

Merged

alexjo2144 mentioned this pull request Jun 9, 2023

Support Iceberg Changelog Scan #17154

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement table changes function for delta lake #16205

Implement table changes function for delta lake #16205

homar commented Feb 21, 2023 •

edited

martint commented Feb 21, 2023

homar commented Feb 21, 2023 •

edited

findinpath Feb 23, 2023

findinpath Feb 23, 2023

alexjo2144 Feb 23, 2023

alexjo2144 left a comment

alexjo2144 Feb 23, 2023

homar commented Feb 25, 2023

homar commented Apr 29, 2023

homar commented Apr 29, 2023

mosabua left a comment

findepi commented May 2, 2023

findepi commented May 4, 2023

martint left a comment

homar commented May 5, 2023

martint commented May 6, 2023

homar commented May 8, 2023

findepi May 10, 2023

findepi May 10, 2023

findepi commented May 10, 2023

homar commented May 12, 2023

findepi commented May 12, 2023

colebow commented May 17, 2023

homar commented May 17, 2023 •

edited

Implement table changes function for delta lake #16205

Implement table changes function for delta lake #16205

Conversation

homar commented Feb 21, 2023 • edited

Description

Additional context and related issues

Release notes

martint commented Feb 21, 2023

homar commented Feb 21, 2023 • edited

findinpath Feb 23, 2023

Choose a reason for hiding this comment

findinpath Feb 23, 2023

Choose a reason for hiding this comment

alexjo2144 Feb 23, 2023

Choose a reason for hiding this comment

alexjo2144 left a comment

Choose a reason for hiding this comment

alexjo2144 Feb 23, 2023

Choose a reason for hiding this comment

homar commented Feb 25, 2023

homar commented Apr 29, 2023

homar commented Apr 29, 2023

mosabua left a comment

Choose a reason for hiding this comment

findepi commented May 2, 2023

findepi commented May 4, 2023

martint left a comment

Choose a reason for hiding this comment

homar commented May 5, 2023

martint commented May 6, 2023

homar commented May 8, 2023

findepi May 10, 2023

Choose a reason for hiding this comment

findepi May 10, 2023

Choose a reason for hiding this comment

findepi commented May 10, 2023

homar commented May 12, 2023

findepi commented May 12, 2023

colebow commented May 17, 2023

homar commented May 17, 2023 • edited

homar commented Feb 21, 2023 •

edited

homar commented Feb 21, 2023 •

edited

homar commented May 17, 2023 •

edited