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);