Remove support for legacy driver in ClickHouse

trinodb · Feb 24, 2023 · b5fa479 · b5fa479
1 parent 24ab0fb
commit b5fa479
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 63 deletions.
diff --git a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClientModule.java b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClientModule.java
@@ -13,6 +13,7 @@
  */
 package io.trino.plugin.clickhouse;
 
+import com.clickhouse.jdbc.ClickHouseDriver;
 import com.google.inject.Binder;
 import com.google.inject.Module;
 import com.google.inject.Provides;
@@ -28,8 +29,6 @@
 import io.trino.plugin.jdbc.JdbcMetadataConfig;
 import io.trino.plugin.jdbc.credential.CredentialProvider;
 
-import java.sql.Driver;
-
 import static io.airlift.configuration.ConfigBinder.configBinder;
 import static io.trino.plugin.clickhouse.ClickHouseClient.DEFAULT_DOMAIN_COMPACTION_THRESHOLD;
 import static io.trino.plugin.jdbc.JdbcModule.bindSessionPropertiesProvider;
@@ -52,16 +51,8 @@ public void configure(Binder binder)
     @Provides
     @Singleton
     @ForBaseJdbc
-    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)
+    public static ConnectionFactory createConnectionFactory(BaseJdbcConfig config, CredentialProvider credentialProvider)
     {
-        if (config.isLegacyDriver()) {
-            return new ru.yandex.clickhouse.ClickHouseDriver();
-        }
-        return new com.clickhouse.jdbc.ClickHouseDriver();
+        return new ClickHouseConnectionFactory(new DriverConnectionFactory(new ClickHouseDriver(), config, credentialProvider));
     }
 }
diff --git a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseConfig.java b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseConfig.java
@@ -15,15 +15,14 @@
 
 import io.airlift.configuration.Config;
 import io.airlift.configuration.ConfigDescription;
+import io.airlift.configuration.DefunctConfig;
 
+@DefunctConfig("clickhouse.legacy-driver")
 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;
@@ -36,19 +35,4 @@ 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;
-    }
 }
diff --git a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConfig.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConfig.java
@@ -42,21 +42,18 @@ public class TestClickHouseConfig
     public void testDefaults()
     {
         assertRecordedDefaults(recordDefaults(ClickHouseConfig.class)
-                .setMapStringAsVarchar(false)
-                .setLegacyDriver(false));
+                .setMapStringAsVarchar(false));
     }
 
     @Test
     public void testExplicitPropertyMappings()
     {
         Map<String, String> properties = ImmutableMap.<String, String>builder()
                 .put("clickhouse.map-string-as-varchar", "true")
-                .put("clickhouse.legacy-driver", "true")
                 .buildOrThrow();
 
         ClickHouseConfig expected = new ClickHouseConfig()
-                .setMapStringAsVarchar(true)
-                .setLegacyDriver(true);
+                .setMapStringAsVarchar(true);
 
         assertFullMapping(properties, expected);
     }

diff --git a/...in/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestingClickHouseServer.java b/...in/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestingClickHouseServer.java
@@ -13,7 +13,6 @@
  */
 package io.trino.plugin.clickhouse;
 
-import com.clickhouse.client.ClickHouseVersion;
 import io.trino.testing.ResourcePresence;
 import org.testcontainers.containers.ClickHouseContainer;
 import org.testcontainers.utility.DockerImageName;
@@ -34,11 +33,6 @@ 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.8.14.5"); // EOL is 31 Aug 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");
@@ -53,34 +47,13 @@ public TestingClickHouseServer()
 
     public TestingClickHouseServer(DockerImageName image)
     {
-        dockerContainer = createContainer(image)
+        dockerContainer = new ClickHouseContainer(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());