From 2ef419475a54b4246e9e073bafe936e39ef64be7 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Sat, 31 Aug 2024 21:44:59 +0900 Subject: [PATCH 1/2] Implement beginCreateTable with boolean replace in connectors --- .../main/java/io/trino/testing/TestingMetadata.java | 2 +- .../test/java/io/trino/connector/MockConnector.java | 2 +- .../io/trino/plugin/accumulo/AccumuloMetadata.java | 5 ++++- .../io/trino/plugin/jdbc/DefaultJdbcMetadata.java | 5 ++++- .../trino/plugin/jdbc/TestDefaultJdbcMetadata.java | 4 ++-- .../java/io/trino/plugin/ignite/IgniteMetadata.java | 5 ++++- .../java/io/trino/plugin/memory/MemoryMetadata.java | 7 +++++-- .../io/trino/plugin/memory/TestMemoryMetadata.java | 12 ++++++++---- .../io/trino/plugin/phoenix5/PhoenixMetadata.java | 5 ++++- .../trino/plugin/raptor/legacy/RaptorMetadata.java | 7 +++++-- .../raptor/legacy/metadata/TestRaptorMetadata.java | 6 +++--- 11 files changed, 41 insertions(+), 19 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java b/core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java index 8d5d81fff38a..a2037746aa69 100644 --- a/core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java +++ b/core/trino-main/src/main/java/io/trino/testing/TestingMetadata.java @@ -326,7 +326,7 @@ public ConnectorInsertTableHandle beginRefreshMaterializedView(ConnectorSession } @Override - public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) + public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { createTable(session, tableMetadata, SaveMode.FAIL); return TestingHandle.INSTANCE; diff --git a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java index 0e82c07091d0..7ab89c8593df 100644 --- a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java +++ b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java @@ -840,7 +840,7 @@ public Optional getLayoutForTableExecute(ConnectorSession } @Override - public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) + public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { return new MockConnectorOutputTableHandle(tableMetadata.getTable()); } diff --git a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloMetadata.java b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloMetadata.java index e328f6dfa2cd..adb3d34bb536 100644 --- a/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloMetadata.java +++ b/plugin/trino-accumulo/src/main/java/io/trino/plugin/accumulo/AccumuloMetadata.java @@ -101,11 +101,14 @@ public void dropSchema(ConnectorSession session, String schemaName, boolean casc } @Override - public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) + public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { if (retryMode != NO_RETRIES) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support query retries"); } + if (replace) { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); + } if (tableMetadata.getComment().isPresent()) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with table comment"); } diff --git a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java index da4787ea6755..a4e79f168516 100644 --- a/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java +++ b/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/DefaultJdbcMetadata.java @@ -1148,9 +1148,12 @@ public Optional getSupportedType(ConnectorSession session, Map layout, RetryMode retryMode) + public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { verifyRetryMode(session, retryMode); + if (replace) { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); + } JdbcOutputTableHandle handle = jdbcClient.beginCreateTable(session, tableMetadata); setRollback(() -> jdbcClient.rollbackCreateTable(session, handle)); return handle; diff --git a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java index e4abd3a83193..858a5685767f 100644 --- a/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java +++ b/plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/TestDefaultJdbcMetadata.java @@ -95,7 +95,7 @@ public void testSupportsRetriesValidation() ImmutableSet.of()); ConnectorTableMetadata tableMetadata = new ConnectorTableMetadata(new SchemaTableName("example", "numbers"), ImmutableList.of()); - assertThatThrownBy(() -> metadata.beginCreateTable(SESSION, tableMetadata, Optional.empty(), RetryMode.RETRIES_ENABLED)) + assertThatThrownBy(() -> metadata.beginCreateTable(SESSION, tableMetadata, Optional.empty(), RetryMode.RETRIES_ENABLED, false)) .hasMessageContaining("This connector does not support query or task retries"); assertThatThrownBy(() -> metadata.beginInsert(SESSION, tableHandle, ImmutableList.of(), RetryMode.RETRIES_ENABLED)) @@ -117,7 +117,7 @@ public void testNonTransactionalInsertValidation() PropertyMetadata.booleanProperty(JdbcWriteSessionProperties.NON_TRANSACTIONAL_INSERT, "description", true, false))) .build(); - assertThatThrownBy(() -> metadata.beginCreateTable(session, tableMetadata, Optional.empty(), RetryMode.RETRIES_ENABLED)) + assertThatThrownBy(() -> metadata.beginCreateTable(session, tableMetadata, Optional.empty(), RetryMode.RETRIES_ENABLED, false)) .hasMessageContaining("Query and task retries are incompatible with non-transactional inserts"); assertThatThrownBy(() -> metadata.beginInsert(session, tableHandle, ImmutableList.of(), RetryMode.RETRIES_ENABLED)) diff --git a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java index b2a59af4aff4..9d978f0b2695 100644 --- a/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java +++ b/plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java @@ -163,11 +163,14 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe } @Override - public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) + public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { if (retryMode != NO_RETRIES) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support query retries"); } + if (replace) { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); + } return igniteClient.beginCreateTable(session, tableMetadata); } diff --git a/plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java b/plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java index 86b9f4c10e8c..a9da77ac5127 100644 --- a/plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java +++ b/plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java @@ -328,13 +328,16 @@ public synchronized void createTable(ConnectorSession session, ConnectorTableMet if (saveMode == REPLACE) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); } - ConnectorOutputTableHandle outputTableHandle = beginCreateTable(session, tableMetadata, Optional.empty(), NO_RETRIES); + ConnectorOutputTableHandle outputTableHandle = beginCreateTable(session, tableMetadata, Optional.empty(), NO_RETRIES, false); finishCreateTable(session, outputTableHandle, ImmutableList.of(), ImmutableList.of()); } @Override - public synchronized MemoryOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) + public synchronized MemoryOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { + if (replace) { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); + } checkSchemaExists(tableMetadata.getTable().getSchemaName()); checkTableNotExists(tableMetadata.getTable()); long tableId = nextTableId.getAndIncrement(); diff --git a/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryMetadata.java b/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryMetadata.java index 638f58dddb34..7dd489ab75a0 100644 --- a/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryMetadata.java +++ b/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryMetadata.java @@ -59,7 +59,8 @@ public void tableIsCreatedAfterCommits() SESSION, new ConnectorTableMetadata(schemaTableName, ImmutableList.of(), ImmutableMap.of()), Optional.empty(), - NO_RETRIES); + NO_RETRIES, + false); metadata.finishCreateTable(SESSION, table, ImmutableList.of(), ImmutableList.of()); @@ -132,7 +133,8 @@ public void testReadTableBeforeCreationCompleted() SESSION, new ConnectorTableMetadata(tableName, ImmutableList.of(), ImmutableMap.of()), Optional.empty(), - NO_RETRIES); + NO_RETRIES, + false); List tableNames = metadata.listTables(SESSION, Optional.empty()); assertThat(tableNames.size()) @@ -287,7 +289,8 @@ public void testCreateTableAndViewInNotExistSchema() SESSION, new ConnectorTableMetadata(table1, ImmutableList.of(), ImmutableMap.of()), Optional.empty(), - NO_RETRIES)) + NO_RETRIES, + false)) .hasErrorCode(NOT_FOUND) .hasMessage("Schema test1 not found"); assertThat(metadata.getTableHandle(SESSION, table1, Optional.empty(), Optional.empty())).isNull(); @@ -317,7 +320,8 @@ public void testRenameTable() SESSION, new ConnectorTableMetadata(tableName, ImmutableList.of(), ImmutableMap.of()), Optional.empty(), - NO_RETRIES); + NO_RETRIES, + false); metadata.finishCreateTable(SESSION, table, ImmutableList.of(), ImmutableList.of()); // rename table to schema which does not exist diff --git a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixMetadata.java b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixMetadata.java index b0951f8366ee..15a3304f5a76 100644 --- a/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixMetadata.java +++ b/plugin/trino-phoenix5/src/main/java/io/trino/plugin/phoenix5/PhoenixMetadata.java @@ -211,11 +211,14 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe } @Override - public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) + public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { if (retryMode != NO_RETRIES) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support query retries"); } + if (replace) { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); + } return phoenixClient.beginCreateTable(session, tableMetadata); } diff --git a/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/RaptorMetadata.java b/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/RaptorMetadata.java index c2424632bef7..6527407f0d67 100644 --- a/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/RaptorMetadata.java +++ b/plugin/trino-raptor-legacy/src/main/java/io/trino/plugin/raptor/legacy/RaptorMetadata.java @@ -462,7 +462,7 @@ public void createTable(ConnectorSession session, ConnectorTableMetadata tableMe throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); } Optional layout = getNewTableLayout(session, tableMetadata); - finishCreateTable(session, beginCreateTable(session, tableMetadata, layout, NO_RETRIES), ImmutableList.of(), ImmutableList.of()); + finishCreateTable(session, beginCreateTable(session, tableMetadata, layout, NO_RETRIES, false), ImmutableList.of(), ImmutableList.of()); } @Override @@ -556,11 +556,14 @@ public void dropColumn(ConnectorSession session, ConnectorTableHandle tableHandl } @Override - public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) + public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { if (retryMode != NO_RETRIES) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support query retries"); } + if (replace) { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); + } if (tableMetadata.getComment().isPresent()) { throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with table comment"); } diff --git a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/metadata/TestRaptorMetadata.java b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/metadata/TestRaptorMetadata.java index facf942a036a..bf901c51fdda 100644 --- a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/metadata/TestRaptorMetadata.java +++ b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/metadata/TestRaptorMetadata.java @@ -387,7 +387,7 @@ public void testCreateBucketedTableAsSelect() RaptorPartitioningHandle partitioning = (RaptorPartitioningHandle) layout.getPartitioning().get(); assertThat(partitioning.getDistributionId()).isEqualTo(1); - ConnectorOutputTableHandle outputHandle = metadata.beginCreateTable(SESSION, ordersTable, Optional.of(layout), NO_RETRIES); + ConnectorOutputTableHandle outputHandle = metadata.beginCreateTable(SESSION, ordersTable, Optional.of(layout), NO_RETRIES, false); metadata.finishCreateTable(SESSION, outputHandle, ImmutableList.of(), ImmutableList.of()); ConnectorTableHandle tableHandle = metadata.getTableHandle(SESSION, DEFAULT_TEST_ORDERS, Optional.empty(), Optional.empty()); @@ -656,7 +656,7 @@ public void testTransactionTableWrite() { // start table creation long transactionId = 1; - ConnectorOutputTableHandle outputHandle = metadata.beginCreateTable(SESSION, getOrdersTable(), Optional.empty(), NO_RETRIES); + ConnectorOutputTableHandle outputHandle = metadata.beginCreateTable(SESSION, getOrdersTable(), Optional.empty(), NO_RETRIES, false); // transaction is in progress assertThat(transactionExists(transactionId)).isTrue(); @@ -696,7 +696,7 @@ public void testTransactionAbort() { // start table creation long transactionId = 1; - ConnectorOutputTableHandle outputHandle = metadata.beginCreateTable(SESSION, getOrdersTable(), Optional.empty(), NO_RETRIES); + ConnectorOutputTableHandle outputHandle = metadata.beginCreateTable(SESSION, getOrdersTable(), Optional.empty(), NO_RETRIES, false); // transaction is in progress assertThat(transactionExists(transactionId)).isTrue(); From 8a0bcdb0862edeaaadc5ecc7c0ef7f25df53ec02 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Sat, 31 Aug 2024 21:46:25 +0900 Subject: [PATCH 2/2] Remove deprecated ConnectorMetadata.beginCreateTable without replace --- .../tracing/TracingConnectorMetadata.java | 9 -------- .../spi/connector/ConnectorMetadata.java | 23 +++---------------- .../ClassLoaderSafeConnectorMetadata.java | 8 ------- 3 files changed, 3 insertions(+), 37 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java b/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java index 1164109300a0..81df2d43fb4e 100644 --- a/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java +++ b/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java @@ -612,15 +612,6 @@ public void finishStatisticsCollection(ConnectorSession session, ConnectorTableH } } - @Override - public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) - { - Span span = startSpan("beginCreateTable", tableMetadata.getTable()); - try (var _ = scopedSpan(span)) { - return delegate.beginCreateTable(session, tableMetadata, layout, retryMode); - } - } - @Override public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java index 43bc7096d383..eacc3d734f3b 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java @@ -728,23 +728,6 @@ default void finishStatisticsCollection(ConnectorSession session, ConnectorTable throw new TrinoException(GENERIC_INTERNAL_ERROR, "ConnectorMetadata beginStatisticsCollection() is implemented without finishStatisticsCollection()"); } - /** - * Begin the atomic creation of a table with data. - *

- * If connector does not support execution with retries, the method should throw: - * 

-     *     new TrinoException(NOT_SUPPORTED, "This connector does not support query retries")
-     *
- * unless {@code retryMode} is set to {@code NO_RETRIES}. - * - * @deprecated use {@link #beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace)} - */ - @Deprecated - default ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) - { - throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with data"); - } - /** * Begin the atomic creation of a table with data. * If connector does not support execution with retries, the method should throw: @@ -756,10 +739,10 @@ default ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, Co default ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) { // Redirect to deprecated SPI to not break existing connectors - if (!replace) { - return beginCreateTable(session, tableMetadata, layout, retryMode); + if (replace) { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); } - throw new TrinoException(NOT_SUPPORTED, "This connector does not support replacing tables"); + throw new TrinoException(NOT_SUPPORTED, "This connector does not support creating tables with data"); } /** diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java index a24552a52e1b..8cb664f1dd61 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java @@ -547,14 +547,6 @@ public void setColumnComment(ConnectorSession session, ConnectorTableHandle tabl } } - @Override - public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode) - { - try (ThreadContextClassLoader _ = new ThreadContextClassLoader(classLoader)) { - return delegate.beginCreateTable(session, tableMetadata, layout, retryMode); - } - } - @Override public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout, RetryMode retryMode, boolean replace) {