Add Delta Lake $properties system table #17592

jkylling · 2023-05-22T13:29:23Z

Description

Adds a $properties system table for Delta Lake, similar to what exists for Iceberg.

The main motivation for adding the $properties system table is to be able to test extra_properties which will be added as part of #17428

Fixes #17294

Release notes

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

# Delta Lake 
* Add `$properties` system table which can be queried to inspect Delta Lake table properties. ({issue}`17294`)

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

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

...n/java/io/trino/tests/product/deltalake/TestDeltaLakeDatabricksCreateTableCompatibility.java

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

ebyhr

Please fix the commit granularity. https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#git-merge-strategy

...no-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DeltaLakeTestUtils.java

plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSystemTables.java

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

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

jkylling · 2023-05-23T16:10:23Z

The failing product test seems to be unrelated. The test io.trino.tests.product.deltalake.TestDeltaLakeColumnMappingMode.testColumnMappingModeNameAddColumn fails because of

io.trino.tempto.query.QueryExecutionException: java.sql.SQLException: Query failed (#20221125_205454_00169_t7w8x): Error opening Hive split s3://trino-ci-test/databricks-compatibility-test-test_dl_column_mapping_mode_add_co
lumn_3cay48eeem/another_varchar=new%20column/part-00000-ca7d4a38-5908-4690-98cd-c627d12f7bda.c000.snappy.parquet (offset=0, length=470): com.amazonaws.services.s3.model.AmazonS3Exception: The specified key does not exist. (Service: Amazon S3; Status Code: 404; Error Code: N
oSuchKey; Request ID: 172AEF45AE130D2C; S3 Extended Request ID: 10f54869-4c1f-453c-af65-14792dd45501; Proxy: null), S3 Extended Request ID: 10f54869-4c1f-453c-af65-14792dd45501 (Path: s3://trino-ci-test/databricks-compatibility-test-test_dl_column_mapping_mode_add_column_3c
ay48eeem/another_varchar=new%20column/part-00000-ca7d4a38-5908-4690-98cd-c627d12f7bda.c000.snappy.parquet)

...no-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DeltaLakeTestUtils.java

ebyhr · 2023-05-24T01:25:42Z

Please check CI failure.

jkylling · 2023-05-26T07:40:41Z

@ebyhr CI is green for this PR again. Would you have time for another review?

...no-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DeltaLakeTestUtils.java

findinpath · 2023-06-04T17:08:12Z

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

+``$properties`` table
+~~~~~~~~~~~~~~~~~~~~~
+
+The ``$properties`` table provides access to general information about Delta


I'd recommend keeping things simple and mention that the table gives access to the properties names and values of the Delta Lake table. See https://trino.io/docs/current/connector/hive.html#metadata-tables

I've changed the wording slightly to distinguish between table configuration (minReaderVersion, etc.), table features (items of the features field of protocol entries), and table properties (custom properties). Arguably this is an implementation detail of Delta. Open to other ways to phrase this.

plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSystemTables.java

...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeTablePropertiesCompatibility.java

findinpath · 2023-06-05T21:03:44Z

...oduct-tests/src/main/java/io/trino/tests/product/deltalake/TestHiveAndDeltaLakeRedirect.java

@@ -877,6 +877,24 @@ public void testViewReferencingHiveAndDeltaTable(boolean legacyHiveViewTranslati
        }
    }

+    @Test(groups = {DELTA_LAKE_DATABRICKS, DELTA_LAKE_OSS, PROFILE_SPECIFIC_TESTS})
+    @Flaky(issue = DATABRICKS_COMMUNICATION_FAILURE_ISSUE, match = DATABRICKS_COMMUNICATION_FAILURE_MATCH)
+    public void testDeltaToHivePropertiesRedirect()


This functionality has worked also before this PR.

We should be testing the other way around

test_redirect_to_delta_properties

Modified both tests to verify redirection to the Delta table $properties table.

findinpath

LGTM % comment

findinpath · 2023-06-06T05:13:43Z

spark-sql> alter table tiny.table1 set tblproperties (delta.minReaderVersion = 1, delta.minWriterVersion = 2, someStringProperty = 'someStringValue');

trino:tiny> select * from "table1$properties";
          key           |      value      
------------------------+-----------------
 someStringProperty     | someStringValue 
 delta.minReaderVersion | 1               
 delta.minWriterVersion | 2               
(3 rows)

It is worth showcasing that the $properties table can read properties names and values with case sensitive.

