Skip to content

[fix](iceberg) Support nested decimal precision promotion #65122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Gabriel39 wants to merge 1 commit into
base: master
Choose a base branch
from
+190 −46
Open

[fix](iceberg) Support nested decimal precision promotion #65122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 75 additions & 45 deletions fe/fe-catalog/src/main/java/org/apache/doris/catalog/ColumnType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Comment thread
Gabriel39 marked this conversation as resolved.
&& 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(
Expand All @@ -262,14 +281,24 @@ private static void validateStructFieldCompatibility(StructField originalField,

// deal with type change
if (!originalType.equals(newType)) {
checkSupportSchemaChangeForComplexType(originalType, newType, true);
checkSupportSchemaChangeForComplexType(originalType, newType, true, allowDecimalPrecisionPromotion);
}
}

// This method defines the complex type which is struct, array, map if nested char-type
// 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;
Expand All @@ -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());
}

Expand All @@ -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");
}
Expand Down
2 changes: 1 addition & 1 deletion fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,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);
}
Expand Down
79 changes: 79 additions & 0 deletions fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,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", "");
Expand Down
35 changes: 35 additions & 0 deletions ...e/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -103,6 +106,38 @@ public void testValidateForModifyComplexColumnRejectsIncompatibleNestedType() {
"Cannot change int to smallint in nested types");
}

@Test
public void testValidateForModifyComplexColumnAllowsNestedDecimalPrecisionPromotion() throws Throwable {

@github-actions github-actions Bot Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still only exercises the new decimal promotion through a struct field. The production path this PR enables is recursive for all complex shapes, and after validation IcebergMetadataOps takes different update paths for arrays and maps: array elements go through applyListChange(... updateColumn(elementPath, ...)), while map values go through applyMapChange(... updateColumn(valuePath, ...)). A struct-only reflection test would not catch a bad element/value path or an UpdateSchema incompatibility in those two supported shapes. Please add Iceberg modify validation coverage for at least array<decimal(5,3)> -> array<decimal(10,3)> and map<int,decimal(5,3)> -> map<int,decimal(10,3)> (ideally through the full modify/apply path, not only the shared helper).

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);
Expand Down
Loading