yuqi1129 commented on code in PR #11806:
URL: https://github.com/apache/gravitino/pull/11806#discussion_r3602001156
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java:
##########
@@ -112,6 +121,98 @@ ARRAY JOIN arrayZip(splitByChar(',', primary_key),
arrayEnumerate(splitByChar(',
ORDER BY COLUMN_NAME
""";
+ @Override
+ public void create(
+ String databaseName,
+ String tableName,
+ JdbcColumn[] columns,
+ String comment,
+ Map<String, String> properties,
+ Transform[] partitioning,
+ Distribution distribution,
+ Index[] indexes,
+ SortOrder[] sortOrders)
+ throws TableAlreadyExistsException {
+ // When columns is empty (distributed table using AS remote_table), the
shard key validation
+ // in handleDistributeTable is skipped. Fetch remote table columns and
validate here.
+ if (ArrayUtils.isEmpty(columns)) {
+ Map<String, String> props =
+ MapUtils.isNotEmpty(properties) ? properties :
Collections.emptyMap();
+ String engine = props.get(GRAVITINO_ENGINE_KEY);
+ if (StringUtils.isNotEmpty(engine) && ENGINE.DISTRIBUTED ==
ENGINE.fromString(engine)) {
+ String shardingKey = props.get(DistributedTableConstants.SHARDING_KEY);
+ String remoteDb = props.get(DistributedTableConstants.REMOTE_DATABASE);
+ String remoteTbl = props.get(DistributedTableConstants.REMOTE_TABLE);
+ if (StringUtils.isNotBlank(shardingKey)) {
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(remoteDb),
+ "Remote database must be specified for Distributed");
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(remoteTbl), "Remote table must be
specified for Distributed");
+ try (Connection conn = getConnection(databaseName)) {
+ JdbcColumn[] remoteCols = fetchRemoteColumns(conn, remoteDb,
remoteTbl);
+ validateShardKeyColumns(
+ remoteCols, shardingKey, "in remote table
%s.%s".formatted(remoteDb, remoteTbl));
+ } catch (SQLException e) {
+ throw exceptionMapper.toGravitinoException(e);
+ }
+ }
+ }
+ }
+ super.create(
+ databaseName,
+ tableName,
+ columns,
+ comment,
+ properties,
+ partitioning,
+ distribution,
+ indexes,
+ sortOrders);
+ }
Review Comment:
```suggest
if ( ArrayUtils.isNotEmpty(columns)) {
super.create(
databaseName,
tableName,
columns,
comment,
properties,
partitioning,
distribution,
indexes,
sortOrders);
}
....
```
Another thing is, why don't you add that check logic in the following code
block
https://github.com/apache/gravitino/blob/8a07043b9edeb372d53c403b99ba94279c31d6a2/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java#L431-L434
This makes me feel a bit out of place
--
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]
