From 34feb64a116bd80ddcc368d6257e70bc29fecddf Mon Sep 17 00:00:00 2001 From: tangjiangling Date: Mon, 17 Jan 2022 14:40:20 +0800 Subject: [PATCH] Implement renameSchema for ClickHouse The current implementation executes `ALTER SCHEMA ... RENAME` unconditionally, and let ClickHouse throw the proper errors. The reasoning for this is that we already do this for some other methods in some connectors. And it's possible that our assumptions today will become false in future if there are more changes in ClickHouse. --- docs/src/main/sphinx/connector/clickhouse.rst | 3 +++ .../plugin/clickhouse/ClickHouseClient.java | 5 ++-- .../BaseClickHouseConnectorSmokeTest.java | 1 - .../TestClickHouseConnectorSmokeTest.java | 25 ++++++++++++++++++- .../TestClickHouseConnectorTest.java | 22 +++++++++++++--- 5 files changed, 48 insertions(+), 8 deletions(-) diff --git a/docs/src/main/sphinx/connector/clickhouse.rst b/docs/src/main/sphinx/connector/clickhouse.rst index 8536fa35fd7d..cbfe8509016c 100644 --- a/docs/src/main/sphinx/connector/clickhouse.rst +++ b/docs/src/main/sphinx/connector/clickhouse.rst @@ -189,3 +189,6 @@ statements, the connector supports the following features: * :doc:`/sql/drop-table` * :doc:`/sql/create-schema` * :doc:`/sql/drop-schema` +* :doc:`/sql/alter-schema` + +.. include:: alter-schema-limitation.fragment diff --git a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java index 978f6494d0f2..4854d479b7e2 100644 --- a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java +++ b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java @@ -278,10 +278,9 @@ public void dropSchema(ConnectorSession session, String schemaName) } @Override - public void renameSchema(ConnectorSession session, String schemaName, String newSchemaName) + protected String renameSchemaSql(String remoteSchemaName, String newRemoteSchemaName) { - // TODO: https://github.com/trinodb/trino/issues/10558 - throw new TrinoException(NOT_SUPPORTED, "This connector does not support renaming schemas"); + return "RENAME DATABASE " + quoted(remoteSchemaName) + " TO " + quoted(newRemoteSchemaName); } @Override diff --git a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/BaseClickHouseConnectorSmokeTest.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/BaseClickHouseConnectorSmokeTest.java index 460a158e7977..92e5c3688c15 100644 --- a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/BaseClickHouseConnectorSmokeTest.java +++ b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/BaseClickHouseConnectorSmokeTest.java @@ -23,7 +23,6 @@ public abstract class BaseClickHouseConnectorSmokeTest protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) { switch (connectorBehavior) { - case SUPPORTS_RENAME_SCHEMA: case SUPPORTS_DELETE: return false; default: diff --git a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorSmokeTest.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorSmokeTest.java index 0765947504f9..4d7105774973 100644 --- a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorSmokeTest.java +++ b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorSmokeTest.java @@ -17,20 +17,43 @@ import io.trino.testing.QueryRunner; import static io.trino.plugin.clickhouse.ClickHouseQueryRunner.createClickHouseQueryRunner; +import static io.trino.testing.sql.TestTable.randomTableSuffix; +import static org.assertj.core.api.Assertions.assertThatThrownBy; public class TestClickHouseConnectorSmokeTest extends BaseClickHouseConnectorSmokeTest { + protected TestingClickHouseServer clickHouseServer; + @Override protected QueryRunner createQueryRunner() throws Exception { + clickHouseServer = closeAfterClass(new TestingClickHouseServer()); return createClickHouseQueryRunner( - closeAfterClass(new TestingClickHouseServer()), + clickHouseServer, ImmutableMap.of(), ImmutableMap.builder() .put("clickhouse.map-string-as-varchar", "true") .build(), REQUIRED_TPCH_TABLES); } + + @Override + public void testRenameSchema() + { + // Override because the default database engine in version < v20.10.2.20-stable doesn't allow renaming schemas + assertThatThrownBy(super::testRenameSchema) + .hasMessageMatching("ClickHouse exception, code: 48,.* Ordinary: RENAME DATABASE is not supported .*\\n"); + + String schemaName = "test_rename_schema_" + randomTableSuffix(); + try { + clickHouseServer.execute("CREATE DATABASE " + schemaName + " ENGINE = Atomic"); + assertUpdate("ALTER SCHEMA " + schemaName + " RENAME TO " + schemaName + "_renamed"); + } + finally { + assertUpdate("DROP SCHEMA IF EXISTS " + schemaName); + assertUpdate("DROP SCHEMA IF EXISTS " + schemaName + "_renamed"); + } + } } diff --git a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java index 74ce54884fd8..6b08e6fc6520 100644 --- a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java +++ b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java @@ -87,9 +87,6 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) case SUPPORTS_DELETE: return false; - case SUPPORTS_RENAME_SCHEMA: - return false; - default: return super.hasBehavior(connectorBehavior); } @@ -102,6 +99,25 @@ public void testColumnName(String columnName) throw new SkipException("TODO: test not implemented yet"); } + @Test + @Override + public void testRenameSchema() + { + // Override because the default database engine in version < v20.10.2.20-stable doesn't allow renaming schemas + assertThatThrownBy(super::testRenameSchema) + .hasMessageMatching("ClickHouse exception, code: 48,.* Ordinary: RENAME DATABASE is not supported .*\\n"); + + String schemaName = "test_rename_schema_" + randomTableSuffix(); + try { + onRemoteDatabase().execute("CREATE DATABASE " + schemaName + " ENGINE = Atomic"); + assertUpdate("ALTER SCHEMA " + schemaName + " RENAME TO " + schemaName + "_renamed"); + } + finally { + assertUpdate("DROP SCHEMA IF EXISTS " + schemaName); + assertUpdate("DROP SCHEMA IF EXISTS " + schemaName + "_renamed"); + } + } + @Override public void testRenameColumn() {