ebyhr · 2023-06-27T05:11:17Z

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

+                ImmutableList.<ColumnMetadata>builder()
+                        .add(new ColumnMetadata("key", VARCHAR))
+                        .add(new ColumnMetadata("value", VARCHAR))
+                        .build());


Let's move this list of ColumnMetadata to the constant.

ebyhr · 2023-06-28T07:18:48Z

...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeTablePropertiesCompatibility.java

+import java.util.List;
+
+import static io.trino.tempto.assertions.QueryAssert.Row.row;
+import static io.trino.tempto.assertions.QueryAssert.assertThat;


This method is deprecated. Please use AssertJ's assertThat.

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

...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeTablePropertiesCompatibility.java

ebyhr · 2023-06-28T10:23:33Z

/test-with-secrets sha=184162cedd6b5ca986deca803ee472f5c1d35222

github-actions · 2023-06-28T11:11:29Z

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/5400162270

ebyhr · 2023-06-28T13:23:44Z

Please fix CI failure with secrets.

jkylling · 2023-06-28T16:22:19Z

This is some matrix of failures to fix:

suite-delta-lake-databricks73

org.apache.spark.sql.AnalysisException: Unknown configuration was specified: delta.minWriterVersion

 java.lang.AssertionError: 
2023-06-28T12:04:09.2752389Z tests               | Expecting actual:
2023-06-28T12:04:09.2752797Z tests               |   [["Foo", "Baz"],
2023-06-28T12:04:09.2753191Z tests               |     ["foo", "bar"],
2023-06-28T12:04:09.2753624Z tests               |     ["delta.minReaderVersion", "1"],
2023-06-28T12:04:09.2754120Z tests               |     ["delta.minWriterVersion", "2"]]
2023-06-28T12:04:09.2754594Z tests               | to contain exactly in any order:
2023-06-28T12:04:09.2755119Z tests               |   [["Type", "EXTERNAL"], ["Foo", "Baz"], ["foo", "bar"]]
2023-06-28T12:04:09.2755571Z tests               | elements not found:
2023-06-28T12:04:09.2755990Z tests               |   [["Type", "EXTERNAL"]]
2023-06-28T12:04:09.2756405Z tests               | and elements not expected:
2023-06-28T12:04:09.2756949Z tests               |   [["delta.minReaderVersion", "1"], ["delta.minWriterVersion", "2"]]

suite-delta-lake-databricks91

java.lang.IllegalArgumentException: requirement failed: delta.minWriterVersion needs to be an integer between [1, 5].

    | java.lang.AssertionError: 
2023-06-28T12:28:38.9760862Z tests               | Expecting actual:
2023-06-28T12:28:38.9761191Z tests               |   [["Foo", "Baz"],
2023-06-28T12:28:38.9761506Z tests               |     ["foo", "bar"],
2023-06-28T12:28:38.9761856Z tests               |     ["delta.minReaderVersion", "1"],
2023-06-28T12:28:38.9762236Z tests               |     ["delta.minWriterVersion", "2"]]
2023-06-28T12:28:38.9762608Z tests               | to contain exactly in any order:
2023-06-28T12:28:38.9762959Z tests               |   [["Type", "EXTERNAL"],
2023-06-28T12:28:38.9763275Z tests               |     ["Foo", "Baz"],
2023-06-28T12:28:38.9763610Z tests               |     ["delta.minWriterVersion", "2"],
2023-06-28T12:28:38.9764090Z tests               |     ["delta.minReaderVersion", "1"],
2023-06-28T12:28:38.9764410Z tests               |     ["foo", "bar"]]
2023-06-28T12:28:38.9764761Z tests               | but could not find the following elements:
2023-06-28T12:28:38.9765122Z tests               |   [["Type", "EXTERNAL"]]

suite-delta-lake-databricks104

java.lang.IllegalArgumentException: requirement failed: delta.minWriterVersion needs to be an integer between [1, 6].

     | Expecting actual:
