Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update supported clickhouse versions #24515

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/main/sphinx/connector/clickhouse.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ from different catalogs accessing ClickHouse or any other supported data source.

To connect to a ClickHouse server, you need:

- ClickHouse (version 23.8 or higher) or Altinity (version 21.8 or higher).
- ClickHouse (version 24.3 or higher) or Altinity (version 22.3 or higher).
- Network access from the Trino coordinator and workers to the ClickHouse
server. Port 8123 is the default port.

Expand Down
8 changes: 8 additions & 0 deletions plugin/trino-clickhouse/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@
<scope>runtime</scope>
</dependency>

<dependency>
<!-- Clickhouse client will by default use LZ4 compression, which requires this dependency -->
<!-- https://github.com/ClickHouse/clickhouse-docs/pull/2408/files -->
<groupId>org.lz4</groupId>
<artifactId>lz4-java</artifactId>
<scope>runtime</scope>
Praveen2112 marked this conversation as resolved.
Show resolved Hide resolved
</dependency>

<dependency>
<groupId>io.airlift</groupId>
<artifactId>junit-extensions</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.Properties;

import static com.clickhouse.client.config.ClickHouseClientOption.USE_BINARY_STRING;
import static com.clickhouse.jdbc.JdbcConfig.PROP_EXTERNAL_DATABASE;
import static com.google.inject.multibindings.Multibinder.newSetBinder;
import static io.airlift.configuration.ConfigBinder.configBinder;
import static io.trino.plugin.clickhouse.ClickHouseClient.DEFAULT_DOMAIN_COMPACTION_THRESHOLD;
Expand Down Expand Up @@ -64,6 +65,11 @@ public static ConnectionFactory createConnectionFactory(BaseJdbcConfig config, C
Properties properties = new Properties();
// The connector expects byte array for FixedString and String types
properties.setProperty(USE_BINARY_STRING.getKey(), "true");
// externalDatabase=false is needed because Schema listing fetch is extremely slow on Clickhouse-server 24.3+
// https://github.com/ClickHouse/clickhouse-java/issues/1245
// https://github.com/ClickHouse/clickhouse-java/issues/1584
// in Clickhouse itself it has been left `true` by default only for backward compatibility.
properties.setProperty(PROP_EXTERNAL_DATABASE, "false");
return new ClickHouseConnectionFactory(DriverConnectionFactory.builder(new ClickHouseDriver(), config.getConnectionUrl(), credentialProvider)
.setConnectionProperties(properties)
.setOpenTelemetry(openTelemetry)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -709,24 +709,11 @@ protected TestTable simpleTable()
return new TestTable(onRemoteDatabase(), "tpch.simple_table", "(col BIGINT) Engine=Log", ImmutableList.of("1", "2"));
}

@Test
@Override
public void testCreateTableWithLongTableName()
protected void verifyTableNameLengthFailurePermissible(Throwable e)
{
// Override because ClickHouse connector can create a table which can't be dropped
String baseTableName = "test_create_" + randomNameSuffix();
String validTableName = baseTableName + "z".repeat(maxTableNameLength().orElseThrow() - baseTableName.length());

assertUpdate("CREATE TABLE " + validTableName + " (a bigint)");
assertThat(getQueryRunner().tableExists(getSession(), validTableName)).isTrue();
assertThatThrownBy(() -> assertUpdate("DROP TABLE " + validTableName))
.hasMessageMatching("(?s).*(Bad path syntax|File name too long).*");

String invalidTableName = baseTableName + "z".repeat(maxTableNameLength().orElseThrow() - baseTableName.length() + 1);
assertThat(query("CREATE TABLE " + invalidTableName + " (a bigint)"))
.failure().hasMessageMatching("(?s).*(Cannot open file|File name too long).*");
// ClickHouse lefts a table even if the above statement failed
assertThat(getQueryRunner().tableExists(getSession(), validTableName)).isTrue();
assertThat(e).hasMessageMatching(".*The max length of table name for database tpch is %d, current length is [0-9]+.*\n"
.formatted(maxTableNameLength().orElseThrow()));
}

@Test
Expand Down Expand Up @@ -767,31 +754,6 @@ protected void verifySchemaNameLengthFailurePermissible(Throwable e)
assertThat(e).hasMessageContaining("File name too long");
}

