github-actions[bot] commented on code in PR #65122:
URL: https://github.com/apache/doris/pull/65122#discussion_r3506222061
##########
fe/fe-catalog/src/main/java/org/apache/doris/catalog/ColumnType.java:
##########
@@ -190,62 +190,77 @@ 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
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();
+ return isSupportedStrictNestedPrimitivePromotion(checkType, other);
+ }
- // INT -> BIGINT, LARGEINT
- if (srcType == 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
metadata-only promotions.
+ static boolean isSupportedStrictNestedPrimitivePromotion(Type src, Type
dst) {
+ return isSupportedStrictNestedIntegerPromotion(src.getPrimitiveType(),
dst.getPrimitiveType())
+ ||
isSupportedStrictNestedFloatingPointPromotion(src.getPrimitiveType(),
dst.getPrimitiveType())
+ || isSupportedStrictNestedDecimalPromotion(src, dst);
+ }
- // TINYINT -> SMALLINT, INT, BIGINT, LARGEINT
- if (srcType == PrimitiveType.TINYINT
- && (dstType == 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;
}
-
- // SMALLINT -> INT, BIGINT, LARGEINT
- if (srcType == PrimitiveType.SMALLINT
- && (dstType == PrimitiveType.INT || dstType ==
PrimitiveType.BIGINT
- || dstType == PrimitiveType.LARGEINT)) {
- return true;
+ if (srcType == PrimitiveType.SMALLINT) {
+ return dstType == PrimitiveType.INT || dstType ==
PrimitiveType.BIGINT
+ || dstType == PrimitiveType.LARGEINT;
}
-
- // BIGINT -> LARGEINT
- if (srcType == PrimitiveType.BIGINT && dstType ==
PrimitiveType.LARGEINT) {
- return true;
+ 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;
+ }
+
+ // Decimal nested promotion here is intentionally stricter than top-level
Doris decimal
+ // schema change. This helper is used by both Doris internal complex type
schema change and
+ // Iceberg complex column evolution, so it only accepts metadata-safe
nested promotion:
+ // decimal(P, S) -> decimal(P', S), where P' >= P and S is unchanged.
+ // 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. Changing scale for
nested decimal fields,
+ // such as decimal(5, 1) -> decimal(10, 2), is not supported by the
current nested schema
+ // change path and must be rejected here.
+ private static boolean isSupportedStrictNestedDecimalPromotion(Type src,
Type dst) {
+ if (!(src instanceof ScalarType) || !(dst instanceof ScalarType)) {
+ return false;
+ }
+ PrimitiveType srcType = src.getPrimitiveType();
+ PrimitiveType dstType = dst.getPrimitiveType();
+ if (!(srcType.isDecimalV3Type() || srcType == PrimitiveType.DECIMALV2)
+ || !(dstType.isDecimalV3Type() || dstType ==
PrimitiveType.DECIMALV2)) {
+ return false;
}
- return false;
+ ScalarType srcDecimal = (ScalarType) src;
+ ScalarType dstDecimal = (ScalarType) dst;
+ return srcDecimal.getScalarScale() == dstDecimal.getScalarScale()
Review Comment:
This accepts decimal precision growth even when the nested Doris primitive
changes, but the internal OLAP path can then apply it as a light schema change.
For example, `ARRAY<DECIMALV3(5,2)> -> ARRAY<DECIMALV3(10,2)>` reaches this
return; `ScalarType.createDecimalV3Type` represents those as different
primitive families (`DECIMAL32` vs `DECIMAL64`). With
`light_schema_change=true`, `SchemaChangeHandler.processModifyColumn` treats
any accepted complex-type change as light and goes straight to
`modifyTableLightSchemaChange`, so existing rowsets are not rewritten through
the cast path. BE stores these as different column implementations
(`ColumnDecimal32`, `ColumnDecimal64`, `ColumnDecimal128V3`, etc.; DecimalV2 is
separate too), so changing the nested primitive family is not metadata-only for
internal tables. Please either require the decimal primitive family to stay the
same in this shared helper, or make the OLAP complex-type modify path fall back
to a real schema-change job whenev
er nested decimal widening changes the primitive storage type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
