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 b096dd2c42 [#11879] fix(clickhouse): fix type conversion for Decimal,
DateTime64, LowCardinality, and IPv4/IPv6 (#11884)
b096dd2c42 is described below
commit b096dd2c42b041fcf4aa03af0c713394fd94bc94
Author: StormSpirit <[email protected]>
AuthorDate: Fri Jul 3 19:40:01 2026 +0800
[#11879] fix(clickhouse): fix type conversion for Decimal, DateTime64,
LowCardinality, and IPv4/IPv6 (#11884)
### What changes were proposed in this pull request?
Fix four type conversion issues in the ClickHouse catalog's
`ClickHouseTypeConverter`:
1. **Decimal(precision > 38) → ExternalType**: ClickHouse supports
Decimal up to precision 76
(Decimal128/Decimal256), but Gravitino core `DecimalType` enforces
precision ≤ 38. Tables with
`Decimal(50, 10)` etc. previously threw `IllegalArgumentException` on
`loadTable`. Now mapped to
`ExternalType` for precision > 38, consistent with the existing pattern
for non-standard types.
2. **DateTime64(N) precision preserved on round-trip**:
`TypeUtils.extractDateTimePrecision()`
regex did not match `DateTime64(3)` — the entire type fell through to
`ExternalType`. Extended
the regex to `^DateTime(?:64)?\((\d+)\)$`. In `fromGravitino`,
`TimestampType` with
precision > 0 now returns `DateTime64(N)` instead of bare `DateTime`.
3. **LowCardinality wrapper stripped before Nullable**: For
`LowCardinality(Nullable(String))`, the outermost wrapper is
`LowCardinality`. Added
`TypeUtils.stripLowCardinality()` and call it before `stripNullable()`
in `toGravitino()`.
4. **IPv4/IPv6 mapped to ExternalType**: Constants `IPV4` and `IPV6`
were declared but had no
`case` branches. Added `case IPV4:` / `case IPV6:` returning
`ExternalType.of(...)`.
### Why are the changes needed?
Fix: #11879
These issues cause ClickHouse tables with common column types
(Decimal(50,x), DateTime64,
LowCardinality, IPv4/IPv6) to either crash on `loadTable` or lose type
semantics on round-trip.
### Does this PR introduce _any_ user-facing change?
Yes. Tables with Decimal(precision > 38), DateTime64(N),
LowCardinality(Nullable(...)),
IPv4, or IPv6 columns can now be loaded without errors. Type round-trip
fidelity is improved.
### How was this patch tested?
- New unit tests in `TestClickHouseTypeConverter`: `./gradlew
:catalogs-contrib:catalog-jdbc-clickhouse:test -PskipITs`
(7 test scenarios: 4 toGravitino assertions — IPv6, DateTime64(3),
LowCardinality(Nullable(String)),
Decimal(50,10); 3 fromGravitino direction checks — DateTime64(3), IPv4,
IPv6)
- Updated existing Docker IT assertion in
`TestClickHouseTableOperations.testTypeConversionAgainstCluster`:
`c_dt64` column type changed from `ExternalType.of("DateTime64(3)")` to
`TimestampType.withoutTimeZone(3)`
to match the new `toGravitino()` behavior
- Docker integration tests: `./gradlew
:catalogs-contrib:catalog-jdbc-clickhouse:test -PskipDockerTests=false
--tests "TestClickHouseTableOperations"`
Signed-off-by: jiangxt2 <[email protected]>
---
.../converter/ClickHouseTypeConverter.java | 23 ++++++++++++++---
.../catalog/clickhouse/converter/TypeUtils.java | 6 ++++-
.../converter/TestClickHouseTypeConverter.java | 29 ++++++++++++++++++++++
.../operations/TestClickHouseTableOperations.java | 4 +--
4 files changed, 55 insertions(+), 7 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 14acbfa84c..8915a8c0c3 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
@@ -76,7 +76,10 @@ public class ClickHouseTypeConverter extends
JdbcTypeConverter {
@Override
public Type toGravitino(JdbcTypeBean typeBean) {
- String typeName = TypeUtils.stripNullable(typeBean.getTypeName());
+ // ClickHouse allows LowCardinality wrapping Nullable:
LowCardinality(Nullable(String)).
+ // Nullable(LowCardinality(X)) is invalid in ClickHouse and not handled
here.
+ String typeName = TypeUtils.stripLowCardinality(typeBean.getTypeName());
+ typeName = TypeUtils.stripNullable(typeName);
Integer dateTimePrecision = TypeUtils.extractDateTimePrecision(typeName);
if (dateTimePrecision != null) {
@@ -126,6 +129,11 @@ public class ClickHouseTypeConverter extends
JdbcTypeConverter {
String.format("Decimal scale %s is out of range [0, %s]", scale,
precision));
}
+ // ClickHouse supports Decimal up to precision 76 (Decimal128=38,
Decimal256=76),
+ // but Gravitino core DecimalType enforces precision <= 38. Use
ExternalType for larger.
+ if (precision > 38) {
+ return Types.ExternalType.of(String.format("%s(%s,%s)", DECIMAL,
precision, scale));
+ }
return Types.DecimalType.of(precision, scale);
case STRING:
return Types.StringType.get();
@@ -141,6 +149,10 @@ public class ClickHouseTypeConverter extends
JdbcTypeConverter {
return Types.BooleanType.get();
case UUID:
return Types.UUIDType.get();
+ case IPV4:
+ return Types.ExternalType.of(IPV4);
+ case IPV6:
+ return Types.ExternalType.of(IPV6);
default:
return Types.ExternalType.of(typeBean.getTypeName());
}
@@ -164,9 +176,12 @@ public class ClickHouseTypeConverter extends
JdbcTypeConverter {
return STRING;
} else if (type instanceof Types.DateType) {
return DATE;
- } else if (type instanceof Types.TimestampType) {
- // Gravitino timestamp type maps to ClickHouse DateTime with precision
0, and
- // Use the external type to handle DateTime64
+ } else if (type instanceof Types.TimestampType timestampType) {
+ // Gravitino timestamp type maps to ClickHouse DateTime with precision 0.
+ // For precision > 0, use DateTime64(N).
+ if (timestampType.precision() > 0) {
+ return DATETIME64 + "(" + timestampType.precision() + ")";
+ }
return DATETIME;
} else if (type instanceof Types.TimeType) {
return TIME;
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/TypeUtils.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/TypeUtils.java
index 8a9bb76bb4..7ac74a143d 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/TypeUtils.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/converter/TypeUtils.java
@@ -29,8 +29,12 @@ public class TypeUtils {
return typeName.replaceFirst("^Nullable\\((.*)\\)$", "$1");
}
+ public static String stripLowCardinality(String typeName) {
+ return typeName.replaceFirst("^LowCardinality\\((.*)\\)$", "$1");
+ }
+
public static Integer extractDateTimePrecision(String typeName) {
- Matcher matcher =
Pattern.compile("^DateTime\\((\\d+)\\)$").matcher(typeName);
+ Matcher matcher =
Pattern.compile("^DateTime(?:64)?\\((\\d+)\\)$").matcher(typeName);
if (matcher.matches()) {
return Integer.parseInt(matcher.group(1));
}
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 36e2ad51dc..c644724994 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
@@ -31,6 +31,8 @@ import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeCo
import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter.INT32;
import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter.INT64;
import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter.INT8;
+import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter.IPV4;
+import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter.IPV6;
import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter.STRING;
import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter.UINT16;
import static
org.apache.gravitino.catalog.clickhouse.converter.ClickHouseTypeConverter.UINT32;
@@ -95,6 +97,28 @@ public class TestClickHouseTypeConverter {
Assertions.assertEquals(
Types.ExternalType.of("IPv4"),
CLICKHOUSE_TYPE_CONVERTER.toGravitino(ipv4));
+ JdbcTypeConverter.JdbcTypeBean ipv6 = createTypeBean("IPv6", null, null);
+ Assertions.assertEquals(
+ Types.ExternalType.of("IPv6"),
CLICKHOUSE_TYPE_CONVERTER.toGravitino(ipv6));
+
+ // DateTime64(3) should map to TimestampType.withoutTimeZone(3)
+ JdbcTypeConverter.JdbcTypeBean dateTime64WithPrecision =
+ createTypeBean("DateTime64(3)", null, null);
+ Assertions.assertEquals(
+ Types.TimestampType.withoutTimeZone(3),
+ CLICKHOUSE_TYPE_CONVERTER.toGravitino(dateTime64WithPrecision));
+
+ // LowCardinality(Nullable(String)) should map to StringType
+ JdbcTypeConverter.JdbcTypeBean lowCardNullable =
+ createTypeBean("LowCardinality(Nullable(String))", null, null);
+ Assertions.assertEquals(
+ Types.StringType.get(),
CLICKHOUSE_TYPE_CONVERTER.toGravitino(lowCardNullable));
+
+ // Decimal(50, 10) should map to ExternalType (not crash, since precision
> 38)
+ JdbcTypeConverter.JdbcTypeBean decimal50 = createTypeBean("Decimal", 50,
10);
+ Assertions.assertEquals(
+ Types.ExternalType.of("Decimal(50,10)"),
CLICKHOUSE_TYPE_CONVERTER.toGravitino(decimal50));
+
JdbcTypeConverter.JdbcTypeBean decimalTooLarge = createTypeBean("Decimal",
77, 2);
Assertions.assertThrows(
IllegalArgumentException.class,
@@ -128,6 +152,11 @@ public class TestClickHouseTypeConverter {
checkGravitinoTypeToJdbcType(UUID, Types.UUIDType.get());
checkGravitinoTypeToJdbcType(USER_DEFINED_TYPE,
Types.ExternalType.of(USER_DEFINED_TYPE));
checkGravitinoTypeToJdbcType("DateTime",
Types.TimestampType.withoutTimeZone(0));
+ // DateTime64(3) round-trip
+ checkGravitinoTypeToJdbcType(DATETIME64 + "(3)",
Types.TimestampType.withoutTimeZone(3));
+ // IPv4/IPv6 round-trip
+ checkGravitinoTypeToJdbcType(IPV4, Types.ExternalType.of(IPV4));
+ checkGravitinoTypeToJdbcType(IPV6, Types.ExternalType.of(IPV6));
checkGravitinoTypeToJdbcType(TIME, Types.TimeType.get());
Assertions.assertThrows(
IllegalArgumentException.class,
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 68f72d2e12..93c928a539 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
@@ -589,7 +589,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
columns.add(
JdbcColumn.builder()
.withName("c_dt64")
- .withType(Types.ExternalType.of("DateTime64(3)"))
+ .withType(Types.TimestampType.withoutTimeZone(3))
.withNullable(false)
.build());
columns.add(
@@ -647,7 +647,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
Assertions.assertEquals(
Types.TimestampType.withoutTimeZone(0),
loaded.columns()[14].dataType());
Assertions.assertEquals(
- Types.ExternalType.of("DateTime64(3)"),
loaded.columns()[15].dataType());
+ Types.TimestampType.withoutTimeZone(3),
loaded.columns()[15].dataType());
Assertions.assertEquals(Types.BooleanType.get(),
loaded.columns()[16].dataType());
Assertions.assertEquals(Types.UUIDType.get(),
loaded.columns()[17].dataType());
Assertions.assertEquals(Types.ExternalType.of("IPv4"),
loaded.columns()[18].dataType());