2023-06-28T12:32:26.3006668Z tests               |   [["Foo", "Baz"],
2023-06-28T12:32:26.3007054Z tests               |     ["foo", "bar"],
2023-06-28T12:32:26.3007494Z tests               |     ["delta.minReaderVersion", "1"],
2023-06-28T12:32:26.3007990Z tests               |     ["delta.minWriterVersion", "2"]]
2023-06-28T12:32:26.3008451Z tests               | to contain exactly in any order:
2023-06-28T12:32:26.3008865Z tests               |   [["Foo", "Baz"],
2023-06-28T12:32:26.3009224Z tests               |     ["Type", "EXTERNAL"],
2023-06-28T12:32:26.3009657Z tests               |     ["delta.minReaderVersion", "1"],
2023-06-28T12:32:26.3010285Z tests               |     ["delta.minWriterVersion", "2"],
2023-06-28T12:32:26.3010705Z tests               |     ["foo", "bar"]]
2023-06-28T12:32:26.3011158Z tests               | but could not find the following elements:
2023-06-28T12:32:26.3011607Z tests               |   [["Type", "EXTERNAL"]]

suite-delta-lake-databricks113

delta.minWriterVersion needs to be an integer between [1, 6].

jkylling · 2023-06-28T16:26:31Z

@ebyhr I'm leaning towards excluding version 73, 91, 104 and 113 from the new product tests? I'll make this change. We could potentially refactor the tests to test less, or be version specific.

findinpath · 2023-06-30T05:01:37Z

~~@jkylling excluding the tests will likely hide potential issues in Trino.~~

Update 05.07.2023

I'm leaning towards excluding version 73, 91, 104 and 113 from the new product tests.

On a second thought, the change makes sense. We're testing actually Trino for accuracy and not Databricks

findinpath · 2023-07-05T16:06:31Z

@ebyhr can you please trigger the build with secrets?

ebyhr · 2023-07-05T23:49:47Z

/test-with-secrets sha=b00b09f7dc6b2cd0818696bb8f9ad19ea419fe06

github-actions · 2023-07-06T05:24:56Z

The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/5470965065

findinpath · 2023-07-06T09:50:07Z

@ebyhr could you please do another check whether this PR is good to land?

...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeTablePropertiesCompatibility.java

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

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

ebyhr · 2023-07-18T08:14:34Z

/test-with-secrets sha=9f1b159d2c75485d8dfe81cfe1a895db7c3cef34

github-actions · 2023-07-18T08:17:52Z

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/5584978296

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

jkylling added the delta-lake Delta Lake connector label May 22, 2023

jkylling requested a review from findinpath May 22, 2023 13:29

github-actions bot added the tests:hive label May 22, 2023

jkylling mentioned this pull request May 22, 2023

Add extra_properties to Delta Lake table properties #17595

Closed

ebyhr self-requested a review May 22, 2023 23:14

ebyhr reviewed May 23, 2023

View reviewed changes

jkylling requested a review from ebyhr May 23, 2023 08:58

github-actions bot added docs labels May 23, 2023

ebyhr reviewed May 23, 2023

View reviewed changes

jkylling force-pushed the delta-lake-properties-table branch 2 times, most recently from 7c36524 to e5b8e68 Compare May 23, 2023 13:20

jkylling requested a review from ebyhr May 23, 2023 16:10

ebyhr reviewed May 24, 2023

View reviewed changes

...no-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DeltaLakeTestUtils.java Outdated Show resolved Hide resolved

jkylling force-pushed the delta-lake-properties-table branch 2 times, most recently from c443e16 to a9112b2 Compare May 24, 2023 10:04

jkylling requested a review from ebyhr May 24, 2023 13:26

findinpath reviewed Jun 4, 2023

View reviewed changes

...no-product-tests/src/main/java/io/trino/tests/product/deltalake/util/DeltaLakeTestUtils.java Outdated Show resolved Hide resolved

findinpath reviewed Jun 4, 2023

View reviewed changes

plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeSystemTables.java Outdated Show resolved Hide resolved

findinpath reviewed Jun 4, 2023

View reviewed changes

...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeTablePropertiesCompatibility.java Outdated Show resolved Hide resolved

jkylling force-pushed the delta-lake-properties-table branch from a9112b2 to 8cd28fd Compare June 5, 2023 18:05

findinpath reviewed Jun 5, 2023

View reviewed changes

findinpath approved these changes Jun 5, 2023

View reviewed changes

jkylling force-pushed the delta-lake-properties-table branch from 8cd28fd to d20f38a Compare June 7, 2023 18:43

ebyhr approved these changes Jun 28, 2023

View reviewed changes

jkylling force-pushed the delta-lake-properties-table branch from b7c19d8 to d29c4e4 Compare June 28, 2023 09:31

ebyhr reviewed Jun 28, 2023

View reviewed changes

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

...rc/main/java/io/trino/tests/product/deltalake/TestDeltaLakeTablePropertiesCompatibility.java Outdated Show resolved Hide resolved

