Replace deprecated driver ru.yandex.clickhouse.ClickHouseDriver

docs/src/main/sphinx/connector/clickhouse.rst

@@ -34,6 +34,15 @@ appropriate for your setup:
     connection-user=exampleuser
     connection-password=examplepassword
 
+.. note::
+
+    Trino uses the new ClickHouse driver(``com.clickhouse.jdbc.ClickHouseDriver``)
+    by default, but the new driver only supports ClickHouse server with version >= 20.7.
+
+    For compatibility with ClickHouse server versions < 20.7,
+    you can temporarily continue to use the old ClickHouse driver(``ru.yandex.clickhouse.ClickHouseDriver``)
+    by adding the following catalog property: ``clickhouse.legacy-driver=true``.
+
 .. _clickhouse-tls:
 
 Connection security

plugin/trino-clickhouse/pom.xml

@@ -46,27 +46,12 @@
         <dependency>
             <groupId>com.clickhouse</groupId>
             <artifactId>clickhouse-jdbc</artifactId>
-            <version>0.3.2-patch1</version>
+            <version>0.3.2-patch3</version>
+            <classifier>all</classifier>
             <exclusions>
                 <exclusion>
-                    <groupId>org.slf4j</groupId>
-                    <artifactId>slf4j-api</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>commons-logging</groupId>
-                    <artifactId>commons-logging</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>org.projectlombok</groupId>
-                    <artifactId>lombok</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>com.clickhouse</groupId>
-                    <artifactId>clickhouse-http-client</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>org.robolectric</groupId>
-                    <artifactId>android-all</artifactId>
+                    <groupId>*</groupId>
+                    <artifactId>*</artifactId>
                 </exclusion>
             </exclusions>
         </dependency>

plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java

@@ -13,6 +13,8 @@
  */
 package io.trino.plugin.clickhouse;
 
+import com.clickhouse.client.ClickHouseColumn;
+import com.clickhouse.client.ClickHouseDataType;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.net.InetAddresses;
@@ -61,9 +63,7 @@
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.sql.Connection;
-import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
-import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Types;
 import java.time.LocalDate;
@@ -350,16 +350,9 @@ public void setColumnComment(ConnectorSession session, JdbcTableHandle handle, J
     }
 
     @Override
-    public ResultSet getTables(Connection connection, Optional<String> schemaName, Optional<String> tableName)
-            throws SQLException
+    protected Optional<List<String>> getTableTypes()
     {
-        // ClickHouse maps their "database" to SQL catalogs and does not have schemas
-        DatabaseMetaData metadata = connection.getMetaData();
-        return metadata.getTables(
-                null,
-                schemaName.orElse(null),
-                escapeNamePattern(tableName, metadata.getSearchStringEscape()).orElse(null),
-                new String[] {"TABLE", "VIEW"});
+        return Optional.empty();
     }
 
     @Override
@@ -410,22 +403,24 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
             return mapping;
         }
 
