This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 2885658c03 [#13348] fix(clickhouse): Reject varchar columns without
length support (#13349)
2885658c03 is described below
commit 2885658c03dc8c92626332ca6f8c398cbf40abc3
Author: Qi Yu <[email protected]>
AuthorDate: Sun Sep 20 16:49:59 2026 +0800
[#13348] fix(clickhouse): Reject varchar columns without length support
(#13349)
### What changes were proposed in this pull request?
Reject `VarChar(n)` for ClickHouse table creation and column addition
with an error that names the supported alternatives. Update the type
documentation and tests that previously relied on implicit conversion to
`String`.
### Why are the changes needed?
ClickHouse does not enforce a `varchar(n)` length limit. Gravitino
currently reports success and echoes the requested type, but later loads
the column as `string`.
Fix: #13348
### Does this PR introduce _any_ user-facing change?
Yes. Creating or adding a ClickHouse `varchar(n)` column now fails with
guidance to use `string` or `fixedchar(n)`.
### How was this patch tested?
- `:catalogs-contrib:catalog-jdbc-clickhouse:spotlessCheck`
- `:catalogs-contrib:catalog-jdbc-clickhouse:test -PskipITs --tests
org.apache.gravitino.catalog.clickhouse.converter.TestClickHouseTypeConverter
--tests
org.apache.gravitino.catalog.clickhouse.operations.TestClickHouseTableOperationsUnit`
(37 tests passed)
---
.../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 cc4d9c8adf..3ad50a84b7 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 855fcd51ef..ee2824733a 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
@@ -248,7 +248,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,
@@ -1032,7 +1032,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,
@@ -1043,7 +1043,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,
@@ -1092,7 +1092,7 @@ public class CatalogClickHouseIT extends BaseIT {
Literals.doubleLiteral(123.45)),
Column.of(
"string_col",
- Types.VarCharType.of(255),
+ Types.StringType.get(),
"string",
false,
false,
@@ -1782,13 +1782,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
@@ -1815,7 +1815,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(
@@ -1832,7 +1832,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(
@@ -1849,7 +1849,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(
@@ -2255,7 +2255,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");
@@ -2690,8 +2690,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 088709c1cd..8ca2c98ece 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 736f327c68..fbbffcca15 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
@@ -82,6 +82,10 @@ public class TestClickHouseTableOperationsUnit {
.withNullable(false)
.build()
};
+ return callGenerateCreateTableSql(columns, properties);
+ }
+
+ String callGenerateCreateTableSql(JdbcColumn[] columns, Map<String,
String> properties) {
return generateCreateTableSql(
"test_table",
columns,
@@ -112,6 +116,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 9674448af0..56ecf81e6a 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
