From b5fa479fd1b396b054bd06ec744be915e640dfea Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Tue, 21 Feb 2023 07:44:00 +0900 Subject: [PATCH] Remove support for legacy driver in ClickHouse --- .../clickhouse/ClickHouseClientModule.java | 15 ++-------- .../plugin/clickhouse/ClickHouseConfig.java | 20 ++----------- .../clickhouse/TestClickHouseConfig.java | 7 ++--- .../clickhouse/TestingClickHouseServer.java | 29 +------------------ 4 files changed, 8 insertions(+), 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 index a70d0b9a753f..96973edcfea0 100644 --- 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 index 43769fec9863..0e405b129fd7 100644 --- 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 index 22caafd51111..07c0a7e3a886 100644 --- 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,8 +42,7 @@ public class TestClickHouseConfig public void testDefaults() { assertRecordedDefaults(recordDefaults(ClickHouseConfig.class) - .setMapStringAsVarchar(false) - .setLegacyDriver(false)); + .setMapStringAsVarchar(false)); } @Test @@ -51,12 +50,10 @@ public void testExplicitPropertyMappings() { Map properties = ImmutableMap.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/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestingClickHouseServer.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestingClickHouseServer.java index 0e88ee3cc497..222ece6cb47f 100644 --- a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestingClickHouseServer.java +++ b/plugin/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());