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.
trinodb · Jan 17, 2022 · 7bf8da9 · 7bf8da9
1 parent 8854c97
commit 7bf8da9
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 8 deletions.
diff --git 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
@@ -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/...clickhouse/src/test/java/io/trino/plugin/clickhouse/BaseClickHouseConnectorSmokeTest.java b/...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/...clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorSmokeTest.java b/...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.<String, String>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/...rino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java b/...rino-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()
     {