jkylling force-pushed the delta-lake-properties-table branch from d29c4e4 to 184162c Compare June 28, 2023 10:16

jkylling force-pushed the delta-lake-properties-table branch from 184162c to b00b09f Compare June 28, 2023 16:34

jkylling requested a review from ebyhr June 29, 2023 10:03

ebyhr mentioned this pull request Jul 7, 2023

Simplify getRawSystemTable in Delta Lake #18157

Merged

ebyhr reviewed Jul 9, 2023

View reviewed changes

jkylling force-pushed the delta-lake-properties-table branch from b00b09f to 214d759 Compare July 17, 2023 20:37

jkylling requested a review from ebyhr July 17, 2023 20:37

ebyhr approved these changes Jul 17, 2023

View reviewed changes

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 Show resolved Hide resolved

jkylling force-pushed the delta-lake-properties-table branch from 214d759 to 9f1b159 Compare July 18, 2023 08:08

jkylling requested a review from ebyhr July 18, 2023 08:09

Add Delta Lake $properties system table

3c61b82

jkylling force-pushed the delta-lake-properties-table branch from 9f1b159 to 3c61b82 Compare July 19, 2023 08:26

ebyhr merged commit 1e188a1 into trinodb:master Jul 19, 2023

github-actions bot added this to the 423 milestone Jul 19, 2023

colebow mentioned this pull request Aug 2, 2023

Add Trino 423 release notes #18496

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Delta Lake $properties system table #17592

Add Delta Lake $properties system table #17592

jkylling commented May 22, 2023 •

edited by ebyhr

Loading

ebyhr left a comment

jkylling commented May 23, 2023

ebyhr commented May 24, 2023

jkylling commented May 26, 2023

findinpath Jun 4, 2023

jkylling Jun 7, 2023

findinpath Jun 5, 2023

jkylling Jun 7, 2023

findinpath left a comment

findinpath commented Jun 6, 2023

ebyhr Jun 27, 2023

ebyhr Jun 28, 2023

ebyhr commented Jun 28, 2023

github-actions bot commented Jun 28, 2023 •

edited

Loading

ebyhr commented Jun 28, 2023

jkylling commented Jun 28, 2023

jkylling commented Jun 28, 2023

findinpath commented Jun 30, 2023 •

edited

Loading

findinpath commented Jul 5, 2023

ebyhr commented Jul 5, 2023

github-actions bot commented Jul 6, 2023

findinpath commented Jul 6, 2023

ebyhr commented Jul 18, 2023

github-actions bot commented Jul 18, 2023 •

edited

Loading

Add Delta Lake $properties system table #17592

Add Delta Lake $properties system table #17592

Conversation

jkylling commented May 22, 2023 • edited by ebyhr Loading

Description

Release notes

ebyhr left a comment

Choose a reason for hiding this comment

jkylling commented May 23, 2023

ebyhr commented May 24, 2023

jkylling commented May 26, 2023

findinpath Jun 4, 2023

Choose a reason for hiding this comment

jkylling Jun 7, 2023

Choose a reason for hiding this comment

findinpath Jun 5, 2023

Choose a reason for hiding this comment

jkylling Jun 7, 2023

Choose a reason for hiding this comment

findinpath left a comment

Choose a reason for hiding this comment

findinpath commented Jun 6, 2023

ebyhr Jun 27, 2023

Choose a reason for hiding this comment

ebyhr Jun 28, 2023

Choose a reason for hiding this comment

ebyhr commented Jun 28, 2023

github-actions bot commented Jun 28, 2023 • edited Loading

ebyhr commented Jun 28, 2023

jkylling commented Jun 28, 2023

suite-delta-lake-databricks73

suite-delta-lake-databricks91

suite-delta-lake-databricks104

suite-delta-lake-databricks113

jkylling commented Jun 28, 2023

findinpath commented Jun 30, 2023 • edited Loading

findinpath commented Jul 5, 2023

ebyhr commented Jul 5, 2023

github-actions bot commented Jul 6, 2023

findinpath commented Jul 6, 2023

ebyhr commented Jul 18, 2023

github-actions bot commented Jul 18, 2023 • edited Loading

jkylling commented May 22, 2023 •

edited by ebyhr

Loading

github-actions bot commented Jun 28, 2023 •

edited

Loading

findinpath commented Jun 30, 2023 •

edited

Loading

github-actions bot commented Jul 18, 2023 •

edited

Loading