This is an automated email from the ASF dual-hosted git repository. jerryshao pushed a commit to branch branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 837d7d2d4f3e36ee2c61e542f401028263a1cb96 Author: yuqi <[email protected]> AuthorDate: Sun Sep 20 15:12:05 2026 +0800 fix(clickhouse): reject varchar columns without length support (cherry picked from commit 61518d12327fb05dd106007e6298720764c6c816) --- .../converter/ClickHouseTypeConverter.java | 4 +++- .../converter/TestClickHouseTypeConverter.java | 13 +++++++++++- .../integration/test/CatalogClickHouseIT.java | 24 +++++++++++----------- .../operations/TestClickHouseTableOperations.java | 2 +- .../TestClickHouseTableOperationsUnit.java | 22 ++++++++++++++++++++ docs/jdbc-clickhouse-catalog.md | 4 +++- 6 files changed, 53 insertions(+), 16 deletions(-) diff --git a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java index 03f184b1f7..2672b2b8f4 100644 --- a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java +++ b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/ClickHouseTypeConverter.java @@ -210,7 +210,9 @@ public class ClickHouseTypeConverter extends JdbcTypeConverter { } else if (type instanceof Types.DecimalType decimalType) { return String.format("%s(%s,%s)", DECIMAL, decimalType.precision(), decimalType.scale()); } else if (type instanceof Types.VarCharType) { - return STRING; + throw new IllegalArgumentException( + "ClickHouse does not support varchar(n) length limits; use string for unlimited text " + + "or fixedchar(n) for a fixed-length value"); } else if (type instanceof Types.FixedCharType fixedCharType) { return FIXEDSTRING + "(" + fixedCharType.length() + ")"; } else if (type instanceof Types.BooleanType) { diff --git a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/converter/TestClickHouseTypeConverter.java b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/converter/TestClickHouseTypeConverter.java index 9298f85a8d..6440d3d0f1 100644 --- a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/converter/TestClickHouseTypeConverter.java +++ b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/converter/TestClickHouseTypeConverter.java @@ -167,7 +167,6 @@ public class TestClickHouseTypeConverter { checkGravitinoTypeToJdbcType(DATE, Types.DateType.get()); checkGravitinoTypeToJdbcType(DATETIME, Types.TimestampType.withoutTimeZone(0)); checkGravitinoTypeToJdbcType(DECIMAL + "(10,2)", Types.DecimalType.of(10, 2)); - checkGravitinoTypeToJdbcType(STRING, Types.VarCharType.of(20)); checkGravitinoTypeToJdbcType(FIXEDSTRING + "(20)", Types.FixedCharType.of(20)); checkGravitinoTypeToJdbcType(STRING, Types.StringType.get()); checkGravitinoTypeToJdbcType(BOOL, Types.BooleanType.get()); @@ -197,6 +196,18 @@ public class TestClickHouseTypeConverter { () -> CLICKHOUSE_TYPE_CONVERTER.fromGravitino(Types.UnparsedType.of(USER_DEFINED_TYPE))); } + @Test + public void testRejectVarcharWithoutLengthSupport() { + IllegalArgumentException exception = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> CLICKHOUSE_TYPE_CONVERTER.fromGravitino(Types.VarCharType.of(64))); + Assertions.assertTrue(exception.getMessage().contains("ClickHouse")); + Assertions.assertTrue(exception.getMessage().contains("varchar(n)")); + Assertions.assertTrue(exception.getMessage().contains("string")); + Assertions.assertTrue(exception.getMessage().contains("fixedchar(n)")); + } + protected void checkGravitinoTypeToJdbcType(String jdbcTypeName, Type gravitinoType) { Assertions.assertEquals(jdbcTypeName, CLICKHOUSE_TYPE_CONVERTER.fromGravitino(gravitinoType)); } diff --git a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java index 45057b5eff..8da517460d 100644 --- a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java +++ b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/integration/test/CatalogClickHouseIT.java @@ -246,7 +246,7 @@ public class CatalogClickHouseIT extends BaseIT { FunctionExpression.of("now")), Column.of( CLICKHOUSE_COL_NAME3, - Types.VarCharType.of(255), + Types.StringType.get(), "col_3_comment", true, false, @@ -882,7 +882,7 @@ public class CatalogClickHouseIT extends BaseIT { Column col3 = Column.of( CLICKHOUSE_COL_NAME3, - Types.VarCharType.of(255), + Types.StringType.get(), "col_3_comment", true, false, @@ -893,7 +893,7 @@ public class CatalogClickHouseIT extends BaseIT { Column col5 = Column.of( CLICKHOUSE_COL_NAME5, - Types.VarCharType.of(255), + Types.StringType.get(), "col_5_comment", true, false, @@ -942,7 +942,7 @@ public class CatalogClickHouseIT extends BaseIT { Literals.doubleLiteral(123.45)), Column.of( "string_col", - Types.VarCharType.of(255), + Types.StringType.get(), "string", false, false, @@ -1631,13 +1631,13 @@ public class CatalogClickHouseIT extends BaseIT { NameIdentifier.of(schemaName, tableName), TableChange.updateColumnDefaultValue( new String[] {columns[1].name()}, FunctionExpression.of("now"))); - // Change default value of varchar + // Change default value of string catalog .asTableCatalog() .alterTable( NameIdentifier.of(schemaName, tableName), TableChange.updateColumnDefaultValue( - new String[] {columns[2].name()}, Literals.of("hello", Types.VarCharType.of(255)))); + new String[] {columns[2].name()}, Literals.stringLiteral("hello"))); // Change default value of int catalog @@ -1664,7 +1664,7 @@ public class CatalogClickHouseIT extends BaseIT { TableChange.updateColumnDefaultValue( new String[] {columns[1].name()}, FunctionExpression.of("now")), TableChange.updateColumnDefaultValue( - new String[] {columns[2].name()}, Literals.of("hello", Types.VarCharType.of(255))), + new String[] {columns[2].name()}, Literals.stringLiteral("hello")), TableChange.updateColumnDefaultValue( new String[] {columns[3].name()}, Literals.of("2000", Types.IntegerType.get())), TableChange.updateColumnDefaultValue( @@ -1681,7 +1681,7 @@ public class CatalogClickHouseIT extends BaseIT { TableChange.updateColumnDefaultValue( new String[] {columns[1].name()}, FunctionExpression.of("now")), TableChange.updateColumnDefaultValue( - new String[] {columns[2].name()}, Literals.of("hello", Types.VarCharType.of(255))), + new String[] {columns[2].name()}, Literals.stringLiteral("hello")), TableChange.updateColumnDefaultValue( new String[] {columns[3].name()}, Literals.of("2000", Types.IntegerType.get())), TableChange.updateColumnDefaultValue( @@ -1698,7 +1698,7 @@ public class CatalogClickHouseIT extends BaseIT { TableChange.updateColumnDefaultValue( new String[] {columns[1].name()}, FunctionExpression.of("now")), TableChange.updateColumnDefaultValue( - new String[] {columns[2].name()}, Literals.of("hello", Types.VarCharType.of(255))), + new String[] {columns[2].name()}, Literals.stringLiteral("hello")), TableChange.updateColumnDefaultValue( new String[] {columns[3].name()}, Literals.of("2000", Types.IntegerType.get())), TableChange.updateColumnDefaultValue( @@ -2104,7 +2104,7 @@ public class CatalogClickHouseIT extends BaseIT { Column col1 = Column.of("create", Types.LongType.get(), "id", false, false, null); Column col2 = Column.of("delete", Types.ByteType.get(), "yes", false, false, null); Column col3 = Column.of("show", Types.DateType.get(), "comment", false, false, null); - Column col4 = Column.of("status", Types.VarCharType.of(255), "code", false, false, null); + Column col4 = Column.of("status", Types.StringType.get(), "code", false, false, null); Column[] newColumns = new Column[] {col1, col2, col3, col4}; TableCatalog tableCatalog = catalog.asTableCatalog(); NameIdentifier tableIdentifier = NameIdentifier.of(schemaName, "table"); @@ -2539,8 +2539,8 @@ public class CatalogClickHouseIT extends BaseIT { @Test void testClickHouseSchemaNameCaseSensitive() { Column col1 = Column.of("col_1", Types.LongType.get(), "id", false, false, null); - Column col2 = Column.of("col_2", Types.VarCharType.of(255), "code", false, false, null); - Column col3 = Column.of("col_3", Types.VarCharType.of(255), "config", false, false, null); + Column col2 = Column.of("col_2", Types.StringType.get(), "code", false, false, null); + Column col3 = Column.of("col_3", Types.StringType.get(), "config", false, false, null); Column[] newColumns = new Column[] {col1, col2, col3}; String[] schemas = {"db_", "db_1", "db_2", "db12"}; diff --git a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java index 5f3fdbd8ea..4ce265908f 100644 --- a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java +++ b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java @@ -637,7 +637,7 @@ public class TestClickHouseTableOperations extends TestClickHouse { columns.add( JdbcColumn.builder() .withName("c_varchar") - .withType(Types.VarCharType.of(5)) + .withType(Types.StringType.get()) .withNullable(false) .build()); columns.add( diff --git a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java index 357d2783ee..5dec3386a0 100644 --- a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java +++ b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperationsUnit.java @@ -77,6 +77,10 @@ public class TestClickHouseTableOperationsUnit { .withNullable(false) .build() }; + return callGenerateCreateTableSql(columns, properties); + } + + String callGenerateCreateTableSql(JdbcColumn[] columns, Map<String, String> properties) { return generateCreateTableSql( "test_table", columns, @@ -93,6 +97,24 @@ public class TestClickHouseTableOperationsUnit { return newOps(null); } + @Test + void testCreateTableRejectsVarchar() { + JdbcColumn[] columns = + new JdbcColumn[] { + JdbcColumn.builder() + .withName("name") + .withType(Types.VarCharType.of(64)) + .withNullable(true) + .build() + }; + + IllegalArgumentException exception = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> newOps().callGenerateCreateTableSql(columns, Map.of())); + Assertions.assertTrue(exception.getMessage().contains("ClickHouse does not support varchar")); + } + private ExposedClickHouseTableOperations newOps(DataSource dataSource) { ExposedClickHouseTableOperations ops = new ExposedClickHouseTableOperations(); ops.initialize( diff --git a/docs/jdbc-clickhouse-catalog.md b/docs/jdbc-clickhouse-catalog.md index 2371ea2bcc..c0f7ee27e4 100644 --- a/docs/jdbc-clickhouse-catalog.md +++ b/docs/jdbc-clickhouse-catalog.md @@ -192,7 +192,7 @@ See [Manage Catalogs and Schemas](./manage-catalogs-and-schemas.md#schema-operat | `Float` | `Float32` | | `Double` | `Float64` | | `Decimal(p,s)` | `Decimal(p,s)` | -| `String`/`VarChar` | `String` | +| `String` | `String` | | `FixedChar(n)` | `FixedString(n)` | | `Date` | `Date` | | `Timestamp[(p)]` | `DateTime` (precision defaults to `0`) | @@ -200,6 +200,8 @@ See [Manage Catalogs and Schemas](./manage-catalogs-and-schemas.md#schema-operat | `UUID` | `UUID` | Other ClickHouse types are exposed as [External Type](./tables-and-views.md#external-type). +`VarChar(n)` is rejected when creating or adding a column because ClickHouse cannot enforce its length limit. +Use `String` for unlimited text or `FixedChar(n)` for fixed-length values. ### Table Properties