-        switch (jdbcTypeName.replaceAll("\\(.*\\)$", "")) {
-            case "IPv4":
+        ClickHouseColumn column = ClickHouseColumn.of("", jdbcTypeName);
+        ClickHouseDataType columnDataType = column.getDataType();
+        switch (columnDataType) {
+            case IPv4:
                 return Optional.of(ipAddressColumnMapping("IPv4StringToNum(?)"));
-            case "IPv6":
+            case IPv6:
                 return Optional.of(ipAddressColumnMapping("IPv6StringToNum(?)"));
-            case "Enum8":
-            case "Enum16":
+            case Enum8:
+            case Enum16:
                 return Optional.of(ColumnMapping.sliceMapping(
                         createUnboundedVarcharType(),
                         varcharReadFunction(createUnboundedVarcharType()),
                         varcharWriteFunction(),
                         // TODO (https://github.com/trinodb/trino/issues/7100) Currently pushdown would not work and may require a custom bind expression
                         DISABLE_PUSHDOWN));
 
-            case "FixedString": // FixedString(n)
-            case "String":
+            case FixedString: // FixedString(n)
+            case String:
                 if (isMapStringAsVarchar(session)) {
                     return Optional.of(ColumnMapping.sliceMapping(
                             createUnboundedVarcharType(),
@@ -435,8 +430,10 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
                 }
                 // TODO (https://github.com/trinodb/trino/issues/7100) test & enable predicate pushdown
                 return Optional.of(varbinaryColumnMapping());
-            case "UUID":
+            case UUID:
                 return Optional.of(uuidColumnMapping());
+            default:
+                // no-op
         }
 
         switch (typeHandle.getJdbcType()) {
@@ -486,7 +483,7 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
                 return Optional.of(dateColumnMappingUsingLocalDate());
 
             case Types.TIMESTAMP:
-                if (jdbcTypeName.equals("DateTime")) {
+                if (columnDataType == ClickHouseDataType.DateTime) {
                     verify(typeHandle.getRequiredDecimalDigits() == 0, "Expected 0 as timestamp precision, but got %s", typeHandle.getRequiredDecimalDigits());
                     return Optional.of(ColumnMapping.longMapping(
                             TIMESTAMP_SECONDS,

plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClientModule.java

@@ -26,7 +26,8 @@
 import io.trino.plugin.jdbc.ForBaseJdbc;
 import io.trino.plugin.jdbc.JdbcClient;
 import io.trino.plugin.jdbc.credential.CredentialProvider;
-import ru.yandex.clickhouse.ClickHouseDriver;
+
+import java.sql.Driver;
 
 import static io.trino.plugin.jdbc.JdbcModule.bindSessionPropertiesProvider;
 import static io.trino.plugin.jdbc.JdbcModule.bindTablePropertiesProvider;
@@ -47,8 +48,16 @@ public void configure(Binder binder)
     @Provides
     @Singleton
     @ForBaseJdbc
-    public static ConnectionFactory createConnectionFactory(BaseJdbcConfig config, CredentialProvider credentialProvider)
+    public static ConnectionFactory createConnectionFactory(ClickHouseConfig clickHouseConfig, BaseJdbcConfig config, CredentialProvider credentialProvider)
+    {
+        return new ClickHouseConnectionFactory(new DriverConnectionFactory(createDriver(clickHouseConfig), config, credentialProvider));
+    }
+
+    private static Driver createDriver(ClickHouseConfig config)
     {
-        return new ClickHouseConnectionFactory(new DriverConnectionFactory(new ClickHouseDriver(), config, credentialProvider));
+        if (config.isLegacyDriver()) {
+            return new ru.yandex.clickhouse.ClickHouseDriver();
+        }
+        return new com.clickhouse.jdbc.ClickHouseDriver();
     }
 }

plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseConfig.java

@@ -21,6 +21,9 @@ public class ClickHouseConfig
     // TODO (https://github.com/trinodb/trino/issues/7102) reconsider default behavior
     private boolean mapStringAsVarchar;
 
+    // TODO: This config needs to be deprecated when we upgrade clickhouse-jdbc to version 0.4.0 or above
+    private boolean legacyDriver;
+
     public boolean isMapStringAsVarchar()
     {
         return mapStringAsVarchar;
@@ -33,4 +36,19 @@ public ClickHouseConfig setMapStringAsVarchar(boolean mapStringAsVarchar)
         this.mapStringAsVarchar = mapStringAsVarchar;
         return this;
     }
+
+    @Deprecated
+    public boolean isLegacyDriver()
+    {
+        return legacyDriver;
+    }
+
+    @Deprecated
+    @Config("clickhouse.legacy-driver")
+    @ConfigDescription("Whether to use a legacy driver")
+    public ClickHouseConfig setLegacyDriver(boolean legacyDriver)
+    {
+        this.legacyDriver = legacyDriver;
+        return this;
+    }
 }

plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestAltinityConnectorSmokeTest.java

@@ -32,6 +32,7 @@ protected QueryRunner createQueryRunner()
                 ImmutableMap.of(),
                 ImmutableMap.<String, String>builder()
                         .put("clickhouse.map-string-as-varchar", "true") // To handle string types in TPCH tables as varchar instead of varbinary
+                        .put("clickhouse.legacy-driver", "true")
                         .buildOrThrow(),
                 REQUIRED_TPCH_TABLES);
     }

plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConfig.java

@@ -42,18 +42,21 @@ public class TestClickHouseConfig
     public void testDefaults()
     {
         assertRecordedDefaults(recordDefaults(ClickHouseConfig.class)
-                .setMapStringAsVarchar(false));
+                .setMapStringAsVarchar(false)
+                .setLegacyDriver(false));
     }
 
     @Test
     public void testExplicitPropertyMappings()
     {
         Map<String, String> properties = new ImmutableMap.Builder<String, String>()
                 .put("clickhouse.map-string-as-varchar", "true")
+                .put("clickhouse.legacy-driver", "true")
                 .buildOrThrow();
 
         ClickHouseConfig expected = new ClickHouseConfig()
-                .setMapStringAsVarchar(true);
+                .setMapStringAsVarchar(true)
+                .setLegacyDriver(true);
 
         assertFullMapping(properties, expected);
     }

plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java

@@ -129,8 +129,8 @@ public void testDropColumn()
         // the columns are referenced by order_by/order_by property can not be dropped
         assertUpdate("CREATE TABLE " + tableName + "(x int NOT NULL, y int, a int NOT NULL) WITH " +
                 "(engine = 'MergeTree', order_by = ARRAY['x'], partition_by = ARRAY['a'])");
-        assertQueryFails("ALTER TABLE " + tableName + " DROP COLUMN x", "ClickHouse exception, code: 47,.*\\n");
-        assertQueryFails("ALTER TABLE " + tableName + " DROP COLUMN a", "ClickHouse exception, code: 47,.*\\n");
+        assertQueryFails("ALTER TABLE " + tableName + " DROP COLUMN x", "Code: 47,.* Missing columns: 'x' while processing query: 'x', required columns: 'x' 'x' .*\\n.*");
+        assertQueryFails("ALTER TABLE " + tableName + " DROP COLUMN a", "Code: 47,.* Missing columns: 'a' while processing query: 'a', required columns: 'a' 'a' .*\\n.*");
     }
 
     @Override
@@ -293,17 +293,17 @@ public void testTableProperty()
         assertUpdate("DROP TABLE " + tableName);
 
         // Log engine DOES NOT any property
-        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'Log', order_by=ARRAY['id'])", ".* doesn't support PARTITION_BY, PRIMARY_KEY, ORDER_BY or SAMPLE_BY clauses.*\\n");
-        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'Log', partition_by=ARRAY['id'])", ".* doesn't support PARTITION_BY, PRIMARY_KEY, ORDER_BY or SAMPLE_BY clauses.*\\n");
-        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'Log', sample_by='id')", ".* doesn't support PARTITION_BY, PRIMARY_KEY, ORDER_BY or SAMPLE_BY clauses.*\\n");
+        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'Log', order_by=ARRAY['id'])", ".* doesn't support PARTITION_BY, PRIMARY_KEY, ORDER_BY or SAMPLE_BY clauses.*\\n.*");
+        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'Log', partition_by=ARRAY['id'])", ".* doesn't support PARTITION_BY, PRIMARY_KEY, ORDER_BY or SAMPLE_BY clauses.*\\n.*");
+        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'Log', sample_by='id')", ".* doesn't support PARTITION_BY, PRIMARY_KEY, ORDER_BY or SAMPLE_BY clauses.*\\n.*");
 
         // optional properties
         assertUpdate("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'MergeTree', order_by = ARRAY['id'])");
         assertTrue(getQueryRunner().tableExists(getSession(), tableName));
         assertUpdate("DROP TABLE " + tableName);
 
         // the column refers by order by must be not null