@Test
@Override
public void testRenameTableToLongTableName()
{
// Override because ClickHouse connector can rename to a table which can't be dropped
String sourceTableName = "test_source_long_table_name_" + randomNameSuffix();
assertUpdate("CREATE TABLE " + sourceTableName + " AS SELECT 123 x", 1);

String baseTableName = "test_target_long_table_name_" + randomNameSuffix();
// The max length is different from CREATE TABLE case
String validTargetTableName = baseTableName + "z".repeat(255 - ".sql".length() - baseTableName.length());

assertUpdate("ALTER TABLE " + sourceTableName + " RENAME TO " + validTargetTableName);
assertThat(getQueryRunner().tableExists(getSession(), validTargetTableName)).isTrue();
assertQuery("SELECT x FROM " + validTargetTableName, "VALUES 123");
assertThatThrownBy(() -> assertUpdate("DROP TABLE " + validTargetTableName))
.hasMessageMatching("(?s).*(Bad path syntax|File name too long).*");

assertUpdate("CREATE TABLE " + sourceTableName + " AS SELECT 123 x", 1);
String invalidTargetTableName = validTargetTableName + "z";
assertThatThrownBy(() -> assertUpdate("ALTER TABLE " + sourceTableName + " RENAME TO " + invalidTargetTableName))
.hasMessageMatching("(?s).*(Cannot rename|File name too long).*");
assertThat(getQueryRunner().tableExists(getSession(), invalidTargetTableName)).isFalse();
}

