This is an automated email from the ASF dual-hosted git repository.
diqiu50 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 98281f7269 [#13160] fix(postgresql): Preserve unsupported constrained
NUMERIC types (#13163)
98281f7269 is described below
commit 98281f726926b525bfcd795c6a0cd5525c123337
Author: Akshay Thorat <[email protected]>
AuthorDate: Thu Oct 8 19:27:38 2026 -0700
[#13160] fix(postgresql): Preserve unsupported constrained NUMERIC types
(#13163)
### What changes were proposed in this pull request?
Return an external type preserving precision and scale when a
constrained PostgreSQL NUMERIC cannot be represented by Gravitino
DecimalType. Decode unsigned JDBC negative-scale metadata and retain
supported decimal mappings. Add converter and catalog integration tests
and update the mapping documentation.
### Why are the changes needed?
Valid NUMERIC(39,0), NUMERIC(2,-3), and NUMERIC(3,5) columns currently
prevent table loading because their precision or scale exceeds
DecimalType limits.
Fixes #13160
### Does this PR introduce _any_ user-facing change?
Previously failing constrained NUMERIC columns load as external types
such as numeric(39,0), numeric(2,-3), and numeric(3,5). Supported
decimals and unconstrained NUMERIC keep their existing mappings.
The Spark JDBC, Trino PostgreSQL, and Flink type converters use the
ExternalType-to-string fallback added in merged PR #13042. These
unsupported declarations therefore expose string types rather than
native decimals. This is intentional: clamping precision or rounding
scale would misrepresent the source declaration. The documentation now
states this tradeoff.
### How was this patch tested?
`JAVA_HOME=<JDK17> ./gradlew
:catalogs:catalog-jdbc-postgresql:spotlessApply
:catalogs:catalog-jdbc-postgresql:build -PskipITs --max-workers=2`
passed (47 tests). The new converter test failed before the fix. Live
PostgreSQL 18 table operations returned all three exact external
declarations and preserved decimal(10,2). Catalog integration tests
compiled, including PostgreSQL 15+ scale cases; the full Docker
integration suite was not run.
Review follow-up validation: 18 targeted tests passed across PostgreSQL,
Spark 3.5, Flink, and Trino. Explicit regression assertions cover
`numeric(39,0)`, `numeric(2,-3)`, and `numeric(3,5)` in each connector's
type converter. These validate schema type conversion, not end-to-end
engine scans. Added versioned PostgreSQL and pgjdbc source links beside
the scale decoding logic.
```bash
JAVA_HOME=<JDK17> ./gradlew \
:catalogs:catalog-jdbc-postgresql:test --tests
'*.TestPostgreSqlTypeConverter' \
:spark-connector:spark-3.5:test --tests '*.TestSparkJdbcTypeConverter' \
:flink-connector:flink-common:test --tests '*.TestTypeUtils' \
:trino-connector:trino-connector:test --tests
'*.TestPostgreSQLDataTypeTransformer' \
-PscalaVersion=2.12 -PskipITs --max-workers=2
```
---
.../converter/PostgreSqlTypeConverter.java | 15 +++++++++-
.../converter/TestPostgreSqlTypeConverter.java | 15 ++++++++++
.../integration/test/CatalogPostgreSqlIT.java | 32 ++++++++++++++++++++++
docs/jdbc-postgresql-catalog.md | 14 ++++++++--
.../flink/connector/utils/TestTypeUtils.java | 7 +++++
.../connector/jdbc/TestSparkJdbcTypeConverter.java | 7 +++++
.../TestPostgreSQLDataTypeTransformer.java | 8 ++++++
7 files changed, 95 insertions(+), 3 deletions(-)
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java
index c87a63f86c..df3f551731 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java
@@ -86,7 +86,20 @@ public class PostgreSqlTypeConverter extends
JdbcTypeConverter {
if (columnSize == null || columnSize == 0) {
return Types.ExternalType.of(NUMERIC);
}
- return Types.DecimalType.of(columnSize, scale == null ? 0 : scale);
+ int numericScale = scale == null ? 0 : scale;
+ // PostgreSQL stores scale as a signed 11-bit value. JDBC metadata can
expose the
+ // unsigned representation, for example 2045 for NUMERIC(2, -3).
+ // PostgreSQL's numeric_typmod_scale sign-extends the low 11 bits:
+ //
https://github.com/postgres/postgres/blob/REL_15_0/src/backend/utils/adt/numeric.c#L851-L864
+ // pgjdbc 42.7.11 TypeInfoCache.getScale instead returns the low 16
bits unsigned:
+ //
https://github.com/pgjdbc/pgjdbc/blob/REL42.7.11/pgjdbc/src/main/java/org/postgresql/jdbc/TypeInfoCache.java#L863-L875
+ if (numericScale >= 1024 && numericScale <= 2047) {
+ numericScale -= 2048;
+ }
+ if (columnSize > 38 || numericScale < 0 || numericScale > columnSize) {
+ return Types.ExternalType.of(NUMERIC + "(" + columnSize + "," +
numericScale + ")");
+ }
+ return Types.DecimalType.of(columnSize, numericScale);
case VARCHAR:
return typeBean.getColumnSize() == null
? Types.StringType.get()
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java
index 23a65e19a0..bef45a8f9b 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java
@@ -104,6 +104,21 @@ public class TestPostgreSqlTypeConverter {
() -> checkGravitinoTypeToJdbcType(INT_4 + ARRAY_TOKEN, list2));
}
+ @Test
+ public void testConstrainedNumericOutsideDecimalLimits() {
+ checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(39,0)"),
NUMERIC, 39, 0, 0);
+ checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(1000,0)"),
NUMERIC, 1000, 0, 0);
+ checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(2,-3)"),
NUMERIC, 2, -3, 0);
+ // PostgreSQL JDBC metadata can expose a negative scale as an unsigned
11-bit value.
+ checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(2,-3)"),
NUMERIC, 2, 2045, 0);
+ checkJdbcTypeToGravitinoType(Types.ExternalType.of("numeric(3,5)"),
NUMERIC, 3, 5, 0);
+ checkJdbcTypeToGravitinoType(Types.DecimalType.of(38, 38), NUMERIC, 38,
38, 0);
+ checkJdbcTypeToGravitinoType(Types.DecimalType.of(38, 0), NUMERIC, 38,
null, 0);
+ for (String declaration : new String[] {"numeric(39,0)", "numeric(2,-3)",
"numeric(3,5)"}) {
+ checkGravitinoTypeToJdbcType(declaration,
Types.ExternalType.of(declaration));
+ }
+ }
+
@Test
public void testFromGravitinoType() {
checkGravitinoTypeToJdbcType(BOOL, Types.BooleanType.get());
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
index 9441493386..126ae8315d 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
@@ -79,6 +79,7 @@ import org.apache.gravitino.utils.RandomNameUtils;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
@@ -1632,6 +1633,37 @@ public class CatalogPostgreSqlIT extends BaseIT {
Assertions.assertEquals(Types.ExternalType.of("bit"),
loadedTable.columns()[0].dataType());
}
+ @Test
+ void testNumericScaleOutsidePrecision() {
+ Assumptions.assumeTrue(
+ postgreImageName == PGImageName.VERSION_15 || postgreImageName ==
PGImageName.VERSION_16);
+ String tableName = GravitinoITUtils.genRandomName("test_numeric_scale");
+ postgreSqlService.executeQuery(
+ String.format(
+ "CREATE TABLE %s.%s (negative numeric(2,-3), fractional
numeric(3,5));",
+ schemaName, tableName));
+ Table loadedTable =
+ catalog.asTableCatalog().loadTable(NameIdentifier.of(schemaName,
tableName));
+ Assertions.assertEquals(
+ Types.ExternalType.of("numeric(2,-3)"),
loadedTable.columns()[0].dataType());
+ Assertions.assertEquals(
+ Types.ExternalType.of("numeric(3,5)"),
loadedTable.columns()[1].dataType());
+ }
+
+ @Test
+ void testWideNumericTypeConverter() {
+ String tableName =
GravitinoITUtils.genRandomName("test_wide_numeric_type");
+ postgreSqlService.executeQuery(
+ String.format(
+ "CREATE TABLE %s.%s (wide numeric(39,0), supported
numeric(38,0));",
+ schemaName, tableName));
+ Table loadedTable =
+ catalog.asTableCatalog().loadTable(NameIdentifier.of(schemaName,
tableName));
+ Assertions.assertEquals(
+ Types.ExternalType.of("numeric(39,0)"),
loadedTable.columns()[0].dataType());
+ Assertions.assertEquals(Types.DecimalType.of(38, 0),
loadedTable.columns()[1].dataType());
+ }
+
@Test
void testUnconstrainedNumericAndArrayTypeConverter() {
String tableName =
GravitinoITUtils.genRandomName("test_numeric_array_type");
diff --git a/docs/jdbc-postgresql-catalog.md b/docs/jdbc-postgresql-catalog.md
index 8fd3cf0cf9..216f6ae8bf 100644
--- a/docs/jdbc-postgresql-catalog.md
+++ b/docs/jdbc-postgresql-catalog.md
@@ -122,8 +122,18 @@ Meanwhile, the data types other than listed above are
mapped to Gravitino **[Ext
An unconstrained `Numeric` column, that is one declared without precision and
scale, accepts values of up to
131072 digits before and 16383 digits after the decimal point, and its
precision and scale vary per row.
Gravitino `Decimal` caps precision at 38 and is fixed per column, so such a
column is mapped to the External
-Type `numeric` instead. A `Numeric(p, s)` column is mapped to `Decimal(p, s)`
and a `Numeric(p)` column to
-`Decimal(p, 0)` as usual.
+Type `numeric` instead. A `Numeric(p, s)` column maps to `Decimal(p, s)` when
`p` is at most 38
+and `s` is between 0 and `p`. Other constrained declarations, such as
`numeric(39,0)`,
+`numeric(2,-3)`, and `numeric(3,5)`, map to External Types preserving their
precision and scale.
+A `Numeric(p)` column follows the same rules with scale 0.
+
+The Spark JDBC, Trino PostgreSQL, and Flink connectors map these External
Types to strings
+(`StringType`, unbounded `VARCHAR`, and `STRING`, respectively), using the
fallback added in
+[PR #13042](https://github.com/apache/gravitino/pull/13042). This keeps schema
type conversion
+from failing, but does not expose native decimal semantics in those engines.
Consumers needing
+numeric operations must choose an explicit conversion compatible with their
data and engine;
+Gravitino does not clamp precision or round scale to fit `Decimal`. Supported
`Numeric(p, s)`
+columns continue to map to native decimal types.
PostgreSQL array elements always accept NULL and cannot be declared otherwise,
so an `Array` column is always
mapped to a `List` whose elements are nullable. A `List` created with
non-nullable elements is accepted and
diff --git
a/flink-connector/flink-common/src/test/java/org/apache/gravitino/flink/connector/utils/TestTypeUtils.java
b/flink-connector/flink-common/src/test/java/org/apache/gravitino/flink/connector/utils/TestTypeUtils.java
index ce64056c5b..f6701a551d 100644
---
a/flink-connector/flink-common/src/test/java/org/apache/gravitino/flink/connector/utils/TestTypeUtils.java
+++
b/flink-connector/flink-common/src/test/java/org/apache/gravitino/flink/connector/utils/TestTypeUtils.java
@@ -288,6 +288,13 @@ public class TestTypeUtils {
@Test
public void testExternalTypeFromOtherCatalogConversion() {
+ for (String declaration : new String[] {"numeric(39,0)", "numeric(2,-3)",
"numeric(3,5)"}) {
+ Assertions.assertEquals(
+ DataTypes.STRING(),
+ TypeUtils.toFlinkType(Types.ExternalType.of(declaration)),
+ declaration);
+ }
+
// A type name of another data source that parses as a Flink type by
accident: a PostgreSQL
// numeric parses as DECIMAL(10, 0)
Assertions.assertEquals(
diff --git
a/spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/jdbc/TestSparkJdbcTypeConverter.java
b/spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/jdbc/TestSparkJdbcTypeConverter.java
index 044af357d8..4a43b77193 100644
---
a/spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/jdbc/TestSparkJdbcTypeConverter.java
+++
b/spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/jdbc/TestSparkJdbcTypeConverter.java
@@ -47,6 +47,13 @@ public class TestSparkJdbcTypeConverter {
@Test
void testConvertExternalTypeToSparkString() {
+ for (String declaration : new String[] {"numeric(39,0)", "numeric(2,-3)",
"numeric(3,5)"}) {
+ Assertions.assertEquals(
+ DataTypes.StringType,
+
sparkJdbcTypeConverter.toSparkType(Types.ExternalType.of(declaration)),
+ declaration);
+ }
+
Assertions.assertEquals(
DataTypes.StringType,
sparkJdbcTypeConverter.toSparkType(Types.ExternalType.of("numeric")));
Assertions.assertEquals(
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/jdbc/postgresql/TestPostgreSQLDataTypeTransformer.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/jdbc/postgresql/TestPostgreSQLDataTypeTransformer.java
index 7e72fc4fa2..5a7456e9e5 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/jdbc/postgresql/TestPostgreSQLDataTypeTransformer.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/jdbc/postgresql/TestPostgreSQLDataTypeTransformer.java
@@ -19,6 +19,7 @@
package org.apache.gravitino.trino.connector.catalog.jdbc.postgresql;
+import io.trino.spi.type.VarcharType;
import org.apache.gravitino.rel.types.Type;
import org.apache.gravitino.rel.types.Types;
import org.apache.gravitino.trino.connector.util.GeneralDataTypeTransformer;
@@ -60,6 +61,13 @@ public class TestPostgreSQLDataTypeTransformer {
@Test
public void testGravitinoExternalTypeToTrinoType() {
+ for (String declaration : new String[] {"numeric(39,0)", "numeric(2,-3)",
"numeric(3,5)"}) {
+ Assertions.assertEquals(
+ VarcharType.createUnboundedVarcharType(),
+ new
PostgreSQLDataTypeTransformer().getTrinoType(Types.ExternalType.of(declaration)),
+ declaration);
+ }
+
GeneralDataTypeTransformer generalDataTypeTransformer = new
PostgreSQLDataTypeTransformer();
Assertions.assertEquals(
generalDataTypeTransformer.getTrinoType(Types.ExternalType.of("numeric")),