-        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'MergeTree', order_by = ARRAY['id', 'x'])", ".* Sorting key cannot contain nullable columns.*\\n");
+        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'MergeTree', order_by = ARRAY['id', 'x'])", ".* Sorting key cannot contain nullable columns.*\\n.*");
 
         assertUpdate("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'MergeTree', order_by = ARRAY['id'], primary_key = ARRAY['id'])");
         assertTrue(getQueryRunner().tableExists(getSession(), tableName));

plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseTypeMapping.java

@@ -28,7 +28,6 @@
 import io.trino.testing.datatype.SqlDataTypeTest;
 import io.trino.testing.sql.TestTable;
 import io.trino.testing.sql.TrinoSqlExecutor;
-import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
@@ -106,7 +105,7 @@ private static void checkIsDoubled(ZoneId zone, LocalDateTime dateTime)
     protected QueryRunner createQueryRunner()
             throws Exception
     {
-        clickhouseServer = new TestingClickHouseServer();
+        clickhouseServer = closeAfterClass(new TestingClickHouseServer());
         return createClickHouseQueryRunner(clickhouseServer, ImmutableMap.of(),
                 ImmutableMap.<String, String>builder()
                         .put("metadata.cache-ttl", "10m")
@@ -115,12 +114,6 @@ protected QueryRunner createQueryRunner()
                 ImmutableList.of());
     }
 