@Test
@Override // Override because the failure message differs
public void testNativeQueryIncorrectSyntax()
Expand Down Expand Up @@ -1179,7 +1141,7 @@ public void testExecuteProcedureWithInvalidQuery()
protected OptionalInt maxTableNameLength()
{
// The numeric value depends on file system
return OptionalInt.of(255 - ".sql.detached".length());
return OptionalInt.of(209);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,30 @@
public class TestingClickHouseServer
implements Closeable
{
/**
* <a href="https://clickhouse.com/docs/en/faq/operations/production#how-to-choose-between-clickhouse-releases">How to Choose Between ClickHouse Releases?</a>
* <p>
* stable is the kind of package we recommend by default.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

**stable**

* They are released roughly monthly (and thus provide new features with reasonable delay)
* and three latest stable releases are supported in terms of diagnostics and backporting of bugfixes.
* lts are released twice a year and are supported for a year after their initial release.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New line so to differentiate between **stable**
**lts**

* <p>
* <a href="https://kb.altinity.com/altinity-kb-setup-and-maintenance/clickhouse-versions/">Versioning schema</a>
*/
private static final DockerImageName CLICKHOUSE_IMAGE = DockerImageName.parse("clickhouse/clickhouse-server");
public static final DockerImageName CLICKHOUSE_LATEST_IMAGE = CLICKHOUSE_IMAGE.withTag("24.1.8.22"); // EOL by Apr 2025
public static final DockerImageName CLICKHOUSE_DEFAULT_IMAGE = CLICKHOUSE_IMAGE.withTag("23.8.12.13"); // EOL by Jun 2024
// https://clickhouse.com/docs/en/whats-new/changelog#-clickhouse-release-2412-2024-12-19
public static final DockerImageName CLICKHOUSE_LATEST_IMAGE = CLICKHOUSE_IMAGE.withTag("24.12.1.1614"); // EOL in 3 releases after 2024-12-19
// https://clickhouse.com/docs/en/whats-new/changelog#-clickhouse-release-243-lts-2024-03-27
public static final DockerImageName CLICKHOUSE_DEFAULT_IMAGE = CLICKHOUSE_IMAGE.withTag("24.3.14.35"); // EOL in 1 year after 2024-03-27
ssheikin marked this conversation as resolved.
Show resolved Hide resolved

// Altinity Stable Builds Life-Cycle Table https://docs.altinity.com/altinitystablebuilds/#altinity-stable-builds-life-cycle-table
// On Mac/arm try `21.8.12.29.altinitydev.arm` instead of the specified stable build
/**
* <a href="https://docs.altinity.com/altinitystablebuilds/#altinity-stable-builds-life-cycle-table">Altinity Stable Builds Life-Cycle Table</a>
* <p>
* On Mac/arm 23.3.13.7.altinitystable, 23.8.8.21.altinitystable and 22.8.15.25.altinitystable and later versions available on ARM.
*/
private static final DockerImageName ALTINITY_IMAGE = DockerImageName.parse("altinity/clickhouse-server").asCompatibleSubstituteFor("clickhouse/clickhouse-server");
public static final DockerImageName ALTINITY_LATEST_IMAGE = ALTINITY_IMAGE.withTag("23.8.8.21.altinitystable"); // EOL is 27 Dec 2026
public static final DockerImageName ALTINITY_DEFAULT_IMAGE = ALTINITY_IMAGE.withTag("21.8.15.15.altinitystable"); // EOL is 30 Aug 2024
public static final DockerImageName ALTINITY_LATEST_IMAGE = ALTINITY_IMAGE.withTag("24.3.12.76.altinitystable"); // EOL is 23 Jul 2027
public static final DockerImageName ALTINITY_DEFAULT_IMAGE = ALTINITY_IMAGE.withTag("22.3.15.34.altinitystable"); // EOL is 15 Jul 2025

private final ClickHouseContainer dockerContainer;

Expand Down
17 changes: 16 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@
<dependency>
<groupId>com.clickhouse</groupId>
<artifactId>clickhouse-jdbc</artifactId>
<version>0.6.3</version>
<version>0.7.1-patch1</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we wait for their official release or is patch1 is their official release ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change might allow us to rename the commit message and release-notes as we are updating the client as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like patch suffix is an official way in their release process.

<classifier>all</classifier>
</dependency>

Expand Down Expand Up @@ -2282,6 +2282,14 @@
</exclusions>
</dependency>

<dependency>
<!-- Clickhouse client will by default use LZ4 compression, which requires this dependency -->
<!-- https://github.com/ClickHouse/clickhouse-docs/pull/2408/files -->
<groupId>org.lz4</groupId>
<artifactId>lz4-java</artifactId>
<version>1.8.0</version>
</dependency>

<dependency>
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
Expand Down Expand Up @@ -2602,6 +2610,13 @@
<groupId>org.basepom.maven</groupId>
<artifactId>duplicate-finder-maven-plugin</artifactId>
<configuration>
<ignoredDependencies>
<ignoredDependency>
<!-- clickhouse-jdbc contains conflicting dependencies inside itself -->
<groupId>com.clickhouse</groupId>
<artifactId>clickhouse-jdbc</artifactId>
</ignoredDependency>
</ignoredDependencies>
<exceptions combine.children="append">
<exception>
<conflictingDependencies>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.trino.tests.product.launcher.env.common.TestsEnvironment;
import io.trino.tests.product.launcher.testcontainers.PortBinder;
import org.testcontainers.containers.startupcheck.IsRunningStartupCheckStrategy;
import org.testcontainers.utility.DockerImageName;

import java.io.IOException;
import java.io.UncheckedIOException;
Expand All @@ -47,6 +48,8 @@ public class EnvMultinodeClickhouse
{
private static final String ZOOKEEPER = "zookeeper";

private static final DockerImageName CLICKHOUSE_IMAGE = DockerImageName.parse("clickhouse/clickhouse-server"); // see TestingClickHouseServer for details
private static final DockerImageName CLICKHOUSE_DEFAULT_IMAGE = CLICKHOUSE_IMAGE.withTag("24.3.14.35"); // EOL in 1 year after 2024-03-27
private static final String CLICKHOUSE = "clickhouse";
private static final String CLICKHOUSE_NTH = CLICKHOUSE + "-";
private static final String CONTAINER_CLICKHOUSE_CONFIG_DIR = "/etc/clickhouse-server/";
Expand Down Expand Up @@ -109,7 +112,7 @@ private static DockerContainer createClickHouse(int number, DockerFiles dockerFi
int httpPort = CLICKHOUSE_DEFAULT_HTTP_PORT + number;
int nativePort = CLICKHOUSE_DEFAULT_NATIVE_PORT + number;

DockerContainer container = new DockerContainer("yandex/clickhouse-server:21.3.2.5", logicalName(number))
DockerContainer container = new DockerContainer(CLICKHOUSE_DEFAULT_IMAGE.toString(), logicalName(number))
.withCopyFileToContainer(
forHostPath(dockerFiles.getDockerFilesHostPath("conf/environment/multinode-clickhouse/test.xml")),
CONTAINER_CLICKHOUSE_USERS_D + "test.xml")
Expand Down
8 changes: 8 additions & 0 deletions testing/trino-product-tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,14 @@
<scope>runtime</scope>
</dependency>

<dependency>
<!-- Clickhouse client will by default use LZ4 compression, which requires this dependency -->
<!-- https://github.com/ClickHouse/clickhouse-docs/pull/2408/files -->
<groupId>org.lz4</groupId>
<artifactId>lz4-java</artifactId>
<scope>runtime</scope>
</dependency>

<dependency>
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
Expand Down
Loading