diff --git a/docs/src/main/sphinx/connector/clickhouse.rst b/docs/src/main/sphinx/connector/clickhouse.rst index 2834f2ccf738..bce765c9f543 100644 --- a/docs/src/main/sphinx/connector/clickhouse.rst +++ b/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 diff --git a/plugin/trino-clickhouse/pom.xml b/plugin/trino-clickhouse/pom.xml index 2ef605f69605..111695db1504 100644 --- a/plugin/trino-clickhouse/pom.xml +++ b/plugin/trino-clickhouse/pom.xml @@ -46,27 +46,12 @@ com.clickhouse clickhouse-jdbc - 0.3.2-patch1 + 0.3.2-patch3 + all - org.slf4j - slf4j-api - - - commons-logging - commons-logging - - - org.projectlombok - lombok - - - com.clickhouse - clickhouse-http-client - - - org.robolectric - android-all + * + * 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 8954025764f8..625893b217c6 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 @@ -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 schemaName, Optional tableName) - throws SQLException + protected Optional> 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,13 +403,15 @@ public Optional 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()), @@ -424,8 +419,8 @@ public Optional toColumnMapping(ConnectorSession session, Connect // 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 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 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, 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 1a538cda6dd2..c310d1adf729 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 @@ -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(); } } 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 f1cb97d7de39..43769fec9863 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 @@ -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; + } } diff --git a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestAltinityConnectorSmokeTest.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestAltinityConnectorSmokeTest.java index ab9356545922..31824edc5669 100644 --- a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestAltinityConnectorSmokeTest.java +++ b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestAltinityConnectorSmokeTest.java @@ -32,6 +32,7 @@ protected QueryRunner createQueryRunner() ImmutableMap.of(), ImmutableMap.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); } 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 ce9159188aef..201609c1c273 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,7 +42,8 @@ public class TestClickHouseConfig public void testDefaults() { assertRecordedDefaults(recordDefaults(ClickHouseConfig.class) - .setMapStringAsVarchar(false)); + .setMapStringAsVarchar(false) + .setLegacyDriver(false)); } @Test @@ -50,10 +51,12 @@ public void testExplicitPropertyMappings() { Map properties = new ImmutableMap.Builder() .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); } 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 b1f31a75147c..d0df3778b8b6 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 @@ -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,9 +293,9 @@ 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'])"); @@ -303,7 +303,7 @@ public void testTableProperty() 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)); diff --git a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseTypeMapping.java b/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseTypeMapping.java index f96b84e1504a..9704b1f0991e 100644 --- a/plugin/trino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseTypeMapping.java +++ b/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.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() { 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 b4b053ae8c7d..9e430cbb7522 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,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());