From de258f73cf87ba362add5022955c2dd05fc0e8b1 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Thu, 2 Jul 2026 14:24:55 +0800 Subject: [PATCH] [fix](iceberg) Support nested decimal precision promotion (#65122) Fix Iceberg complex column schema evolution validation for nested decimal precision widening. Doris validates complex column modifications before committing Iceberg schema updates. The previous nested primitive promotion rules covered varchar length growth, exact matches, integer widening, and `FLOAT -> DOUBLE`, but missed Iceberg's valid decimal promotion rule: ```text decimal(P, S) -> decimal(P', S), where P' >= P and S is unchanged ``` This patch keeps the internal table complex type schema-change path on the existing strict nested promotion rules: - integer widening - `FLOAT -> DOUBLE` Iceberg complex column schema evolution uses a separate validation entry point that additionally accepts decimal precision widening when both sides are decimal, scale is unchanged, and destination precision is not smaller. This keeps internal table nested decimal schema-change behavior unchanged. ## Why The internal table schema change matrix is broader than Iceberg schema evolution semantics. It allows conversions such as decimal bucket changes, date/string conversions, and other casts that are not safe metadata-only promotions for Iceberg nested schema evolution. ## Tests ```bash mvn test -pl fe-core -am \ -Dcheckstyle.skip=true \ -DfailIfNoTests=false \ -Dmaven.build.cache.enabled=false \ -Dtest=ColumnTest,IcebergMetadataOpsValidationTest ``` --- .../org/apache/doris/catalog/ColumnType.java | 120 +++++++++++------- .../iceberg/IcebergMetadataOps.java | 2 +- .../org/apache/doris/catalog/ColumnTest.java | 79 ++++++++++++ .../IcebergMetadataOpsValidationTest.java | 35 +++++ 4 files changed, 190 insertions(+), 46 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java index 39e852a4b94b66..332cde4898f1e4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColumnType.java @@ -190,66 +190,85 @@ public static void checkForTypeLengthChange(Type src, Type dst) throws DdlExcept } } - // This method checks if a primitive type change is allowed in nested complex types. - // Supports: - // 1. VARCHAR length increase - // 2. Safe numeric type promotions (INT -> BIGINT, FLOAT -> DOUBLE, etc.) - // 3. Exact type match - private static boolean checkSupportSchemaChangeForNestedPrimitive(Type checkType, Type other) throws DdlException { - // 1. Check VARCHAR length increase + private static boolean checkSupportSchemaChangeForNestedPrimitive(Type checkType, Type other, + boolean allowDecimalPrecisionPromotion) throws DdlException { if (checkType.getPrimitiveType() == PrimitiveType.VARCHAR && other.getPrimitiveType() == PrimitiveType.VARCHAR) { checkForTypeLengthChange(checkType, other); return true; } - // 2. Check exact type match (including STRING == STRING) if (checkType.equals(other)) { return true; } - // 3. Check safe numeric type promotions for nested types - // These are safe promotions that don't lose precision: - // - INT -> BIGINT, LARGEINT - // - FLOAT -> DOUBLE - PrimitiveType srcType = checkType.getPrimitiveType(); - PrimitiveType dstType = other.getPrimitiveType(); - - // INT -> BIGINT, LARGEINT - if (srcType == PrimitiveType.INT - && (dstType == PrimitiveType.BIGINT || dstType == PrimitiveType.LARGEINT)) { + if (allowDecimalPrecisionPromotion && isSupportedIcebergNestedDecimalPromotion(checkType, other)) { return true; } + return isSupportedStrictNestedPrimitivePromotion(checkType, other); + } - // TINYINT -> SMALLINT, INT, BIGINT, LARGEINT - if (srcType == PrimitiveType.TINYINT - && (dstType == PrimitiveType.SMALLINT || dstType == PrimitiveType.INT - || dstType == PrimitiveType.BIGINT || dstType == PrimitiveType.LARGEINT)) { - return true; - } + // This strict nested promotion rule is shared by internal table complex type schema change and + // Iceberg complex column schema evolution. Do not reuse schemaChangeMatrix here, because it + // includes Doris internal cast/rewrite conversions that are not valid nested metadata changes. + static boolean isSupportedStrictNestedPrimitivePromotion(Type src, Type dst) { + return isSupportedStrictNestedIntegerPromotion(src.getPrimitiveType(), dst.getPrimitiveType()) + || isSupportedStrictNestedFloatingPointPromotion(src.getPrimitiveType(), dst.getPrimitiveType()); + } - // SMALLINT -> INT, BIGINT, LARGEINT - if (srcType == PrimitiveType.SMALLINT - && (dstType == PrimitiveType.INT || dstType == PrimitiveType.BIGINT - || dstType == PrimitiveType.LARGEINT)) { - return true; + private static boolean isSupportedStrictNestedIntegerPromotion(PrimitiveType srcType, PrimitiveType dstType) { + // These are safe promotions that don't lose integer precision: + // - TINYINT -> SMALLINT, INT, BIGINT, LARGEINT + // - SMALLINT -> INT, BIGINT, LARGEINT + // - INT -> BIGINT, LARGEINT + // - BIGINT -> LARGEINT + if (srcType == PrimitiveType.TINYINT) { + return dstType == PrimitiveType.SMALLINT || dstType == PrimitiveType.INT + || dstType == PrimitiveType.BIGINT || dstType == PrimitiveType.LARGEINT; } - - // BIGINT -> LARGEINT - if (srcType == PrimitiveType.BIGINT && dstType == PrimitiveType.LARGEINT) { - return true; + if (srcType == PrimitiveType.SMALLINT) { + return dstType == PrimitiveType.INT || dstType == PrimitiveType.BIGINT + || dstType == PrimitiveType.LARGEINT; + } + if (srcType == PrimitiveType.INT) { + return dstType == PrimitiveType.BIGINT || dstType == PrimitiveType.LARGEINT; } + return srcType == PrimitiveType.BIGINT && dstType == PrimitiveType.LARGEINT; + } - // FLOAT -> DOUBLE - if (srcType == PrimitiveType.FLOAT && dstType == PrimitiveType.DOUBLE) { - return true; + private static boolean isSupportedStrictNestedFloatingPointPromotion( + PrimitiveType srcType, PrimitiveType dstType) { + return srcType == PrimitiveType.FLOAT && dstType == PrimitiveType.DOUBLE; + } + + // Iceberg decimal promotion is metadata-only and only allows precision widening with the + // same scale: decimal(P, S) -> decimal(P', S), where P' >= P. + // Scale is part of decimal value interpretation. For example, the same unscaled value 123 + // means 12.3 with scale 1, but 1.23 with scale 2. If scale changes without rewriting old + // files, historical values would be decoded differently, so decimal(5, 1) -> decimal(10, 2) + // must be rejected here. + static boolean isSupportedIcebergNestedDecimalPromotion(Type src, Type dst) { + if (!(src instanceof ScalarType) || !(dst instanceof ScalarType)) { + return false; + } + PrimitiveType srcType = src.getPrimitiveType(); + PrimitiveType dstType = dst.getPrimitiveType(); + if (!isDecimalType(srcType) || !isDecimalType(dstType)) { + return false; } - return false; + ScalarType srcDecimal = (ScalarType) src; + ScalarType dstDecimal = (ScalarType) dst; + return srcDecimal.getScalarScale() == dstDecimal.getScalarScale() + && srcDecimal.getScalarPrecision() <= dstDecimal.getScalarPrecision(); } - private static void validateStructFieldCompatibility(StructField originalField, StructField newField) - throws DdlException { + private static boolean isDecimalType(PrimitiveType type) { + return type.isDecimalV3Type() || type == PrimitiveType.DECIMALV2; + } + + private static void validateStructFieldCompatibility(StructField originalField, StructField newField, + boolean allowDecimalPrecisionPromotion) throws DdlException { // check field name if (!originalField.getName().equals(newField.getName())) { throw new DdlException( @@ -262,7 +281,7 @@ private static void validateStructFieldCompatibility(StructField originalField, // deal with type change if (!originalType.equals(newType)) { - checkSupportSchemaChangeForComplexType(originalType, newType, true); + checkSupportSchemaChangeForComplexType(originalType, newType, true, allowDecimalPrecisionPromotion); } } @@ -270,6 +289,16 @@ private static void validateStructFieldCompatibility(StructField originalField, // to support the schema-change behavior of length growth. public static void checkSupportSchemaChangeForComplexType(Type checkType, Type other, boolean nested) throws DdlException { + checkSupportSchemaChangeForComplexType(checkType, other, nested, false); + } + + public static void checkSupportIcebergSchemaChangeForComplexType(Type checkType, Type other, boolean nested) + throws DdlException { + checkSupportSchemaChangeForComplexType(checkType, other, nested, true); + } + + private static void checkSupportSchemaChangeForComplexType(Type checkType, Type other, boolean nested, + boolean allowDecimalPrecisionPromotion) throws DdlException { if (checkType.isStructType() && other.isStructType()) { StructType thisStructType = (StructType) checkType; StructType otherStructType = (StructType) other; @@ -289,7 +318,7 @@ public static void checkSupportSchemaChangeForComplexType(Type checkType, Type o StructField originalField = originalFields.get(i); StructField newField = newFields.get(i); - validateStructFieldCompatibility(originalField, newField); + validateStructFieldCompatibility(originalField, newField, allowDecimalPrecisionPromotion); existingNames.add(originalField.getName()); } @@ -306,16 +335,17 @@ public static void checkSupportSchemaChangeForComplexType(Type checkType, Type o throw new DdlException("Cannot change " + checkType.toSql() + " to " + other.toSql()); } checkSupportSchemaChangeForComplexType(((ArrayType) checkType).getItemType(), - ((ArrayType) other).getItemType(), true); + ((ArrayType) other).getItemType(), true, allowDecimalPrecisionPromotion); } else if (checkType.isMapType() && other.isMapType()) { checkSupportSchemaChangeForComplexType(((MapType) checkType).getKeyType(), - ((MapType) other).getKeyType(), true); + ((MapType) other).getKeyType(), true, allowDecimalPrecisionPromotion); checkSupportSchemaChangeForComplexType(((MapType) checkType).getValueType(), - ((MapType) other).getValueType(), true); + ((MapType) other).getValueType(), true, allowDecimalPrecisionPromotion); } else { // Support safe type promotions for nested primitive types // if nested is false, we do not check return value. - if (nested && !checkSupportSchemaChangeForNestedPrimitive(checkType, other)) { + if (nested && !checkSupportSchemaChangeForNestedPrimitive(checkType, other, + allowDecimalPrecisionPromotion)) { throw new DdlException( "Cannot change " + checkType.toSql() + " to " + other.toSql() + " in nested types"); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java index c08a822ed0f48e..8276bf71dfdf3a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java @@ -808,7 +808,7 @@ private void validateForModifyComplexColumn(Column column, NestedField currentCo + oldDorisType.toSql() + " to " + newDorisType.toSql()); } try { - ColumnType.checkSupportSchemaChangeForComplexType(oldDorisType, newDorisType, false); + ColumnType.checkSupportIcebergSchemaChangeForComplexType(oldDorisType, newDorisType, false); } catch (DdlException e) { throw new UserException(e.getMessage(), e); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnTest.java index 375f5066146e2c..3e934da2640eba 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnTest.java @@ -157,6 +157,85 @@ public void testSchemaChangeArrayToArray() throws DdlException { oldColumn.checkSchemaChangeAllowed(newColumn); } + @Test + public void testStrictNestedPrimitivePromotionRules() { + Assert.assertTrue(ColumnType.isSupportedStrictNestedPrimitivePromotion(Type.TINYINT, Type.INT)); + Assert.assertTrue(ColumnType.isSupportedStrictNestedPrimitivePromotion(Type.FLOAT, Type.DOUBLE)); + + Assert.assertFalse(ColumnType.isSupportedStrictNestedPrimitivePromotion(Type.INT, Type.FLOAT)); + Assert.assertFalse(ColumnType.isSupportedStrictNestedPrimitivePromotion(Type.VARCHAR, Type.INT)); + Assert.assertFalse(ColumnType.isSupportedStrictNestedPrimitivePromotion( + ScalarType.createDecimalV3Type(5, 2), ScalarType.createDecimalV3Type(10, 2))); + Assert.assertFalse(ColumnType.isSupportedStrictNestedPrimitivePromotion( + ScalarType.createDecimalV3Type(10, 2), ScalarType.createDecimalV3Type(5, 2))); + Assert.assertFalse(ColumnType.isSupportedStrictNestedPrimitivePromotion( + ScalarType.createDecimalV3Type(5, 2), ScalarType.createDecimalV3Type(10, 3))); + } + + @Test + public void testIcebergNestedDecimalPromotionRules() { + Assert.assertTrue(ColumnType.isSupportedIcebergNestedDecimalPromotion( + ScalarType.createDecimalV3Type(5, 2), ScalarType.createDecimalV3Type(10, 2))); + + Assert.assertFalse(ColumnType.isSupportedIcebergNestedDecimalPromotion(Type.INT, Type.BIGINT)); + Assert.assertFalse(ColumnType.isSupportedIcebergNestedDecimalPromotion( + ScalarType.createDecimalV3Type(10, 2), ScalarType.createDecimalV3Type(5, 2))); + Assert.assertFalse(ColumnType.isSupportedIcebergNestedDecimalPromotion( + ScalarType.createDecimalV3Type(5, 2), ScalarType.createDecimalV3Type(10, 3))); + } + + @Test(expected = DdlException.class) + public void testSchemaChangeArrayDecimalPrecisionPromotionRejectedForInternalTable() throws DdlException { + Column oldColumn = new Column("a", ArrayType.create(ScalarType.createDecimalV3Type(5, 2), true), + false, null, true, "0", ""); + Column newColumn = new Column("a", ArrayType.create(ScalarType.createDecimalV3Type(10, 2), true), + false, null, true, "0", ""); + oldColumn.checkSchemaChangeAllowed(newColumn); + Assert.fail("No exception throws."); + } + + @Test(expected = DdlException.class) + public void testSchemaChangeMapDecimalValuePrecisionPromotionRejectedForInternalTable() throws DdlException { + Column oldColumn = new Column("a", new MapType(Type.INT, ScalarType.createDecimalV3Type(5, 2)), + false, null, true, "0", ""); + Column newColumn = new Column("a", new MapType(Type.INT, ScalarType.createDecimalV3Type(10, 2)), + false, null, true, "0", ""); + oldColumn.checkSchemaChangeAllowed(newColumn); + Assert.fail("No exception throws."); + } + + @Test(expected = DdlException.class) + public void testSchemaChangeStructDecimalFieldPrecisionPromotionRejectedForInternalTable() throws DdlException { + Column oldColumn = new Column("a", + new StructType(new StructField("d", ScalarType.createDecimalV3Type(5, 2))), + false, null, true, "0", ""); + Column newColumn = new Column("a", + new StructType(new StructField("d", ScalarType.createDecimalV3Type(10, 2))), + false, null, true, "0", ""); + oldColumn.checkSchemaChangeAllowed(newColumn); + Assert.fail("No exception throws."); + } + + @Test(expected = DdlException.class) + public void testSchemaChangeArrayDecimalPrecisionNarrowing() throws DdlException { + Column oldColumn = new Column("a", ArrayType.create(ScalarType.createDecimalV3Type(10, 2), true), + false, null, true, "0", ""); + Column newColumn = new Column("a", ArrayType.create(ScalarType.createDecimalV3Type(5, 2), true), + false, null, true, "0", ""); + oldColumn.checkSchemaChangeAllowed(newColumn); + Assert.fail("No exception throws."); + } + + @Test(expected = DdlException.class) + public void testSchemaChangeArrayDecimalScaleChange() throws DdlException { + Column oldColumn = new Column("a", ArrayType.create(ScalarType.createDecimalV3Type(5, 2), true), + false, null, true, "0", ""); + Column newColumn = new Column("a", ArrayType.create(ScalarType.createDecimalV3Type(10, 3), true), + false, null, true, "0", ""); + oldColumn.checkSchemaChangeAllowed(newColumn); + Assert.fail("No exception throws."); + } + @Test(expected = DdlException.class) public void testSchemaChangeArrayToArrayDowngrade() throws DdlException { Column oldColumn = new Column("a", ArrayType.create(Type.INT, true), false, null, true, "0", ""); diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java index 74baff368d8a2d..937167c34c351d 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java @@ -20,6 +20,9 @@ import org.apache.doris.catalog.ArrayType; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.MapType; +import org.apache.doris.catalog.ScalarType; +import org.apache.doris.catalog.StructField; +import org.apache.doris.catalog.StructType; import org.apache.doris.catalog.Type; import org.apache.doris.common.UserException; import org.apache.doris.common.security.authentication.ExecutionAuthenticator; @@ -103,6 +106,38 @@ public void testValidateForModifyComplexColumnRejectsIncompatibleNestedType() { "Cannot change int to smallint in nested types"); } + @Test + public void testValidateForModifyComplexColumnAllowsNestedDecimalPrecisionPromotion() throws Throwable { + Column column = new Column("struct_col", + new StructType(new StructField("d", ScalarType.createDecimalV3Type(10, 3))), true); + NestedField currentCol = Types.NestedField.required(1, "struct_col", + Types.StructType.of(Types.NestedField.optional(2, "d", + Types.DecimalType.of(5, 3)))); + invokeValidateForModifyComplexColumn(column, currentCol); + } + + @Test + public void testValidateForModifyComplexColumnRejectsNestedDecimalPrecisionNarrowing() { + Column column = new Column("struct_col", + new StructType(new StructField("d", ScalarType.createDecimalV3Type(5, 3))), true); + NestedField currentCol = Types.NestedField.required(1, "struct_col", + Types.StructType.of(Types.NestedField.optional(2, "d", + Types.DecimalType.of(10, 3)))); + assertUserException(() -> invokeValidateForModifyComplexColumn(column, currentCol), + "Cannot change decimalv3(10,3) to decimalv3(5,3) in nested types"); + } + + @Test + public void testValidateForModifyComplexColumnRejectsNestedDecimalScaleChange() { + Column column = new Column("struct_col", + new StructType(new StructField("d", ScalarType.createDecimalV3Type(10, 4))), true); + NestedField currentCol = Types.NestedField.required(1, "struct_col", + Types.StructType.of(Types.NestedField.optional(2, "d", + Types.DecimalType.of(5, 3)))); + assertUserException(() -> invokeValidateForModifyComplexColumn(column, currentCol), + "Cannot change decimalv3(5,3) to decimalv3(10,4) in nested types"); + } + @Test public void testValidateForModifyComplexColumnRejectsPrimitiveToComplex() { Column column = new Column("arr_i", ArrayType.create(Type.INT, true), true);