-    @AfterClass(alwaysRun = true)
-    public final void destroy()
-    {
-        clickhouseServer.close();
-    }
-
     @Test
     public void testBasicTypes()
     {

plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestingClickHouseServer.java

@@ -13,6 +13,7 @@
  */
 package io.trino.plugin.clickhouse;
 
+import com.clickhouse.client.ClickHouseVersion;
 import org.testcontainers.containers.ClickHouseContainer;
 import org.testcontainers.utility.DockerImageName;
 
@@ -32,6 +33,11 @@ public class TestingClickHouseServer
     public static final DockerImageName CLICKHOUSE_LATEST_IMAGE = CLICKHOUSE_IMAGE.withTag("21.11.10.1");
     public static final DockerImageName CLICKHOUSE_DEFAULT_IMAGE = CLICKHOUSE_IMAGE.withTag("21.3.2.5"); // EOL is 30 Mar 2022
 
+    private static final String CLICKHOUSE_LATEST_DRIVER_CLASS_NAME = "com.clickhouse.jdbc.ClickHouseDriver";
+    // TODO: This Driver will not be available when clickhouse-jdbc is upgraded to 0.4.0 or above
+    private static final String CLICKHOUSE_DEPRECATED_DRIVER_CLASS_NAME = "ru.yandex.clickhouse.ClickHouseDriver";
+    private static final String CLICKHOUSE_LATEST_DRIVER_MINIMUM_SUPPORTED_VERSION = "20.7";
+
     // Altinity Stable Builds Life-Cycle Table https://docs.altinity.com/altinitystablebuilds/#altinity-stable-builds-life-cycle-table
     private static final DockerImageName ALTINITY_IMAGE = DockerImageName.parse("altinity/clickhouse-server").asCompatibleSubstituteFor("yandex/clickhouse-server");
     public static final DockerImageName ALTINITY_LATEST_IMAGE = ALTINITY_IMAGE.withTag("21.8.13.1.altinitystable");
@@ -46,13 +52,34 @@ public TestingClickHouseServer()
 
     public TestingClickHouseServer(DockerImageName image)
     {
-        dockerContainer = (ClickHouseContainer) new ClickHouseContainer(image)
+        dockerContainer = (ClickHouseContainer) createContainer(image)
                 .withCopyFileToContainer(forClasspathResource("custom.xml"), "/etc/clickhouse-server/config.d/custom.xml")
                 .withStartupAttempts(10);
 
         dockerContainer.start();
     }
 
+    private static ClickHouseContainer createContainer(DockerImageName image)
+    {
+        return new ClickHouseContainer(image)
+        {
+            @Override
+            public String getDriverClassName()
+            {
+                return getClickhouseDriverClassName(image);
+            }
+        };
+    }
+
+    private static String getClickhouseDriverClassName(DockerImageName image)
+    {
+        if (ClickHouseVersion.of(image.getVersionPart()).isNewerOrEqualTo(CLICKHOUSE_LATEST_DRIVER_MINIMUM_SUPPORTED_VERSION)) {
+            return CLICKHOUSE_LATEST_DRIVER_CLASS_NAME;
+        }
+
+        return CLICKHOUSE_DEPRECATED_DRIVER_CLASS_NAME;
+    }
+
     public void execute(String sql)
     {
         try (Connection connection = DriverManager.getConnection(getJdbcUrl());