Copilot commented on code in PR #9878: URL: https://github.com/apache/gravitino/pull/9878#discussion_r2791144847
########## catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java: ########## @@ -0,0 +1,1646 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.clickhouse.integration.test; + +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE.MERGETREE; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.GRAVITINO_ENGINE_KEY; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseUtils.getSortOrders; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import java.io.IOException; +import java.sql.SQLException; +import java.time.LocalDate; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.CatalogChange; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.Schema; +import org.apache.gravitino.SupportsSchemas; +import org.apache.gravitino.auth.AuthConstants; +import org.apache.gravitino.catalog.clickhouse.integration.test.service.ClickHouseService; +import org.apache.gravitino.catalog.jdbc.config.JdbcConfig; +import org.apache.gravitino.client.GravitinoMetalake; +import org.apache.gravitino.exceptions.NoSuchSchemaException; +import org.apache.gravitino.exceptions.NotFoundException; +import org.apache.gravitino.integration.test.container.ClickHouseContainer; +import org.apache.gravitino.integration.test.container.ContainerSuite; +import org.apache.gravitino.integration.test.util.BaseIT; +import org.apache.gravitino.integration.test.util.GravitinoITUtils; +import org.apache.gravitino.integration.test.util.ITUtils; +import org.apache.gravitino.integration.test.util.TestDatabaseName; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.TableCatalog; +import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.UnparsedExpression; +import org.apache.gravitino.rel.expressions.distributions.Distribution; +import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.expressions.sorts.SortOrder; +import org.apache.gravitino.rel.expressions.transforms.Transform; +import org.apache.gravitino.rel.expressions.transforms.Transforms; +import org.apache.gravitino.rel.indexes.Index; +import org.apache.gravitino.rel.indexes.Indexes; +import org.apache.gravitino.rel.types.Decimal; +import org.apache.gravitino.rel.types.Types; +import org.apache.gravitino.utils.RandomNameUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestInstance.Lifecycle; +import org.junit.jupiter.api.condition.EnabledIf; + +@Tag("gravitino-docker-test") +@TestInstance(Lifecycle.PER_CLASS) +public class CatalogClickHouseIT extends BaseIT { + private static final ContainerSuite containerSuite = ContainerSuite.getInstance(); + private static final String provider = "jdbc-clickhouse"; + + public String metalakeName = GravitinoITUtils.genRandomName("clickhouse_it_metalake"); + public String catalogName = GravitinoITUtils.genRandomName("clickhouse_it_catalog"); + public String schemaName = GravitinoITUtils.genRandomName("clickhouse_it_schema"); + public String tableName = GravitinoITUtils.genRandomName("clickhouse_it_table"); + public String alertTableName = "alert_table_name"; + public String table_comment = "table_comment"; + + // ClickHouse doesn't support schema comment + public String schema_comment = null; + public String CLICKHOUSE_COL_NAME1 = "clickhouse_col_name1"; + public String CLICKHOUSE_COL_NAME2 = "clickhouse_col_name2"; + public String CLICKHOUSE_COL_NAME3 = "clickhouse_col_name3"; + public String CLICKHOUSE_COL_NAME4 = "clickhouse_col_name4"; + public String CLICKHOUSE_COL_NAME5 = "clickhouse_col_name5"; + + private GravitinoMetalake metalake; + + protected Catalog catalog; + + private ClickHouseService clickhouseService; + + private ClickHouseContainer CLICKHOUSE_CONTAINER; + + private TestDatabaseName TEST_DB_NAME; + + public static final String defaultClickhouseImageName = "clickhouse:8.0"; + + protected String clickhouseImageName = defaultClickhouseImageName; + Review Comment: `defaultClickhouseImageName` / `clickhouseImageName` are declared but never used in this test class. Please remove them (or wire them into container startup) to avoid dead code; also the tag `clickhouse:8.0` looks unrelated to ClickHouse versioning and may be misleading if kept. ```suggestion ``` ########## integration-test-common/src/test/java/org/apache/gravitino/integration/test/util/BaseIT.java: ########## @@ -194,7 +197,11 @@ private void recoverGravitinoServerConfig() throws IOException { } private void setupJdbcDrivers() throws IOException { - String[] driverUrls = {DOWNLOAD_MYSQL_JDBC_DRIVER_URL, DOWNLOAD_POSTGRESQL_JDBC_DRIVER_URL}; + String[] driverUrls = { + DOWNLOAD_MYSQL_JDBC_DRIVER_URL, + DOWNLOAD_POSTGRESQL_JDBC_DRIVER_URL, + DOWNLOAD_CLICKHOUSE_JDBC_DRIVER_URL + }; Review Comment: The added ClickHouse driver URL will always trigger a warning in `checkAndCleanDriverConflicts` because conflict-cleaning only supports mysql/postgresql patterns. Either add ClickHouse to `SUPPORTED_CLEAN_CONFLICTS_DRIVER_TYPES` (and update the warning text) or skip calling conflict-cleaning for unsupported driver types to avoid noisy logs and potential jar conflicts. ########## catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java: ########## @@ -0,0 +1,1646 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.clickhouse.integration.test; + +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE.MERGETREE; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.GRAVITINO_ENGINE_KEY; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseUtils.getSortOrders; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import java.io.IOException; +import java.sql.SQLException; +import java.time.LocalDate; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.CatalogChange; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.Schema; +import org.apache.gravitino.SupportsSchemas; +import org.apache.gravitino.auth.AuthConstants; +import org.apache.gravitino.catalog.clickhouse.integration.test.service.ClickHouseService; +import org.apache.gravitino.catalog.jdbc.config.JdbcConfig; +import org.apache.gravitino.client.GravitinoMetalake; +import org.apache.gravitino.exceptions.NoSuchSchemaException; +import org.apache.gravitino.exceptions.NotFoundException; +import org.apache.gravitino.integration.test.container.ClickHouseContainer; +import org.apache.gravitino.integration.test.container.ContainerSuite; +import org.apache.gravitino.integration.test.util.BaseIT; +import org.apache.gravitino.integration.test.util.GravitinoITUtils; +import org.apache.gravitino.integration.test.util.ITUtils; +import org.apache.gravitino.integration.test.util.TestDatabaseName; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.TableCatalog; +import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.UnparsedExpression; +import org.apache.gravitino.rel.expressions.distributions.Distribution; +import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.expressions.sorts.SortOrder; +import org.apache.gravitino.rel.expressions.transforms.Transform; +import org.apache.gravitino.rel.expressions.transforms.Transforms; +import org.apache.gravitino.rel.indexes.Index; +import org.apache.gravitino.rel.indexes.Indexes; +import org.apache.gravitino.rel.types.Decimal; +import org.apache.gravitino.rel.types.Types; +import org.apache.gravitino.utils.RandomNameUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestInstance.Lifecycle; +import org.junit.jupiter.api.condition.EnabledIf; + +@Tag("gravitino-docker-test") +@TestInstance(Lifecycle.PER_CLASS) +public class CatalogClickHouseIT extends BaseIT { + private static final ContainerSuite containerSuite = ContainerSuite.getInstance(); + private static final String provider = "jdbc-clickhouse"; + + public String metalakeName = GravitinoITUtils.genRandomName("clickhouse_it_metalake"); + public String catalogName = GravitinoITUtils.genRandomName("clickhouse_it_catalog"); + public String schemaName = GravitinoITUtils.genRandomName("clickhouse_it_schema"); + public String tableName = GravitinoITUtils.genRandomName("clickhouse_it_table"); + public String alertTableName = "alert_table_name"; + public String table_comment = "table_comment"; + + // ClickHouse doesn't support schema comment + public String schema_comment = null; + public String CLICKHOUSE_COL_NAME1 = "clickhouse_col_name1"; + public String CLICKHOUSE_COL_NAME2 = "clickhouse_col_name2"; + public String CLICKHOUSE_COL_NAME3 = "clickhouse_col_name3"; + public String CLICKHOUSE_COL_NAME4 = "clickhouse_col_name4"; + public String CLICKHOUSE_COL_NAME5 = "clickhouse_col_name5"; + + private GravitinoMetalake metalake; + + protected Catalog catalog; + + private ClickHouseService clickhouseService; + + private ClickHouseContainer CLICKHOUSE_CONTAINER; + + private TestDatabaseName TEST_DB_NAME; + + public static final String defaultClickhouseImageName = "clickhouse:8.0"; + + protected String clickhouseImageName = defaultClickhouseImageName; + + boolean SupportColumnDefaultValueExpression() { + return true; + } Review Comment: Method name `SupportColumnDefaultValueExpression` doesn’t follow Java/Google style (methods should be lowerCamelCase). Since it’s referenced by `@EnabledIf`, consider renaming to something like `supportsColumnDefaultValueExpression` and updating the annotation accordingly. ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java: ########## @@ -302,6 +309,11 @@ private void handleDistributeTable( Preconditions.checkArgument( StringUtils.isNotBlank(remoteTable), "Remote table must be specified for Distributed"); + // User must ensure the sharding key is a trusted value. + // TODO(yuqi) WE need to check the columns in shard keys should be integer and not nullable, + // as clickhouse distributed table requires the sharding key to be integer and not nullable. + // We can add this validation after we support user defined sharding key in index, as we can + // reuse the index field definition for validation. Review Comment: This TODO doesn’t include an issue reference. Please link it to an existing issue or file a new one and reference it (project guideline: avoid TODO/FIXME without tracking). ```suggestion // Note: columns used in the sharding key are expected to be integer and not nullable, // as ClickHouse distributed tables require the sharding key to be integer and not nullable. // Validation enforcing this constraint can be added after user-defined sharding keys in // indexes are supported, reusing the index field definition for validation. ``` ########## integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ClickHouseContainer.java: ########## @@ -141,18 +146,60 @@ public String getDriverClassName(TestDatabaseName testDatabaseName) throws SQLEx return DriverManager.getDriver(getJdbcUrl(testDatabaseName)).getClass().getName(); } + public void createDatabaseOnCluster(TestDatabaseName testDatabaseName, String clusterName) { + String clickHouseJdbcUrl = + StringUtils.substring( + getJdbcUrl(testDatabaseName), 0, getJdbcUrl(testDatabaseName).lastIndexOf("/")); + + try (Connection connection = + DriverManager.getConnection(clickHouseJdbcUrl, USER_NAME, getPassword()); + Statement statement = connection.createStatement()) { + + String query = + String.format( + "CREATE DATABASE IF NOT EXISTS %s ON CLUSTER %s;", testDatabaseName, clusterName); + statement.execute(query); + LOG.info( + String.format( + "clickHouse cluster database %s has been created with cluster %s", + testDatabaseName, clusterName)); + } catch (Exception e) { + LOG.error(e.getMessage(), e); + } Review Comment: `createDatabaseOnCluster` swallows failures by only logging the exception. If database creation fails, the subsequent ITs will proceed and fail later with harder-to-diagnose errors. Consider rethrowing as a runtime exception (or returning a boolean and asserting) to fail fast. ########## core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java: ########## @@ -114,6 +115,14 @@ public class CatalogManager implements CatalogDispatcher, Closeable { private static final Logger LOG = LoggerFactory.getLogger(CatalogManager.class); + private static final Set<String> CONTRIB_CATALOGS_TYPES = + new HashSet<>() { + { + add("jdbc-oceanbase"); + add("jdbc-clickhouse"); + } + }; + Review Comment: Double-brace initialization creates an anonymous class and can lead to unnecessary classloading/memory retention. Prefer `Set.of(...)`/`ImmutableSet.of(...)` or a static initializer that populates a plain `HashSet`. ```suggestion private static final Set<String> CONTRIB_CATALOGS_TYPES = new HashSet<>(); static { CONTRIB_CATALOGS_TYPES.add("jdbc-oceanbase"); CONTRIB_CATALOGS_TYPES.add("jdbc-clickhouse"); } ``` ########## catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogOperations.java: ########## @@ -450,8 +450,12 @@ public Table createTable( SortOrder[] sortOrders, Index[] indexes) throws NoSuchSchemaException, TableAlreadyExistsException { - Preconditions.checkArgument( - null == sortOrders || sortOrders.length == 0, "jdbc-catalog does not support sort orders"); + // clickhouse support sortOrders + if (!tableOperation.getClass().getSimpleName().equals("ClickHouseTableOperations")) { + Preconditions.checkArgument( + null == sortOrders || sortOrders.length == 0, + "jdbc-catalog does not support sort orders"); + } Review Comment: This relies on comparing `tableOperation.getClass().getSimpleName()` to a string to decide whether sort orders are allowed. That’s brittle (breaks on refactors, subclasses, shading, proxies). Prefer a type-based check (e.g., `instanceof ClickHouseTableOperations`) or a capability/flag on the operation implementation. ########## common/src/main/java/org/apache/gravitino/dto/rel/expressions/FuncExpressionDTO.java: ########## @@ -64,6 +65,17 @@ public ArgType argType() { return ArgType.FUNCTION; } + /** + * @return The string representation of the function expression. + */ + @Override + public String toString() { + if (functionArgs.length == 0) { + return functionName + "()"; + } + return functionName + "(" + String.join(", ", Arrays.toString(functionArgs)) + ")"; + } Review Comment: `toString()` builds the argument list incorrectly: `String.join(", ", Arrays.toString(functionArgs))` treats `Arrays.toString(...)` as a single element, so output becomes like `fn([a, b])` (extra brackets) and won’t format args properly. Build the string by joining each `functionArg.toString()` (e.g., stream/map) instead. ########## catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java: ########## @@ -0,0 +1,1646 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.catalog.clickhouse.integration.test; + +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE.MERGETREE; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.GRAVITINO_ENGINE_KEY; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseUtils.getSortOrders; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; +import java.io.IOException; +import java.sql.SQLException; +import java.time.LocalDate; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.Catalog; +import org.apache.gravitino.CatalogChange; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.Schema; +import org.apache.gravitino.SupportsSchemas; +import org.apache.gravitino.auth.AuthConstants; +import org.apache.gravitino.catalog.clickhouse.integration.test.service.ClickHouseService; +import org.apache.gravitino.catalog.jdbc.config.JdbcConfig; +import org.apache.gravitino.client.GravitinoMetalake; +import org.apache.gravitino.exceptions.NoSuchSchemaException; +import org.apache.gravitino.exceptions.NotFoundException; +import org.apache.gravitino.integration.test.container.ClickHouseContainer; +import org.apache.gravitino.integration.test.container.ContainerSuite; +import org.apache.gravitino.integration.test.util.BaseIT; +import org.apache.gravitino.integration.test.util.GravitinoITUtils; +import org.apache.gravitino.integration.test.util.ITUtils; +import org.apache.gravitino.integration.test.util.TestDatabaseName; +import org.apache.gravitino.rel.Column; +import org.apache.gravitino.rel.Table; +import org.apache.gravitino.rel.TableCatalog; +import org.apache.gravitino.rel.TableChange; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.UnparsedExpression; +import org.apache.gravitino.rel.expressions.distributions.Distribution; +import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.expressions.sorts.SortOrder; +import org.apache.gravitino.rel.expressions.transforms.Transform; +import org.apache.gravitino.rel.expressions.transforms.Transforms; +import org.apache.gravitino.rel.indexes.Index; +import org.apache.gravitino.rel.indexes.Indexes; +import org.apache.gravitino.rel.types.Decimal; +import org.apache.gravitino.rel.types.Types; +import org.apache.gravitino.utils.RandomNameUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestInstance.Lifecycle; +import org.junit.jupiter.api.condition.EnabledIf; + +@Tag("gravitino-docker-test") +@TestInstance(Lifecycle.PER_CLASS) +public class CatalogClickHouseIT extends BaseIT { + private static final ContainerSuite containerSuite = ContainerSuite.getInstance(); + private static final String provider = "jdbc-clickhouse"; + + public String metalakeName = GravitinoITUtils.genRandomName("clickhouse_it_metalake"); + public String catalogName = GravitinoITUtils.genRandomName("clickhouse_it_catalog"); + public String schemaName = GravitinoITUtils.genRandomName("clickhouse_it_schema"); + public String tableName = GravitinoITUtils.genRandomName("clickhouse_it_table"); + public String alertTableName = "alert_table_name"; + public String table_comment = "table_comment"; + + // ClickHouse doesn't support schema comment Review Comment: The comment says ClickHouse doesn’t support schema comments, but `ClickHouseDatabaseOperations.supportSchemaComment()` returns `true` and schema creation tests rely on comments working. Please update/remove this comment to match current behavior. ```suggestion // Schema comment used in ClickHouse tests (ClickHouse supports schema comments). ``` ########## integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java: ########## @@ -534,10 +540,96 @@ public void startClickHouseContainer(TestDatabaseName testDatabaseName) { } } + public void startClickHouseClusterContainer( + TestDatabaseName testDatabaseName, String remoteServersTemplatePath) { + if (clickHouseClusterContainer == null) { + synchronized (ContainerSuite.class) { + if (clickHouseClusterContainer == null) { + initIfNecessary(); + startZooKeeperContainer(); + String zkHost = zooKeeperContainer.getContainerIpAddress(); + String resolvedConfigPath = prepareRemoteServersConfig(remoteServersTemplatePath, zkHost); + ClickHouseContainer.Builder clickHouseBuilder = + ClickHouseContainer.builder() + .withHostName("gravitino-ci-clickhouse-cluster") + .withEnvVars( + ImmutableMap.<String, String>builder() + .put("CLICKHOUSE_PASSWORD", ClickHouseContainer.PASSWORD) + .build()) + .withRemoteServersConfig(resolvedConfigPath) + .withExposePorts( + ImmutableSet.of( + ClickHouseContainer.CLICKHOUSE_PORT, + ClickHouseContainer.CLICKHOUSE_NATIVE_PORT)) + .withNetwork(network); + + ClickHouseContainer container = closer.register(clickHouseBuilder.build()); + container.start(); + clickHouseClusterContainer = container; + } + } + } + synchronized (ClickHouseContainer.class) { + clickHouseClusterContainer.createDatabaseOnCluster( + testDatabaseName, ClickHouseContainer.DEFAULT_CLUSTER_NAME); + } + } + + private String prepareRemoteServersConfig(String templatePath, String zkHost) { + try { + String content = + new String( + Files.readAllBytes(Paths.get(templatePath)), java.nio.charset.StandardCharsets.UTF_8); + String replaced = + content + .replace("${ZOOKEEPER_HOST}", zkHost) + .replace("${ZOOKEEPER_PORT}", String.valueOf(ZooKeeperContainer.ZK_PORT)); + java.nio.file.Path tempFile = Files.createTempFile("remote_servers", ".xml").toAbsolutePath(); + Files.writeString(tempFile, replaced); + // Change the access mode of temFile to 644 + Files.setPosixFilePermissions( + tempFile, + Set.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.GROUP_READ, + PosixFilePermission.OTHERS_READ)); Review Comment: `Files.setPosixFilePermissions(...)` will throw `UnsupportedOperationException` on non-POSIX filesystems (e.g., Windows). Consider guarding it (check file store supports `posix`) or catching the exception so the cluster ITs can run cross-platform. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
