This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-2.1 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 9a94681b292f32b4e45543c5d8b8a5ebf7eca928 Author: morrySnow <[email protected]> AuthorDate: Thu May 9 10:51:41 2024 +0800 [refactor](type) AggStateType should not extends ScalarType (#34463) 1. let AggStateType extends Type 2. remove useless interface isFixedLengthType and supportsTablePartitioning 3. let MapType implement interface isSupported 4. let VariantType extends ScalarType --- .../org/apache/doris/catalog/AggStateType.java | 49 +++++++++++++++++----- .../java/org/apache/doris/catalog/ArrayType.java | 12 +----- .../java/org/apache/doris/catalog/MapType.java | 5 +++ .../java/org/apache/doris/catalog/ScalarType.java | 22 ---------- .../java/org/apache/doris/catalog/StructType.java | 13 +----- .../main/java/org/apache/doris/catalog/Type.java | 39 ++++++++++------- .../java/org/apache/doris/catalog/VariantType.java | 46 ++------------------ .../main/java/org/apache/doris/analysis/Expr.java | 4 +- .../apache/doris/nereids/types/AggStateType.java | 2 +- .../agg_state/max/test_agg_state_max.groovy | 2 +- 10 files changed, 78 insertions(+), 116 deletions(-) diff --git a/fe/fe-common/src/main/java/org/apache/doris/catalog/AggStateType.java b/fe/fe-common/src/main/java/org/apache/doris/catalog/AggStateType.java index 72f93a2f696..e28364103f4 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/catalog/AggStateType.java +++ b/fe/fe-common/src/main/java/org/apache/doris/catalog/AggStateType.java @@ -17,16 +17,19 @@ package org.apache.doris.catalog; +import org.apache.doris.thrift.TScalarType; import org.apache.doris.thrift.TTypeDesc; import org.apache.doris.thrift.TTypeNode; +import org.apache.doris.thrift.TTypeNodeType; -import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.gson.annotations.SerializedName; import java.util.ArrayList; import java.util.List; +import java.util.Objects; -public class AggStateType extends ScalarType { +public class AggStateType extends Type { @SerializedName(value = "subTypes") private List<Type> subTypes; @@ -42,11 +45,12 @@ public class AggStateType extends ScalarType { public AggStateType(String functionName, Boolean resultIsNullable, List<Type> subTypes, List<Boolean> subTypeNullables) { - super(PrimitiveType.AGG_STATE); - Preconditions.checkState(subTypes != null); - Preconditions.checkState(subTypeNullables != null); - Preconditions.checkState(subTypes.size() == subTypeNullables.size(), - "AggStateType' subTypes.size()!=subTypeNullables.size()"); + Objects.requireNonNull(subTypes, "subTypes should not be null"); + Objects.requireNonNull(subTypeNullables, "subTypeNullables should not be null"); + if (subTypes.size() != subTypeNullables.size()) { + throw new IllegalStateException("AggStateType's subTypes.size() [" + subTypes.size() + + "] != subTypeNullables.size() [" + subTypeNullables.size() + "]"); + } this.functionName = functionName; this.subTypes = subTypes; this.subTypeNullables = subTypeNullables; @@ -86,13 +90,24 @@ public class AggStateType extends ScalarType { return stringBuilder.toString(); } + @Override + protected String prettyPrint(int lpad) { + return Strings.repeat(" ", lpad) + toSql(); + } + @Override public void toThrift(TTypeDesc container) { - super.toThrift(container); - List<TTypeDesc> types = new ArrayList<TTypeDesc>(); + TTypeNode node = new TTypeNode(); + container.types.add(node); + // ATTN: use scalar only for compatibility. + node.setType(TTypeNodeType.SCALAR); + TScalarType scalarType = new TScalarType(); + scalarType.setType(getPrimitiveType().toThrift()); + node.setScalarType(scalarType); + List<TTypeDesc> types = new ArrayList<>(); for (int i = 0; i < subTypes.size(); i++) { TTypeDesc desc = new TTypeDesc(); - desc.setTypes(new ArrayList<TTypeNode>()); + desc.setTypes(new ArrayList<>()); subTypes.get(i).toThrift(desc); desc.setIsNullable(subTypeNullables.get(i)); types.add(desc); @@ -122,4 +137,18 @@ public class AggStateType extends ScalarType { } return true; } + + public PrimitiveType getPrimitiveType() { + return PrimitiveType.AGG_STATE; + } + + @Override + public int getSlotSize() { + return PrimitiveType.AGG_STATE.getSlotSize(); + } + + @Override + public boolean matchesType(Type t) { + return t.isAggStateType() || t.isStringType(); + } } diff --git a/fe/fe-common/src/main/java/org/apache/doris/catalog/ArrayType.java b/fe/fe-common/src/main/java/org/apache/doris/catalog/ArrayType.java index 27f02c802cb..19a55a06755 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/catalog/ArrayType.java +++ b/fe/fe-common/src/main/java/org/apache/doris/catalog/ArrayType.java @@ -198,7 +198,7 @@ public class ArrayType extends Type { @Override public boolean isSupported() { - return !itemType.isNull(); + return itemType.isSupported() && !itemType.isNull(); } @Override @@ -223,16 +223,6 @@ public class ArrayType extends Type { return thrift; } - @Override - public boolean isFixedLengthType() { - return false; - } - - @Override - public boolean supportsTablePartitioning() { - return false; - } - @Override public int getSlotSize() { return PrimitiveType.ARRAY.getSlotSize(); diff --git a/fe/fe-common/src/main/java/org/apache/doris/catalog/MapType.java b/fe/fe-common/src/main/java/org/apache/doris/catalog/MapType.java index 1291a5f364e..53cd926ebe7 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/catalog/MapType.java +++ b/fe/fe-common/src/main/java/org/apache/doris/catalog/MapType.java @@ -250,4 +250,9 @@ public class MapType extends Type { public int hashCode() { return Objects.hash(keyType, valueType); } + + @Override + public boolean isSupported() { + return keyType.isSupported() && !keyType.isNull() && valueType.isSupported() && !valueType.isNull(); + } } diff --git a/fe/fe-common/src/main/java/org/apache/doris/catalog/ScalarType.java b/fe/fe-common/src/main/java/org/apache/doris/catalog/ScalarType.java index 12cf7af02ba..4b4155c3d77 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/catalog/ScalarType.java +++ b/fe/fe-common/src/main/java/org/apache/doris/catalog/ScalarType.java @@ -704,9 +704,6 @@ public class ScalarType extends Type { case JSONB: stringBuilder.append("JSON"); break; - case AGG_STATE: - stringBuilder.append("AGG_STATE(UNKNOWN)"); - break; default: stringBuilder.append("unknown type: ").append(type); break; @@ -860,17 +857,6 @@ public class ScalarType extends Type { return type == PrimitiveType.CHAR && (len == -1 || len == MAX_CHAR_LENGTH); } - @Override - public boolean isFixedLengthType() { - return type == PrimitiveType.BOOLEAN || type == PrimitiveType.TINYINT - || type == PrimitiveType.SMALLINT || type == PrimitiveType.INT - || type == PrimitiveType.BIGINT || type == PrimitiveType.FLOAT - || type == PrimitiveType.DOUBLE || type == PrimitiveType.DATE - || type == PrimitiveType.DATETIME || type == PrimitiveType.DECIMALV2 || type.isDecimalV3Type() - || type == PrimitiveType.CHAR || type == PrimitiveType.DATEV2 || type == PrimitiveType.DATETIMEV2 - || type == PrimitiveType.TIMEV2; - } - @Override public boolean isSupported() { switch (type) { @@ -882,14 +868,6 @@ public class ScalarType extends Type { } } - @Override - public boolean supportsTablePartitioning() { - if (!isSupported() || isComplexType()) { - return false; - } - return true; - } - @Override public int getSlotSize() { return type.getSlotSize(); diff --git a/fe/fe-common/src/main/java/org/apache/doris/catalog/StructType.java b/fe/fe-common/src/main/java/org/apache/doris/catalog/StructType.java index 4f21286c636..e447f2dceeb 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/catalog/StructType.java +++ b/fe/fe-common/src/main/java/org/apache/doris/catalog/StructType.java @@ -18,7 +18,6 @@ package org.apache.doris.catalog; import org.apache.doris.thrift.TColumnType; -import org.apache.doris.thrift.TStructField; import org.apache.doris.thrift.TTypeDesc; import org.apache.doris.thrift.TTypeNode; import org.apache.doris.thrift.TTypeNodeType; @@ -320,7 +319,7 @@ public class StructType extends Type { Preconditions.checkNotNull(fields); Preconditions.checkState(!fields.isEmpty()); node.setType(TTypeNodeType.STRUCT); - node.setStructFields(new ArrayList<TStructField>()); + node.setStructFields(new ArrayList<>()); for (StructField field : fields) { field.toThrift(container, node); } @@ -342,16 +341,6 @@ public class StructType extends Type { return thrift; } - @Override - public boolean isFixedLengthType() { - return false; - } - - @Override - public boolean supportsTablePartitioning() { - return false; - } - @Override public int getSlotSize() { return PrimitiveType.STRUCT.getSlotSize(); diff --git a/fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java b/fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java index 258a0f57bfd..46cda54753e 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java +++ b/fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java @@ -600,7 +600,7 @@ public abstract class Type { } public boolean isScalarType(PrimitiveType t) { - return isScalarType() && ((ScalarType) this).getPrimitiveType() == t; + return isScalarType() && this.getPrimitiveType() == t; } public boolean isFixedPointType() { @@ -632,11 +632,6 @@ public abstract class Type { return isScalarType(PrimitiveType.LARGEINT); } - // TODO: Handle complex types properly. Some instances may be fixed length. - public boolean isFixedLengthType() { - return false; - } - public boolean isNumericType() { return isFixedPointType() || isFloatingPointType() || isDecimalV2() || isDecimalV3(); } @@ -679,7 +674,7 @@ public abstract class Type { } public boolean isAggStateType() { - return isScalarType(PrimitiveType.AGG_STATE); + return this instanceof AggStateType; } public boolean isMultiRowType() { @@ -771,13 +766,6 @@ public abstract class Type { return -1; } - /** - * Indicates whether we support partitioning tables on columns of this type. - */ - public boolean supportsTablePartitioning() { - return false; - } - public PrimitiveType getPrimitiveType() { return PrimitiveType.INVALID_TYPE; } @@ -862,6 +850,27 @@ public abstract class Type { return true; } else if (sourceType.isStructType() && targetType.isStructType()) { return StructType.canCastTo((StructType) sourceType, (StructType) targetType); + } else if (sourceType.isAggStateType() && targetType.getPrimitiveType().isCharFamily()) { + return true; + } else if (sourceType.isAggStateType() && targetType.isAggStateType()) { + AggStateType sourceAggState = (AggStateType) sourceType; + AggStateType targetAggState = (AggStateType) targetType; + if (!sourceAggState.getFunctionName().equalsIgnoreCase(targetAggState.getFunctionName())) { + return false; + } + if (sourceAggState.getSubTypes().size() != targetAggState.getSubTypes().size()) { + return false; + } + for (int i = 0; i < sourceAggState.getSubTypes().size(); i++) { + // target subtype is not null but source subtype is nullable + if (!targetAggState.getSubTypeNullables().get(i) && sourceAggState.getSubTypeNullables().get(i)) { + return false; + } + if (!canCastTo(sourceAggState.getSubTypes().get(i), targetAggState.getSubTypes().get(i))) { + return false; + } + } + return true; } return sourceType.isNull() || sourceType.getPrimitiveType().isCharFamily(); } @@ -967,7 +976,7 @@ public abstract class Type { MapType mapType = (MapType) this; return mapType.getValueType().exceedsMaxNestingDepth(d + 1); } else { - Preconditions.checkState(isScalarType()); + Preconditions.checkState(isScalarType() || isAggStateType()); } return false; } diff --git a/fe/fe-common/src/main/java/org/apache/doris/catalog/VariantType.java b/fe/fe-common/src/main/java/org/apache/doris/catalog/VariantType.java index e9363dd0141..924b197e4d7 100644 --- a/fe/fe-common/src/main/java/org/apache/doris/catalog/VariantType.java +++ b/fe/fe-common/src/main/java/org/apache/doris/catalog/VariantType.java @@ -21,58 +21,20 @@ import org.apache.doris.thrift.TTypeDesc; import org.apache.doris.thrift.TTypeNode; import org.apache.doris.thrift.TTypeNodeType; -public class VariantType extends Type { - public VariantType() { - - } +public class VariantType extends ScalarType { - @Override - public String toSql(int depth) { - return "VARIANT"; - } - - @Override - protected String prettyPrint(int lpad) { - return "VARIANT"; - } - - @Override - public boolean equals(Object other) { - return other instanceof VariantType; + public VariantType() { + super(PrimitiveType.VARIANT); } @Override public void toThrift(TTypeDesc container) { + // not use ScalarType's toThrift for compatibility, because VariantType is not extends ScalarType previously TTypeNode node = new TTypeNode(); container.types.add(node); node.setType(TTypeNodeType.VARIANT); } - @Override - public String toString() { - return toSql(0); - } - - @Override - public PrimitiveType getPrimitiveType() { - return PrimitiveType.VARIANT; - } - - @Override - public boolean supportsTablePartitioning() { - return false; - } - - @Override - public int getSlotSize() { - return PrimitiveType.VARIANT.getSlotSize(); - } - - @Override - public boolean isSupported() { - return true; - } - @Override public boolean matchesType(Type t) { return t.isVariantType() || t.isStringType(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java index 91fd0dc9827..c31bacbf87b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java @@ -1922,11 +1922,11 @@ public abstract class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl f.setNullableMode(NullableMode.ALWAYS_NOT_NULLABLE); } else { Function original = f; - f = ((AggregateFunction) f).clone(); + f = f.clone(); f.setArgs(argList); if (isUnion) { f.setName(new FunctionName(name + AGG_UNION_SUFFIX)); - f.setReturnType((ScalarType) argList.get(0)); + f.setReturnType(argList.get(0)); f.setNullableMode(NullableMode.ALWAYS_NOT_NULLABLE); } if (isMerge) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java index e7d8dbdfe3d..ffda27e7ba5 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java @@ -78,7 +78,7 @@ public class AggStateType extends DataType { @Override public Type toCatalogDataType() { - List<Type> types = subTypes.stream().map(t -> t.toCatalogDataType()).collect(Collectors.toList()); + List<Type> types = subTypes.stream().map(DataType::toCatalogDataType).collect(Collectors.toList()); return Expr.createAggStateType(functionName, types, subTypeNullables); } diff --git a/regression-test/suites/datatype_p0/agg_state/max/test_agg_state_max.groovy b/regression-test/suites/datatype_p0/agg_state/max/test_agg_state_max.groovy index 4bcc98a8b22..86618fd0bae 100644 --- a/regression-test/suites/datatype_p0/agg_state/max/test_agg_state_max.groovy +++ b/regression-test/suites/datatype_p0/agg_state/max/test_agg_state_max.groovy @@ -30,7 +30,7 @@ suite("test_agg_state_max") { test { sql "insert into a_table values(100,max_state(null));" - exception "which is illegal for non_nullable" + exception "can not cast from origin type AGG_STATE<max(NULL_TYPE NULL)> to target type=AGG_STATE<max(INT)>" } sql """insert into a_table --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
