diqiu50 commented on code in PR #9776: URL: https://github.com/apache/gravitino/pull/9776#discussion_r2753248656
########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseColumnDefaultValueConverter.java: ########## @@ -1,25 +1,164 @@ /* - * 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 + * 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 + * 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. + * 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.converter; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_OF_CURRENT_TIMESTAMP; + +import java.time.LocalDate; +import java.time.LocalDateTime; import org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter; +import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; +import org.apache.gravitino.rel.expressions.Expression; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.UnparsedExpression; +import org.apache.gravitino.rel.expressions.literals.Literal; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.types.Decimal; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; public class ClickHouseColumnDefaultValueConverter extends JdbcColumnDefaultValueConverter { - // Implement ClickHouse specific default value conversions if needed + + protected static final String NOW = "now"; + protected static final Expression DEFAULT_VALUE_OF_NOW = FunctionExpression.of("now"); + + public String fromGravitino(Expression defaultValue) { + if (DEFAULT_VALUE_NOT_SET.equals(defaultValue)) { + return null; + } + + if (defaultValue instanceof FunctionExpression) { + FunctionExpression functionExpression = (FunctionExpression) defaultValue; + return String.format("(%s)", functionExpression); + } + + if (defaultValue instanceof Literal) { + Literal<?> literal = (Literal<?>) defaultValue; + Type type = literal.dataType(); + if (defaultValue.equals(Literals.NULL)) { + return NULL; + } else if (type instanceof Type.NumericType) { + return literal.value().toString(); + } else { + Object value = literal.value(); + if (value instanceof LocalDateTime) { + value = ((LocalDateTime) value).format(DATE_TIME_FORMATTER); + } + return String.format("'%s'", value); + } + } + + throw new IllegalArgumentException("Not a supported column default value: " + defaultValue); + } + + @Override + public Expression toGravitino( + JdbcTypeConverter.JdbcTypeBean type, + String columnDefaultValue, + boolean isExpression, + boolean nullable) { + if (columnDefaultValue == null || columnDefaultValue.isEmpty()) { + return nullable ? Literals.NULL : DEFAULT_VALUE_NOT_SET; + } + + String reallyType = type.getTypeName(); + if (reallyType.startsWith("Nullable(")) { + reallyType = type.getTypeName().substring(9, type.getTypeName().length() - 1); + } + + if (reallyType.startsWith("Decimal(")) { + reallyType = "Decimal"; + } + + if (reallyType.startsWith("FixedString(")) { + reallyType = "FixedString"; + } + + if (nullable) { + if (columnDefaultValue.equals("NULL")) { + return Literals.NULL; + } + } + + // TODO clickhouse has bug which isExpression is false when is really expression + if (isExpression) { + if (columnDefaultValue.equals(NOW)) { + return DEFAULT_VALUE_OF_NOW; + } + // The parsing of ClickHouse expressions is complex, so we are not currently undertaking the + // parsing. + return UnparsedExpression.of(columnDefaultValue); + } + + // need exclude begin and end "'" + String reallyValue = + columnDefaultValue.startsWith("'") + ? columnDefaultValue.substring(1, columnDefaultValue.length() - 1) + : columnDefaultValue; + + try { + switch (reallyType) { + case ClickHouseTypeConverter.INT8: + return Literals.byteLiteral(Byte.valueOf(reallyValue)); + case ClickHouseTypeConverter.UINT8: + return Literals.unsignedByteLiteral(Short.valueOf(reallyValue)); + case ClickHouseTypeConverter.INT16: + return Literals.shortLiteral(Short.valueOf(reallyValue)); + case ClickHouseTypeConverter.UINT16: + return Literals.unsignedShortLiteral(Integer.valueOf(reallyValue)); + case ClickHouseTypeConverter.INT32: + return Literals.integerLiteral(Integer.valueOf(reallyValue)); + case ClickHouseTypeConverter.UINT32: + return Literals.unsignedIntegerLiteral(Long.valueOf(reallyValue)); + case ClickHouseTypeConverter.INT64: + return Literals.longLiteral(Long.valueOf(reallyValue)); + case ClickHouseTypeConverter.UINT64: + return Literals.unsignedLongLiteral(Decimal.of(reallyValue)); + case ClickHouseTypeConverter.FLOAT32: + return Literals.floatLiteral(Float.valueOf(reallyValue)); + case ClickHouseTypeConverter.FLOAT64: + return Literals.doubleLiteral(Double.valueOf(reallyValue)); + case ClickHouseTypeConverter.DECIMAL: + if (reallyValue.equals("0.")) { + reallyValue = "0.0"; + } + return Literals.decimalLiteral( + Decimal.of(reallyValue, type.getColumnSize(), type.getScale())); + case ClickHouseTypeConverter.DATE: + if (reallyValue.equals("")) { + return Literals.NULL; Review Comment: Should throw exception here ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseColumnDefaultValueConverter.java: ########## @@ -1,25 +1,164 @@ /* - * 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 + * 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 + * 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. + * 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.converter; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_OF_CURRENT_TIMESTAMP; + +import java.time.LocalDate; +import java.time.LocalDateTime; import org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter; +import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; +import org.apache.gravitino.rel.expressions.Expression; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.UnparsedExpression; +import org.apache.gravitino.rel.expressions.literals.Literal; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.types.Decimal; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; public class ClickHouseColumnDefaultValueConverter extends JdbcColumnDefaultValueConverter { - // Implement ClickHouse specific default value conversions if needed + + protected static final String NOW = "now"; + protected static final Expression DEFAULT_VALUE_OF_NOW = FunctionExpression.of("now"); + + public String fromGravitino(Expression defaultValue) { + if (DEFAULT_VALUE_NOT_SET.equals(defaultValue)) { + return null; + } + + if (defaultValue instanceof FunctionExpression) { + FunctionExpression functionExpression = (FunctionExpression) defaultValue; + return String.format("(%s)", functionExpression); + } + + if (defaultValue instanceof Literal) { + Literal<?> literal = (Literal<?>) defaultValue; + Type type = literal.dataType(); + if (defaultValue.equals(Literals.NULL)) { + return NULL; + } else if (type instanceof Type.NumericType) { + return literal.value().toString(); + } else { + Object value = literal.value(); + if (value instanceof LocalDateTime) { + value = ((LocalDateTime) value).format(DATE_TIME_FORMATTER); + } + return String.format("'%s'", value); + } + } + + throw new IllegalArgumentException("Not a supported column default value: " + defaultValue); + } + + @Override + public Expression toGravitino( + JdbcTypeConverter.JdbcTypeBean type, + String columnDefaultValue, + boolean isExpression, + boolean nullable) { + if (columnDefaultValue == null || columnDefaultValue.isEmpty()) { + return nullable ? Literals.NULL : DEFAULT_VALUE_NOT_SET; + } + + String reallyType = type.getTypeName(); + if (reallyType.startsWith("Nullable(")) { + reallyType = type.getTypeName().substring(9, type.getTypeName().length() - 1); Review Comment: Using “Nullable(".lenght" to replace 9 ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java: ########## @@ -38,13 +109,190 @@ protected String generateCreateTableSql( Distribution distribution, Index[] indexes) { throw new UnsupportedOperationException( - "ClickHouseTableOperations.generateCreateTableSql is not implemented yet."); + "generateCreateTableSql with out sortOrders in clickhouse is not supported"); + } + + @Override + protected String generateCreateTableSql( + String tableName, + JdbcColumn[] columns, + String comment, + Map<String, String> properties, + Transform[] partitioning, + Distribution distribution, + Index[] indexes, + SortOrder[] sortOrders) { + + // These two are not yet supported in Gravitino now and will be supported in the future. + if (ArrayUtils.isNotEmpty(partitioning)) { + throw new UnsupportedOperationException( + "Currently we do not support Partitioning in clickhouse"); + } + Preconditions.checkArgument( + Distributions.NONE.equals(distribution), "ClickHouse does not support distribution"); + + validateIncrementCol(columns, indexes); + + // First build the CREATE TABLE statement + StringBuilder sqlBuilder = new StringBuilder(); + sqlBuilder + .append("CREATE TABLE ") + .append(BACK_QUOTE) + .append(tableName) + .append(BACK_QUOTE) + .append(" (\n"); + Review Comment: Using string format is more readable ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java: ########## @@ -1,37 +1,171 @@ /* - * 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 + * 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 + * 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. + * 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.converter; import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; +/** Type converter for ClickHouse. */ public class ClickHouseTypeConverter extends JdbcTypeConverter { + static final String INT8 = "Int8"; + static final String INT16 = "Int16"; + static final String INT32 = "Int32"; + static final String INT64 = "Int64"; + static final String INT128 = "Int128"; + static final String INT256 = "Int256"; + static final String UINT8 = "UInt8"; + static final String UINT16 = "UInt16"; + static final String UINT32 = "UInt32"; + static final String UINT64 = "UInt64"; + static final String UINT128 = "UInt128"; + static final String UINT256 = "UInt256"; + + static final String FLOAT32 = "Float32"; + static final String FLOAT64 = "Float64"; + static final String BFLOAT16 = "BFloat16"; + static final String DECIMAL = "Decimal"; + static final String STRING = "String"; + static final String FIXEDSTRING = "FixedString"; + static final String DATE = "Date"; + static final String DATE32 = "Date32"; + static final String DATETIME = "DateTime"; + static final String DATETIME64 = "DateTime64"; + static final String ENUM = "Enum"; + static final String BOOL = "Bool"; + static final String UUID = "UUID"; + + // bellow is Object Data Type + static final String IPV4 = "IPv4"; + static final String IPV6 = "IPv6"; + static final String ARRAY = "Array"; + static final String TUPLE = "Tuple"; + static final String MAP = "Map"; + static final String VARIANT = "Variant"; + static final String LOWCARDINALITY = "LowCardinality"; + static final String NULLABLE = "Nullable"; + static final String AGGREGATEFUNCTION = "AggregateFunction"; + static final String SIMPLEAGGREGATEFUNCTION = "SimpleAggregateFunction"; + static final String GEO = "Geo"; + + // bellow is Special Data Types + static final String Domains = "Domains"; + static final String Nested = "Nested"; + static final String Dynamic = "Dynamic"; + static final String JSON = "JSON"; @Override - public String fromGravitino(Type type) { - throw new UnsupportedOperationException( - "ClickHouseTypeConverter.fromGravitino is not implemented yet."); + public Type toGravitino(JdbcTypeBean typeBean) { + String typeName = typeBean.getTypeName(); + if (typeName.startsWith("Nullable(")) { + typeName = typeName.substring(9, typeName.length() - 1); + } + + if (typeName.startsWith("Decimal(")) { + typeName = "Decimal"; + } + + if (typeName.startsWith("FixedString(")) { + typeName = "FixedString"; + } + + switch (typeName) { + case INT8: + return Types.ByteType.get(); + case INT16: + return Types.ShortType.get(); + case INT32: + return Types.IntegerType.get(); + case INT64: + return Types.LongType.get(); + case UINT8: + return Types.ByteType.unsigned(); + case UINT16: + return Types.ShortType.unsigned(); + case UINT32: + return Types.IntegerType.unsigned(); + case UINT64: + return Types.LongType.unsigned(); + case FLOAT32: + return Types.FloatType.get(); + case FLOAT64: + return Types.DoubleType.get(); + case DECIMAL: + return Types.DecimalType.of(typeBean.getColumnSize(), typeBean.getScale()); + case STRING: + return Types.StringType.get(); + case FIXEDSTRING: + return Types.FixedCharType.of(typeBean.getColumnSize()); + case DATE: + return Types.DateType.get(); + case DATE32: + return Types.DateType.get(); + case DATETIME: + return Types.TimestampType.withoutTimeZone(); + case DATETIME64: + return Types.TimestampType.withoutTimeZone(); + case BOOL: + return Types.BooleanType.get(); + case UUID: + return Types.UUIDType.get(); + default: + return Types.ExternalType.of(typeBean.getTypeName()); + } } @Override - public Type toGravitino(JdbcTypeBean jdbcTypeBean) { - throw new UnsupportedOperationException( - "ClickHouseTypeConverter.toGravitino is not implemented yet."); + public String fromGravitino(Type type) { + if (type instanceof Types.ByteType byteType) { + return byteType.signed() ? INT8 : UINT8; + } else if (type instanceof Types.ShortType shortType) { + return shortType.signed() ? INT16 : UINT16; + } else if (type instanceof Types.IntegerType integerType) { + return integerType.signed() ? INT32 : UINT32; + } else if (type instanceof Types.LongType longType) { + return longType.signed() ? INT64 : UINT64; + } else if (type instanceof Types.FloatType) { + return FLOAT32; + } else if (type instanceof Types.DoubleType) { + return FLOAT64; + } else if (type instanceof Types.StringType) { + return STRING; + } else if (type instanceof Types.DateType) { + return DATE; + } else if (type instanceof Types.TimestampType) { + return DATETIME; + } else if (type instanceof Types.TimeType) { + return INT64; Review Comment: Why do we return int64？ ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java: ########## @@ -38,13 +109,190 @@ protected String generateCreateTableSql( Distribution distribution, Index[] indexes) { throw new UnsupportedOperationException( - "ClickHouseTableOperations.generateCreateTableSql is not implemented yet."); + "generateCreateTableSql with out sortOrders in clickhouse is not supported"); + } + + @Override + protected String generateCreateTableSql( + String tableName, + JdbcColumn[] columns, + String comment, + Map<String, String> properties, + Transform[] partitioning, + Distribution distribution, + Index[] indexes, + SortOrder[] sortOrders) { + + // These two are not yet supported in Gravitino now and will be supported in the future. + if (ArrayUtils.isNotEmpty(partitioning)) { + throw new UnsupportedOperationException( + "Currently we do not support Partitioning in clickhouse"); + } + Preconditions.checkArgument( + Distributions.NONE.equals(distribution), "ClickHouse does not support distribution"); + + validateIncrementCol(columns, indexes); + + // First build the CREATE TABLE statement + StringBuilder sqlBuilder = new StringBuilder(); + sqlBuilder + .append("CREATE TABLE ") + .append(BACK_QUOTE) + .append(tableName) + .append(BACK_QUOTE) + .append(" (\n"); + + // Add columns + for (int i = 0; i < columns.length; i++) { + JdbcColumn column = columns[i]; + sqlBuilder + .append(SPACE) + .append(SPACE) + .append(BACK_QUOTE) + .append(column.name()) + .append(BACK_QUOTE); + + appendColumnDefinition(column, sqlBuilder); + // Add a comma for the next column, unless it's the last one + if (i < columns.length - 1) { + sqlBuilder.append(",\n"); + } + } + + // Index definition, we only support primary index now, secondary index will be supported in + // future + appendIndexesSql(indexes, sqlBuilder); Review Comment: In ClickHouse, a primary key is optional, and it must be the same as the ORDER BY clause or a subset of it. ########## api/src/main/java/org/apache/gravitino/rel/indexes/Indexes.java: ########## @@ -30,6 +30,11 @@ public class Indexes { /** MySQL does not support setting the name of the primary key, so the default name is used. */ public static final String DEFAULT_MYSQL_PRIMARY_KEY_NAME = "PRIMARY"; + /** + * ClickHouse does not support setting the name of the primary key, so the default name is used. + */ + public static final String DEFAULT_CLICKHOUSE_PRIMARY_KEY_NAME = "PRIMARY"; + /** Review Comment: It's duplicated with DEFAULT_MYSQL_PRIMARY_KEY_NAME. Please keep one.` MYSQL` and `CLICKHOUSE` should not in this class ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseColumnDefaultValueConverter.java: ########## @@ -1,25 +1,164 @@ /* - * 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 + * 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 + * 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. + * 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.converter; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_OF_CURRENT_TIMESTAMP; + +import java.time.LocalDate; +import java.time.LocalDateTime; import org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter; +import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; +import org.apache.gravitino.rel.expressions.Expression; +import org.apache.gravitino.rel.expressions.FunctionExpression; +import org.apache.gravitino.rel.expressions.UnparsedExpression; +import org.apache.gravitino.rel.expressions.literals.Literal; +import org.apache.gravitino.rel.expressions.literals.Literals; +import org.apache.gravitino.rel.types.Decimal; +import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; public class ClickHouseColumnDefaultValueConverter extends JdbcColumnDefaultValueConverter { - // Implement ClickHouse specific default value conversions if needed + + protected static final String NOW = "now"; + protected static final Expression DEFAULT_VALUE_OF_NOW = FunctionExpression.of("now"); Review Comment: Please using the const value `NOW` ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java: ########## @@ -1,37 +1,171 @@ /* - * 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 + * 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 + * 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. + * 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.converter; import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; +/** Type converter for ClickHouse. */ public class ClickHouseTypeConverter extends JdbcTypeConverter { + static final String INT8 = "Int8"; + static final String INT16 = "Int16"; + static final String INT32 = "Int32"; + static final String INT64 = "Int64"; + static final String INT128 = "Int128"; + static final String INT256 = "Int256"; + static final String UINT8 = "UInt8"; + static final String UINT16 = "UInt16"; + static final String UINT32 = "UInt32"; + static final String UINT64 = "UInt64"; + static final String UINT128 = "UInt128"; + static final String UINT256 = "UInt256"; + + static final String FLOAT32 = "Float32"; + static final String FLOAT64 = "Float64"; + static final String BFLOAT16 = "BFloat16"; + static final String DECIMAL = "Decimal"; + static final String STRING = "String"; + static final String FIXEDSTRING = "FixedString"; + static final String DATE = "Date"; + static final String DATE32 = "Date32"; + static final String DATETIME = "DateTime"; + static final String DATETIME64 = "DateTime64"; + static final String ENUM = "Enum"; + static final String BOOL = "Bool"; + static final String UUID = "UUID"; + + // bellow is Object Data Type + static final String IPV4 = "IPv4"; + static final String IPV6 = "IPv6"; + static final String ARRAY = "Array"; + static final String TUPLE = "Tuple"; + static final String MAP = "Map"; + static final String VARIANT = "Variant"; + static final String LOWCARDINALITY = "LowCardinality"; + static final String NULLABLE = "Nullable"; + static final String AGGREGATEFUNCTION = "AggregateFunction"; + static final String SIMPLEAGGREGATEFUNCTION = "SimpleAggregateFunction"; + static final String GEO = "Geo"; + + // bellow is Special Data Types + static final String Domains = "Domains"; + static final String Nested = "Nested"; + static final String Dynamic = "Dynamic"; + static final String JSON = "JSON"; @Override - public String fromGravitino(Type type) { - throw new UnsupportedOperationException( - "ClickHouseTypeConverter.fromGravitino is not implemented yet."); + public Type toGravitino(JdbcTypeBean typeBean) { + String typeName = typeBean.getTypeName(); + if (typeName.startsWith("Nullable(")) { + typeName = typeName.substring(9, typeName.length() - 1); + } + + if (typeName.startsWith("Decimal(")) { + typeName = "Decimal"; + } + + if (typeName.startsWith("FixedString(")) { + typeName = "FixedString"; + } + + switch (typeName) { + case INT8: + return Types.ByteType.get(); + case INT16: + return Types.ShortType.get(); + case INT32: + return Types.IntegerType.get(); + case INT64: + return Types.LongType.get(); + case UINT8: + return Types.ByteType.unsigned(); + case UINT16: + return Types.ShortType.unsigned(); + case UINT32: + return Types.IntegerType.unsigned(); + case UINT64: + return Types.LongType.unsigned(); + case FLOAT32: + return Types.FloatType.get(); + case FLOAT64: + return Types.DoubleType.get(); + case DECIMAL: + return Types.DecimalType.of(typeBean.getColumnSize(), typeBean.getScale()); + case STRING: + return Types.StringType.get(); + case FIXEDSTRING: + return Types.FixedCharType.of(typeBean.getColumnSize()); + case DATE: + return Types.DateType.get(); + case DATE32: + return Types.DateType.get(); + case DATETIME: + return Types.TimestampType.withoutTimeZone(); + case DATETIME64: + return Types.TimestampType.withoutTimeZone(); Review Comment: Should we need to handle the time precision ########## catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java: ########## @@ -106,12 +107,43 @@ public void create( Distribution distribution, Index[] indexes) throws TableAlreadyExistsException { + create( + databaseName, + tableName, + columns, + comment, + properties, + partitioning, + distribution, + indexes, + null); + } + + @Override + public void create( + String databaseName, + String tableName, + JdbcColumn[] columns, + String comment, + Map<String, String> properties, + Transform[] partitioning, + Distribution distribution, + Index[] indexes, + SortOrder[] sortOrders) Review Comment: In the Iceberg catalog, we previously put the sort order into table properties. Is it reasonable to expose it as an interface field just for ClickHouse? How did we handle this for Doris and StarRocks? Do we need to keep it consistent? ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java: ########## @@ -18,16 +18,87 @@ */ package org.apache.gravitino.catalog.clickhouse.operations; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.SETTINGS_PREFIX; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.CLICKHOUSE_ENGINE_KEY; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.ENGINE_PROPERTY_ENTRY; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; + +import com.google.common.base.Preconditions; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; +import org.apache.commons.collections4.MapUtils; +import org.apache.commons.lang3.ArrayUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.gravitino.catalog.jdbc.JdbcColumn; import org.apache.gravitino.catalog.jdbc.operation.JdbcTableOperations; +import org.apache.gravitino.exceptions.NoSuchTableException; import org.apache.gravitino.rel.TableChange; import org.apache.gravitino.rel.expressions.distributions.Distribution; +import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.sorts.SortOrder; import org.apache.gravitino.rel.expressions.transforms.Transform; import org.apache.gravitino.rel.indexes.Index; +import org.apache.gravitino.rel.indexes.Indexes; public class ClickHouseTableOperations extends JdbcTableOperations { + private static final String BACK_QUOTE = "`"; + + @SuppressWarnings("unused") + private static final String CLICKHOUSE_AUTO_INCREMENT = "AUTO_INCREMENT"; + + @SuppressWarnings("unused") + private static final String CLICKHOUSE_NOT_SUPPORT_NESTED_COLUMN_MSG = + "Clickhouse does not support nested column names."; + + @Override + protected List<Index> getIndexes(Connection connection, String databaseName, String tableName) { + // cause clickhouse not impl getPrimaryKeys yet, ref: Review Comment: The index of ClickHouse is different with the relation database. Do we really need to implement it this way? Also, this SQL might not work on some versions. ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/ClickHouseTablePropertiesMetadata.java: ########## @@ -0,0 +1,193 @@ +/* + * 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; + +import static org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.CLUSTER_REMOTE_DATABASE; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseConstants.ON_CLUSTER; +import static org.apache.gravitino.connector.PropertyEntry.enumImmutablePropertyEntry; +import static org.apache.gravitino.connector.PropertyEntry.stringOptionalPropertyEntry; +import static org.apache.gravitino.connector.PropertyEntry.stringReservedPropertyEntry; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.apache.commons.collections4.BidiMap; +import org.apache.commons.collections4.bidimap.TreeBidiMap; +import org.apache.gravitino.catalog.jdbc.JdbcTablePropertiesMetadata; +import org.apache.gravitino.connector.PropertyEntry; + +public class ClickHouseTablePropertiesMetadata extends JdbcTablePropertiesMetadata { + public static final String GRAVITINO_ENGINE_KEY = + ClickHouseConstants.GRAVITINO_CLICKHOUSE_ENGINE_NAME; + public static final String CLICKHOUSE_ENGINE_KEY = ClickHouseConstants.CLICKHOUSE_ENGINE_NAME; + + public static final PropertyEntry<String> COMMENT_PROPERTY_ENTRY = + stringReservedPropertyEntry(COMMENT_KEY, "The table comment", true); + public static final PropertyEntry<ENGINE> ENGINE_PROPERTY_ENTRY = + enumImmutablePropertyEntry( + GRAVITINO_ENGINE_KEY, + "The table engine", + false, + ENGINE.class, + ENGINE.MERGETREE, + false, + false); + public static final PropertyEntry<String> ON_CLUSTER_PROPERTY_ENTRY = + stringOptionalPropertyEntry( + ON_CLUSTER, "The cluster name for ClickHouse distributed tables", false, "", false); + public static final PropertyEntry<String> CLUSTER_REMOTE_DATABASE_PROPERTY_ENTRY = + stringOptionalPropertyEntry( + CLUSTER_REMOTE_DATABASE, + "The remote database name for ClickHouse distributed tables", + false, + "", + false); + public static final PropertyEntry<String> CLUSTER_REMOTE_TABLE_PROPERTY_ENTRY = + stringOptionalPropertyEntry( + ClickHouseConstants.CLUSTER_REMOTE_TABLE, + "The remote table name for ClickHouse distributed tables", + false, + "", + false); + public static final PropertyEntry<String> CLUSTER_SHARDING_KEY_PROPERTY_ENTRY = + stringOptionalPropertyEntry( + ClickHouseConstants.CLUSTER_SHARDING_KEY, + "The sharding key for ClickHouse distributed tables", + false, + "", + false); Review Comment: How are we going to handle this? ClickHouse tables have many properties, and they depend on the table engine type. Are we going to define those properties here? ########## catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java: ########## @@ -0,0 +1,386 @@ +/* + * 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.operations; + +import static org.apache.gravitino.catalog.clickhouse.ClickHouseTablePropertiesMetadata.CLICKHOUSE_ENGINE_KEY; +import static org.apache.gravitino.catalog.clickhouse.ClickHouseUtils.getSortOrders; +import static org.apache.gravitino.rel.Column.DEFAULT_VALUE_NOT_SET; + +import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.commons.lang3.RandomStringUtils; +import org.apache.gravitino.catalog.jdbc.JdbcColumn; +import org.apache.gravitino.catalog.jdbc.JdbcTable; +import org.apache.gravitino.rel.expressions.distributions.Distributions; +import org.apache.gravitino.rel.expressions.literals.Literals; +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.Type; +import org.apache.gravitino.rel.types.Types; +import org.apache.gravitino.utils.RandomNameUtils; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +@Tag("gravitino-docker-test") +public class TestClickHouseTableOperations extends TestClickHouse { + private static final Type STRING = Types.StringType.get(); + private static final Type INT = Types.IntegerType.get(); + + @Test + public void testOperationTable() { Review Comment: What the different with testCreateAndLoadTable ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java: ########## @@ -1,37 +1,171 @@ /* - * 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 + * 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 + * 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. + * 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.converter; import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter; import org.apache.gravitino.rel.types.Type; +import org.apache.gravitino.rel.types.Types; +/** Type converter for ClickHouse. */ public class ClickHouseTypeConverter extends JdbcTypeConverter { + static final String INT8 = "Int8"; + static final String INT16 = "Int16"; + static final String INT32 = "Int32"; + static final String INT64 = "Int64"; + static final String INT128 = "Int128"; + static final String INT256 = "Int256"; + static final String UINT8 = "UInt8"; + static final String UINT16 = "UInt16"; + static final String UINT32 = "UInt32"; + static final String UINT64 = "UInt64"; + static final String UINT128 = "UInt128"; + static final String UINT256 = "UInt256"; + + static final String FLOAT32 = "Float32"; + static final String FLOAT64 = "Float64"; + static final String BFLOAT16 = "BFloat16"; + static final String DECIMAL = "Decimal"; + static final String STRING = "String"; + static final String FIXEDSTRING = "FixedString"; + static final String DATE = "Date"; + static final String DATE32 = "Date32"; + static final String DATETIME = "DateTime"; + static final String DATETIME64 = "DateTime64"; + static final String ENUM = "Enum"; + static final String BOOL = "Bool"; + static final String UUID = "UUID"; + + // bellow is Object Data Type + static final String IPV4 = "IPv4"; + static final String IPV6 = "IPv6"; + static final String ARRAY = "Array"; + static final String TUPLE = "Tuple"; + static final String MAP = "Map"; + static final String VARIANT = "Variant"; + static final String LOWCARDINALITY = "LowCardinality"; + static final String NULLABLE = "Nullable"; + static final String AGGREGATEFUNCTION = "AggregateFunction"; + static final String SIMPLEAGGREGATEFUNCTION = "SimpleAggregateFunction"; + static final String GEO = "Geo"; + + // bellow is Special Data Types + static final String Domains = "Domains"; + static final String Nested = "Nested"; + static final String Dynamic = "Dynamic"; + static final String JSON = "JSON"; @Override - public String fromGravitino(Type type) { - throw new UnsupportedOperationException( - "ClickHouseTypeConverter.fromGravitino is not implemented yet."); + public Type toGravitino(JdbcTypeBean typeBean) { + String typeName = typeBean.getTypeName(); + if (typeName.startsWith("Nullable(")) { + typeName = typeName.substring(9, typeName.length() - 1); + } Review Comment: I suggest extracting a method to handle this consistently. ########## catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java: ########## @@ -38,13 +109,190 @@ protected String generateCreateTableSql( Distribution distribution, Index[] indexes) { throw new UnsupportedOperationException( - "ClickHouseTableOperations.generateCreateTableSql is not implemented yet."); + "generateCreateTableSql with out sortOrders in clickhouse is not supported"); + } + + @Override + protected String generateCreateTableSql( + String tableName, + JdbcColumn[] columns, + String comment, + Map<String, String> properties, + Transform[] partitioning, + Distribution distribution, + Index[] indexes, + SortOrder[] sortOrders) { + + // These two are not yet supported in Gravitino now and will be supported in the future. + if (ArrayUtils.isNotEmpty(partitioning)) { + throw new UnsupportedOperationException( + "Currently we do not support Partitioning in clickhouse"); + } + Preconditions.checkArgument( + Distributions.NONE.equals(distribution), "ClickHouse does not support distribution"); + + validateIncrementCol(columns, indexes); Review Comment: ClickHouse does not support `auto-increment` column -- 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]
