Implement CREATE OR REPLACE TABLE for delta lake connector #19991

Praveen2112 · 2023-12-01T12:24:33Z

Description

Introduces CREATE OR REPLACE TABLE operation for Delta lake connector. It comes with some restrictions like we can't run CORTAS on table with cdc enabled or on cdc enabled roles.

Additional context and related issues

Release notes

( ) This is not user-visible or is 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 Lake Connector
* Add support for CREATE OR REPLACE TABLE statements. ({issue}`issuenumber`)

findinpath · 2023-12-13T16:28:45Z

...io/trino/tests/product/deltalake/TestDeltaLakeCreateOrReplaceTableAsSelectCompatibility.java

    {
        String tableName = "test_replace_table_with_schema_change_" + randomNameSuffix();
        onTrino().executeQuery("CREATE TABLE delta.default." + tableName + " (ts VARCHAR) " +
                "with (location = 's3://" + bucketName + "/databricks-compatibility-test-" + tableName + "', checkpoint_interval = 10)");
        try {
-            ImmutableList.Builder<QueryAssert.Row> expected = ImmutableList.builder();


I'm not sure why necessarily is this change necessary. The code after this change looks slightly the same as before.

We are extracting the INSERT operations to a static method so as to reuse them in the next commit.

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

findinpath · 2023-12-13T16:35:59Z

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

+                            tableHandle.getProjectedColumns(),
+                            session);
+                    protocolEntry = protocolEntryForTable(tableHandle.getProtocolEntry(), containsTimestampType, tableMetadata.getProperties());
+                    statisticsAccess.deleteExtendedStatistics(session, schemaTableName, location);


Add a test covering the fact that the extended statistics of the table are missing after CREATE OR REPLACE table.

Maybe use extended_statistics_collect_on_write session property on CREATE OR REPLACE TABLE AS SELECT to showcase that the old statistics have been removed.

findinpath · 2023-12-13T16:40:18Z

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

+                    statisticsAccess.deleteExtendedStatistics(session, schemaTableName, location);
+                }
+                else {
+                    setRollback(() -> deleteRecursivelyIfExists(fileSystem, deltaLogDirectory));


I'm assuming that this branch is for the situation where the transaction log exists , but we're not dealing with a CREATE OR REPLACE TABLE statement.

This is however handled through the check

if (!replaceExistingTable && transactionLogFileExists) { throw new TrinoException( NOT_SUPPORTED, "Using CREATE TABLE with an existing table content is disallowed, instead use the system.register_table() procedure."); }

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

findinpath · 2023-12-13T16:49:46Z

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

-            try {
-                Location transactionLogDir = Location.of(getTransactionLogDir(location));
-                fileSystemFactory.create(session).deleteDirectory(transactionLogDir);
+            if (!writeCommitted) {


writeCommited logic could be added in a preparatory commit.

But the writeCommited logic is very specific to CORTAS operation so I'm not sure if adding them in a preparatory commit would help us here. WDYT ?

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

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

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

findinpath · 2023-12-13T19:54:24Z

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

+                    "CREATE OR REPLACE TABLE " + table.getName() + " (a BIGINT) WITH (location = '%s_2')".formatted(location),
+                    "The provided location '%1$s_2' does not match the existing table location '%1$s'".formatted(location));
+
+            assertLatestTableOperation(table.getName(), CREATE_TABLE_OPERATION);


maybe verify that the table location has not changed.

Added additional assertion.

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

ebyhr · 2024-04-09T23:49:45Z

...io/trino/tests/product/deltalake/TestDeltaLakeCreateOrReplaceTableAsSelectCompatibility.java

+            assertThat(onTrino().executeQuery("SELECT to_iso8601(ts) FROM delta.default." + tableName)).containsOnly(expectedRows);
+        }
+        finally {
+            dropDeltaTableWithRetry(tableName);


No need to use dropDeltaTableWithRetry here because the method is for Glue environment. Same for others, but please feel free to ignore as it's pre-existing.

Suggested change

dropDeltaTableWithRetry(tableName);

onTrino().executeQuery("DROP TABLE delta.default." + tableName);

I think we can handle it as a follow-up.

ebyhr · 2024-04-09T23:51:55Z

...io/trino/tests/product/deltalake/TestDeltaLakeCreateOrReplaceTableAsSelectCompatibility.java

+        String tableName = "test_replace_table_with_schema_change_" + randomNameSuffix();
+
+        onTrino().executeQuery("CREATE TABLE delta.default." + tableName + " (ts VARCHAR) " +
+                "with (location = 's3://" + bucketName + "/databricks-compatibility-test-" + tableName + "', checkpoint_interval = 10)");


nit: Uppercase with. Please feel free to ignore as it's pre-existing.

ebyhr · 2024-04-09T23:53:14Z

...io/trino/tests/product/deltalake/TestDeltaLakeCreateOrReplaceTableAsSelectCompatibility.java

+        assertQueryFailure(() -> onDelta().executeQuery("" +
+                "CREATE OR REPLACE TABLE default." + tableName +
+                "         (a INT,c INT) " +
+                "         USING delta " +
+                "         LOCATION 's3://" + bucketName + "/databricks-compatibility-test-" + tableName + "' " +
+                "         TBLPROPERTIES ('delta.appendOnly' = true)"))
+                .hasMessageContaining("This table is configured to only allow appends");
+
+        assertQueryFailure(() -> onTrino().executeQuery("" +
+                "CREATE OR REPLACE TABLE delta.default." + tableName + " (a INT,c INT)" +
+                " WITH (location = 's3://" + bucketName + "/databricks-compatibility-test-" + tableName + "')"))
+                .hasMessageContaining("Cannot replace a table when 'delta.appendOnly' is set to true");


Does Delta Lake allow replacing tables when delta.appendOnly is true in the existing table and the delta.appendOnly is false in the new table definition?

I think delta lake doesn't allow to replace tables with change in delta.appendOnly it is being tested in L: 130 or is it something different ?

I wanted to know what happens if we set 'delta.appendOnly' = false at L135.

ebyhr · 2024-04-10T00:01:02Z

...io/trino/tests/product/deltalake/TestDeltaLakeCreateOrReplaceTableAsSelectCompatibility.java

+    public void testCreateOrReplaceTableOnAppendOnlyTableFails()
+    {
+        String tableName = "test_replace_on_append_only_table_fails_" + randomNameSuffix();
+        onDelta().executeQuery("" +


nit: It would be nice to use try-catch for consistency in this class.

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

ebyhr · 2024-04-10T00:13:56Z

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

+            assertLatestTableOperation(tableName, CREATE_OR_REPLACE_TABLE_OPERATION);
+        }
+        finally {
+            assertUpdate("DROP TABLE " + tableName);


This DROP TABLE may throw exceptions if CREATE OR REPLACE TABLE failed for some reason. I would move CREATE OR REPLACE TABLE before try. Same for others.

How about DROP TABLE IF EXISTS ?

Praveen2112 · 2024-04-10T15:45:27Z

@ebyhr / @findinpath Thanks a lot for the review. AC

core/trino-main/src/main/java/io/trino/sql/rewrite/ShowStatsRewrite.java

findinpath

LGTM % left-over

findinpath · 2024-04-11T08:42:15Z

Pls rebase to address the code conflicts

ebyhr · 2024-04-11T10:02:34Z

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

+        assertTableOperationWithChangeInColumnMappingMode("name");
+    }
+
+    public void assertTableOperationWithChangeInColumnMappingMode(String columnMappingMode)


nit: testTableOperationWithChangeInColumnMappingMode sounds better. I expect assert* methods do simple assertions. This method contains testing data setup.

ebyhr · 2024-04-11T10:06:56Z

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

+        assertThat(query("SELECT DISTINCT regexp_replace(\"$path\", '/[^/]*$', '') FROM " + tableName))
+                .skippingTypesCheck()
+                .matches("VALUES '%s'".formatted(expectedLocation));


nit: We could use computeScalar method to avoid skippingTypesCheck method:

assertThat(computeScalar("SELECT DISTINCT regexp_replace(\"$path\", '/[^/]*$', '') FROM " + tableName)) .isEqualTo(expectedLocation);

By the way, this method doesn't work for empty tables and is inefficient when table contains many rows. That's why we use SHOW CREATE TABLE result in other Delta Lake tests.

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

Lines 357 to 367 in ac71d15

protected String getTableLocation(String tableName)

{

Pattern locationPattern = Pattern.compile(".*location = '(.*?)'.*", Pattern.DOTALL);

Matcher m = locationPattern.matcher((String) computeActual("SHOW CREATE TABLE " + tableName).getOnlyValue());

if (m.find()) {

String location = m.group(1);

verify(!m.find(), "Unexpected second match");

return location;

}

throw new IllegalStateException("Location not found in SHOW CREATE TABLE result");

}

Actually that method was not used and we have replaced with SHOW CREATE type of assertion. Removed that method.

Praveen2112 · 2024-04-11T13:45:30Z

@ebyhr Thanks a lot for the review once again.

Praveen2112 · 2024-04-11T13:50:54Z

/test-with-secrets sha=26b0e08b91321bb78fafaee70b87c8ec50db4e7a

github-actions · 2024-04-11T15:31:58Z

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

Praveen2112 · 2024-04-12T06:32:04Z

/test-with-secrets sha=4f82268a69d8ac97eba47d30138194a9c59ac593

github-actions · 2024-04-12T06:33:06Z

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

Praveen2112 · 2024-04-12T06:38:29Z

/test-with-secrets sha=fb0a6f37155b190956efc664562f10a806f3765f

github-actions · 2024-04-12T06:39:57Z

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

...io/trino/tests/product/deltalake/TestDeltaLakeCreateOrReplaceTableAsSelectCompatibility.java

Praveen2112 · 2024-04-12T16:23:01Z

/test-with-secrets sha=98e554834032de0a5417fcddf3374f08a4adbd29

github-actions · 2024-04-12T16:25:43Z

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

cla-bot bot added the cla-signed label Dec 1, 2023

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

findinpath mentioned this pull request Dec 4, 2023

Support CREATE OR REPLACE in table materialization starburstdata/dbt-trino#377

Merged

Praveen2112 force-pushed the praveen/cortas_delta_connector branch 2 times, most recently from b2a6b28 to 3ad2c65 Compare December 13, 2023 14:55

Praveen2112 requested review from ebyhr and findinpath December 13, 2023 15:04

findinpath reviewed Dec 13, 2023

View reviewed changes

findinpath requested a review from pajaks December 13, 2023 17:39

findinpath reviewed Dec 13, 2023

View reviewed changes

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

findinpath reviewed Dec 13, 2023

View reviewed changes

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

findinpath reviewed Dec 13, 2023

View reviewed changes

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

findinpath reviewed Dec 14, 2023

View reviewed changes

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

findinpath mentioned this pull request Dec 14, 2023

CREATE [OR REPLACE] TABLE support for Iceberg and Delta #13180

Closed

findepi mentioned this pull request Dec 20, 2023

Make createExclusive atomic: fix GcsTransactionLogSynchronizer atomicity with GCS native filesystem implementation #20180

Merged

hovaesco mentioned this pull request Jan 22, 2024

deltalake rename managed table not allowed arised starburstdata/dbt-trino#382

Open

1 task

ebyhr mentioned this pull request Mar 15, 2024

Add support for time travel with version in Delta Lake #21052

Merged

3 tasks

Praveen2112 force-pushed the praveen/cortas_delta_connector branch 7 times, most recently from e11de06 to 110b7ab Compare March 20, 2024 08:36

Praveen2112 mentioned this pull request Apr 1, 2024

Race condition in Alluxio cache #21342

Closed

Praveen2112 force-pushed the praveen/cortas_delta_connector branch 3 times, most recently from 7b2c1b6 to 65a9146 Compare April 2, 2024 16:28

ebyhr approved these changes Apr 10, 2024

View reviewed changes

Praveen2112 force-pushed the praveen/cortas_delta_connector branch from 45d4af4 to d9559b5 Compare April 10, 2024 15:45

findinpath reviewed Apr 10, 2024

View reviewed changes

core/trino-main/src/main/java/io/trino/sql/rewrite/ShowStatsRewrite.java Outdated Show resolved Hide resolved

findinpath approved these changes Apr 10, 2024

View reviewed changes

Praveen2112 force-pushed the praveen/cortas_delta_connector branch from d9559b5 to d9d6bc8 Compare April 11, 2024 07:29

Praveen2112 force-pushed the praveen/cortas_delta_connector branch from d9d6bc8 to af8977a Compare April 11, 2024 09:47

ebyhr approved these changes Apr 11, 2024

View reviewed changes

Praveen2112 force-pushed the praveen/cortas_delta_connector branch from af8977a to 26b0e08 Compare April 11, 2024 13:44

Praveen2112 added 3 commits April 12, 2024 11:59

Minor cleanup in DeltaLakeMetadata#createTable

db007ac

Extract CORTAS operation on delta table to a dedicated test

735ebf3

Minor cleanup in TestDeltaLakeCreateOrReplaceTableAsSelectCompatibility

7baa5bc

Praveen2112 force-pushed the praveen/cortas_delta_connector branch from 26b0e08 to 4f82268 Compare April 12, 2024 06:30

Praveen2112 force-pushed the praveen/cortas_delta_connector branch from 4f82268 to fb0a6f3 Compare April 12, 2024 06:37

findinpath reviewed Apr 12, 2024

View reviewed changes

...io/trino/tests/product/deltalake/TestDeltaLakeCreateOrReplaceTableAsSelectCompatibility.java Outdated Show resolved Hide resolved

Implement CREATE OR REPLACE TABLE for delta lake connector

98e5548

Praveen2112 force-pushed the praveen/cortas_delta_connector branch from fb0a6f3 to 98e5548 Compare April 12, 2024 13:41

Praveen2112 merged commit 558fc59 into trinodb:master Apr 13, 2024
51 checks passed

github-actions bot added this to the 445 milestone Apr 13, 2024

colebow mentioned this pull request Apr 17, 2024

Add Trino 445 release notes #21472

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement CREATE OR REPLACE TABLE for delta lake connector #19991

Implement CREATE OR REPLACE TABLE for delta lake connector #19991

Praveen2112 commented Dec 1, 2023 •

edited

findinpath Dec 13, 2023

Praveen2112 Mar 20, 2024

findinpath Dec 13, 2023

findinpath Dec 13, 2023

findinpath Dec 13, 2023

Praveen2112 Apr 4, 2024

findinpath Dec 13, 2023

Praveen2112 Apr 4, 2024

ebyhr Apr 9, 2024

Praveen2112 Apr 10, 2024

ebyhr Apr 9, 2024

Praveen2112 Apr 10, 2024

ebyhr Apr 9, 2024

Praveen2112 Apr 10, 2024

ebyhr Apr 11, 2024

ebyhr Apr 10, 2024

ebyhr Apr 10, 2024

Praveen2112 Apr 10, 2024

Praveen2112 commented Apr 10, 2024

findinpath left a comment

findinpath commented Apr 11, 2024

ebyhr Apr 11, 2024

ebyhr Apr 11, 2024 •

edited

Praveen2112 Apr 11, 2024

Praveen2112 commented Apr 11, 2024

Praveen2112 commented Apr 11, 2024

github-actions bot commented Apr 11, 2024 •

edited

Praveen2112 commented Apr 12, 2024

github-actions bot commented Apr 12, 2024 •

edited

Praveen2112 commented Apr 12, 2024

github-actions bot commented Apr 12, 2024

Praveen2112 commented Apr 12, 2024

github-actions bot commented Apr 12, 2024

	dropDeltaTableWithRetry(tableName);
	onTrino().executeQuery("DROP TABLE delta.default." + tableName);

	protected String getTableLocation(String tableName)
	{
	Pattern locationPattern = Pattern.compile(".location = '(.?)'.*", Pattern.DOTALL);
	Matcher m = locationPattern.matcher((String) computeActual("SHOW CREATE TABLE " + tableName).getOnlyValue());
	if (m.find()) {
	String location = m.group(1);
	verify(!m.find(), "Unexpected second match");
	return location;
	}
	throw new IllegalStateException("Location not found in SHOW CREATE TABLE result");
	}

Implement CREATE OR REPLACE TABLE for delta lake connector #19991

Implement CREATE OR REPLACE TABLE for delta lake connector #19991

Conversation

Praveen2112 commented Dec 1, 2023 • edited

Description

Additional context and related issues

Release notes

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

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

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

Praveen2112 commented Apr 10, 2024

findinpath left a comment

Choose a reason for hiding this comment

findinpath commented Apr 11, 2024

Choose a reason for hiding this comment

ebyhr Apr 11, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Praveen2112 commented Apr 11, 2024

Praveen2112 commented Apr 11, 2024

github-actions bot commented Apr 11, 2024 • edited

Praveen2112 commented Apr 12, 2024

github-actions bot commented Apr 12, 2024 • edited

Praveen2112 commented Apr 12, 2024

github-actions bot commented Apr 12, 2024

Praveen2112 commented Apr 12, 2024

github-actions bot commented Apr 12, 2024

Praveen2112 commented Dec 1, 2023 •

edited

ebyhr Apr 11, 2024 •

edited

github-actions bot commented Apr 11, 2024 •

edited

github-actions bot commented Apr 12, 2024 •

edited