github-actions[bot] commented on code in PR #67154:
URL: https://github.com/apache/doris/pull/67154#discussion_r4025552712
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -566,52 +577,463 @@ private static FunctionSignature
defaultTimePrecisionPromotion(FunctionSignature
}
private static FunctionSignature defaultDecimalV3PrecisionPromotion(
- FunctionSignature signature, List<Expression> arguments) {
- DecimalV3Type finalType = null;
+ FunctionSignature signature, List<Expression> arguments,
ComputeSignature computeSignature) {
+ // The wider type across all decimal slots, used for decimal slots
that are not
+ // inside a MAP (keeping the original behavior), for the placeholder
return type,
+ // and for MAP-nested leaves whose group has no concrete type
information.
+ DecimalV3Type widerType = null;
+
+ // Decimal leaves inside a MAP are independent type variables: they
must keep
+ // their own precision/scale instead of being merged into one wider
type,
+ // otherwise widening one leaf (e.g. the scale of a big integral key)
may overflow
+ // the other leaf. They are grouped by the full structural path
through nested
+ // containers (e.g. "key", "value", "value/array", "value/key") and
the resolved
+ // leaf type, so the leaves of different (or repeated) MAP arguments
on the same
+ // path aggregate while leaves on different paths stay independent.
+ Map<String, DecimalV3Type> groupWider = Maps.newHashMap();
+
+ // The template signature carrying the original Any/Follow slots that
the resolved
+ // signature was derived from. It lets us link a top-level scalar slot
with the MAP
+ // leaf it belongs to by the original Any/Follow group identity (the
index) instead
+ // of the resolved concrete type, which can collide when independent
slots resolve
+ // to the same type (e.g. the key and the value of a MAP both becoming
DECIMAL(10,3)).
+ FunctionSignature template = findDecimalV3Template(computeSignature,
signature);
+
+ // The outermost MAP leaf group of each Any/Follow index (from the
template), used
+ // to link a top-level scalar slot (e.g. map_contains_value's probe,
element_at's
+ // lookup) with the MAP leaf that carries the same index.
+ Map<Integer, String> indexToMapLeafGroup = Maps.newHashMap();
+
+ // Fallback used when the template can not be recovered: the outermost
MAP leaf
+ // group of each resolved type, used to link a top-level scalar slot
with the MAP
+ // leaf it was resolved from (after Any/Follow resolution both carry
the same type).
+ Map<DecimalV3Type, String> mapLeafGroupByType = Maps.newHashMap();
+
+ // Top-level scalar decimal leaves with a concrete resolved type,
whose promoted
+ // type must also be folded into the linked MAP leaf group.
+ List<DecimalLeaf> scalarLeaves = Lists.newArrayList();
+
+ // Top-level scalar decimal slots are independent logical type
variables
+ // (e.g. the key/value of map_agg(k, v) are Any(0) and Any(1)); group
them by
+ // the resolved type so the slots of one logical group aggregate while
the slots
+ // of different groups keep their own precision/scale.
+ Map<DecimalV3Type, DecimalV3Type> scalarGroupWider = Maps.newHashMap();
+
+ DecimalV3Type[] widerHolder = new DecimalV3Type[1];
for (int i = 0; i < arguments.size(); i++) {
- DataType targetType;
- if (i >= signature.argumentsTypes.size()) {
- Preconditions.checkState(signature.getVarArgType().isPresent(),
- "argument size larger than signature");
- targetType = signature.getVarArgType().get();
+ DataType targetType = getSignatureArgumentType(signature, i);
+ DataType templateType = template == null ? null :
getSignatureArgumentType(template, i);
+ collectDecimalLeaf(targetType, arguments.get(i).getDataType(),
arguments.get(i),
+ "", templateType, -1, indexToMapLeafGroup,
mapLeafGroupByType, groupWider,
+ scalarGroupWider, scalarLeaves, widerHolder);
+ }
+ widerType = widerHolder[0];
+ if (widerType == null) {
+ return signature;
+ }
+
+ // Fold the promoted type of every top-level scalar slot into the MAP
leaf group it
+ // is linked with (by the original Any/Follow identity when available,
otherwise by
+ // the resolved type), so the MAP leaf and the scalar slot linked with
it are
+ // promoted to one type.
+ for (DecimalLeaf scalarLeaf : scalarLeaves) {
+ String linkedGroup;
+ if (scalarLeaf.index >= 0) {
+ linkedGroup = indexToMapLeafGroup.get(scalarLeaf.index);
} else {
- targetType = signature.getArgType(i);
+ linkedGroup = mapLeafGroupByType.get(scalarLeaf.resolvedType);
}
- List<DataType> argTypes =
extractArgumentTypeBySignature(DecimalV3Type.class, targetType,
- arguments.get(i).getDataType());
- if (argTypes.isEmpty()) {
- continue;
+ if (linkedGroup != null) {
+ groupWider.merge(linkedGroup, scalarLeaf.promotedType,
+ ComputeSignatureHelper::mergeDecimalV3Type);
}
+ }
- for (DataType argType : argTypes) {
- Expression arg = arguments.get(i);
- DecimalV3Type decimalV3Type;
- if (arg.isLiteral() && arg.getDataType().isIntegralType()) {
- // create decimalV3 with minimum scale enough to hold the
integral literal
- decimalV3Type = DecimalV3Type.createDecimalV3Type(new
BigDecimal(((Literal) arg).getStringValue()));
- } else {
- decimalV3Type = DecimalV3Type.forType(argType);
+ List<DataType> newArgTypes =
Lists.newArrayListWithCapacity(signature.argumentsTypes.size());
+ for (int i = 0; i < signature.argumentsTypes.size(); i++) {
+ DataType templateType = template == null ? null :
getSignatureArgumentType(template, i);
+
newArgTypes.add(replaceDecimalV3Leaf(signature.argumentsTypes.get(i), "",
templateType, -1,
+ indexToMapLeafGroup, mapLeafGroupByType, groupWider,
scalarGroupWider, widerType));
+ }
+ signature = signature.withArgumentTypes(signature.hasVarArgs,
newArgTypes);
+ if (signature.returnType instanceof DecimalV3Type
+ && ((DecimalV3Type) signature.returnType).getPrecision() <= 0)
{
+ signature = signature.withReturnType(widerType);
+ }
+ return signature;
+ }
+
+ private static DataType getSignatureArgumentType(FunctionSignature
signature, int index) {
+ if (index >= signature.argumentsTypes.size()) {
+ Preconditions.checkState(signature.getVarArgType().isPresent(),
+ "argument size larger than signature");
+ return signature.getVarArgType().get();
+ }
+ return signature.getArgType(index);
+ }
+
+ /**
+ * Compute the promoted DecimalV3Type for one decimal slot from its
argument type.
+ */
+ private static DecimalV3Type promotedDecimalV3Type(Expression arg,
DataType argType) {
+ if (arg.isLiteral() && arg.getDataType().isIntegralType()) {
+ // create decimalV3 with minimum scale enough to hold the integral
literal
+ return DecimalV3Type.createDecimalV3Type(new BigDecimal(((Literal)
arg).getStringValue()));
+ }
+ return DecimalV3Type.forType(argType);
+ }
+
+ /**
+ * Collect every decimal leaf of one argument and fold its promoted type
into the
+ * corresponding group. {@code path} is the full structural path through
nested
+ * containers (empty for a top-level slot, {@link #MAP_KEY}/{@link
#MAP_VALUE} for
+ * the key/value of a MAP, {@link #ARRAY_ITEM} for an ARRAY item), so an
ARRAY nested
+ * in a MAP value (e.g. "value/array") or the key/value of a nested MAP
(e.g.
+ * "value/key") keep the enclosing group instead of being merged with the
outer
+ * leaves. {@code templateType} is the corresponding slot of the template
signature
+ * that still carries the original Any/Follow identity of this leaf, and
+ * {@code containerIndex} is the Any/Follow index of an enclosing MAP
container that
+ * owns this leaf as a whole (i.e. the container itself is an Any/Follow
slot), or -1
+ * when there is none. {@code widerHolder} accumulates the wider type
across all
+ * decimal leaves.
+ */
+ private static void collectDecimalLeaf(DataType sigType, DataType argType,
Expression arg,
+ String path, DataType templateType, int containerIndex,
+ Map<Integer, String> indexToMapLeafGroup, Map<DecimalV3Type,
String> mapLeafGroupByType,
+ Map<String, DecimalV3Type> groupWider, Map<DecimalV3Type,
DecimalV3Type> scalarGroupWider,
+ List<DecimalLeaf> scalarLeaves, DecimalV3Type[] widerHolder) {
+ if (sigType instanceof DecimalV3Type) {
+ DecimalV3Type sigDecimal = (DecimalV3Type) sigType;
+ DecimalV3Type promoted = null;
+ if (!(argType instanceof NullType)) {
+ promoted = promotedDecimalV3Type(arg, argType);
+ widerHolder[0] = mergeDecimalV3Type(widerHolder[0], promoted);
+ }
+ if (path.isEmpty()) {
+ // top-level scalar slot: a concrete resolved type may be
linked with a
+ // MAP leaf below by the original Any/Follow identity, and
otherwise the
+ // slots of the same resolved type form one logical group
(e.g. the two
+ // arguments of map_agg) and stay independent from the slots
of other groups
+ if (promoted != null && sigDecimal.getPrecision() > 0) {
+ scalarLeaves.add(new DecimalLeaf(sigDecimal, promoted,
anyFollowIndex(templateType)));
+ scalarGroupWider.merge(sigDecimal, promoted,
+ ComputeSignatureHelper::mergeDecimalV3Type);
+ }
+ } else if (containerIndex >= 0) {
+ // the leaf is inside a MAP container that is itself an
Any/Follow slot: the
+ // container absorbs the outer structural path, so the
descendants of all the
+ // containers that share this identity aggregate (e.g. the key
of ARRAY<MAP>
+ // and the key of a plain MAP argument of the same Any(0) slot)
+ if (promoted != null) {
+ String groupKey = MAP_CONTAINER_GROUP + containerIndex +
"/" + path + ":" + sigDecimal;
+ groupWider.merge(groupKey, promoted,
ComputeSignatureHelper::mergeDecimalV3Type);
}
- if (finalType == null) {
- finalType = decimalV3Type;
+ } else if (isMapNested(path)) {
+ String groupKey;
+ int index = anyFollowIndex(templateType);
+ if (index >= 0) {
+ // leaves that share the original Any/Follow identity
aggregate into one
+ // group (e.g. the value of map_contains_value and its
probe). The group
+ // is registered even when this leaf is NULL so a concrete
linked scalar
+ // can still join it; only non-NULL evidence is merged
below.
+ groupKey = ANY_INDEX_GROUP + index;
+ indexToMapLeafGroup.putIfAbsent(index, groupKey);
} else {
- finalType = (DecimalV3Type)
DecimalV3Type.widerDecimalV3Type(finalType, decimalV3Type, false);
+ // no identity: keep the outermost group (shortest path,
key before value)
+ // for linking by the resolved type
+ groupKey = path + ":" + sigDecimal;
+ mapLeafGroupByType.putIfAbsent(sigDecimal, groupKey);
+ }
+ if (promoted != null) {
+ groupWider.merge(groupKey, promoted,
ComputeSignatureHelper::mergeDecimalV3Type);
+ }
+ } else if (promoted != null) {
+ // non-MAP ARRAY leaf (e.g. the item of ARRAY<Any(index)>):
keep the original
+ // Any/Follow identity so it is promoted together with the
linked scalar slot
+ // of the same group (e.g. array_contains(ARRAY<Any(0)>,
Any(0)))
+ int index = anyFollowIndex(templateType);
+ if (index >= 0) {
+ String groupKey = ANY_INDEX_GROUP + index;
+ indexToMapLeafGroup.putIfAbsent(index, groupKey);
+ groupWider.merge(groupKey, promoted,
ComputeSignatureHelper::mergeDecimalV3Type);
+ }
+ // without an identity the leaf keeps the original single
wider-type behavior
+ }
+ return;
+ } else if (sigType instanceof MapType) {
+ MapType mapType = (MapType) sigType;
+ DataType templateKey = null;
+ DataType templateValue = null;
+ int childContainerIndex = containerIndex;
+ String childPath = path;
+ if (templateType instanceof MapType) {
+ templateKey = ((MapType) templateType).getKeyType();
+ templateValue = ((MapType) templateType).getValueType();
+ } else {
+ // the whole MAP container is an Any/Follow slot: propagate
the group
+ // identity into the descendant-relative keys and absorb the
outer path
+ int index = anyFollowIndex(templateType);
+ if (index >= 0) {
+ childContainerIndex = index;
+ childPath = "";
+ }
+ }
+ if (argType instanceof MapType) {
+ MapType argMapType = (MapType) argType;
+ collectDecimalLeaf(mapType.getKeyType(),
argMapType.getKeyType(), arg,
+ appendPath(childPath, MAP_KEY), templateKey,
childContainerIndex,
+ indexToMapLeafGroup, mapLeafGroupByType, groupWider,
+ scalarGroupWider, scalarLeaves, widerHolder);
+ collectDecimalLeaf(mapType.getValueType(),
argMapType.getValueType(), arg,
+ appendPath(childPath, MAP_VALUE), templateValue,
childContainerIndex,
+ indexToMapLeafGroup, mapLeafGroupByType, groupWider,
+ scalarGroupWider, scalarLeaves, widerHolder);
+ } else if (argType instanceof NullType) {
+ collectDecimalLeaf(mapType.getKeyType(), argType, arg,
+ appendPath(childPath, MAP_KEY), templateKey,
childContainerIndex,
+ indexToMapLeafGroup, mapLeafGroupByType, groupWider,
+ scalarGroupWider, scalarLeaves, widerHolder);
+ collectDecimalLeaf(mapType.getValueType(), argType, arg,
+ appendPath(childPath, MAP_VALUE), templateValue,
childContainerIndex,
+ indexToMapLeafGroup, mapLeafGroupByType, groupWider,
+ scalarGroupWider, scalarLeaves, widerHolder);
+ }
+ return;
+ } else if (sigType instanceof ArrayType) {
+ DataType itemArgType;
+ if (argType instanceof ArrayType) {
+ itemArgType = ((ArrayType) argType).getItemType();
+ } else if (argType instanceof NullType) {
+ itemArgType = argType;
+ } else {
+ return;
+ }
+ // carry the enclosing MAP path through the ARRAY so items nested
in a MAP
+ // value stay in the value group; when the ARRAY itself is an
Any/Follow slot
+ // (e.g. the item of ARRAY<Any(0)>), propagate the container
identity into the
+ // item and absorb the outer structural path so all occurrences of
the slot
+ // share descendant-relative keys
+ DataType templateItem = null;
+ int childContainerIndex = containerIndex;
+ String childPath = path;
+ if (templateType instanceof ArrayType) {
+ templateItem = ((ArrayType) templateType).getItemType();
+ } else {
+ int index = anyFollowIndex(templateType);
+ if (index >= 0) {
+ childContainerIndex = index;
+ childPath = "";
}
}
+ collectDecimalLeaf(((ArrayType) sigType).getItemType(),
itemArgType, arg,
+ appendPath(childPath, ARRAY_ITEM), templateItem,
childContainerIndex,
+ indexToMapLeafGroup, mapLeafGroupByType, groupWider,
+ scalarGroupWider, scalarLeaves, widerHolder);
}
- DecimalV3Type argType = finalType;
- if (finalType == null) {
- return signature;
+ // StructType and other types are not supported
+ }
+
+ /**
+ * Replace every decimal leaf in {@code sigType}: leaves inside a MAP use
the wider
+ * type of their own structural group, top-level scalar slots use the
wider type of
+ * their own logical group (slots of the same resolved type), and all
other leaves
+ * (e.g. ARRAY items not nested in a MAP) keep the original behavior of
using the
+ * single wider type across all decimal slots.
+ */
+ private static DataType replaceDecimalV3Leaf(DataType sigType, String
path, DataType templateType,
+ int containerIndex, Map<Integer, String> indexToMapLeafGroup,
+ Map<DecimalV3Type, String> mapLeafGroupByType,
+ Map<String, DecimalV3Type> groupWider, Map<DecimalV3Type,
DecimalV3Type> scalarGroupWider,
+ DecimalV3Type widerType) {
+ if (sigType instanceof DecimalV3Type) {
+ DecimalV3Type sigDecimal = (DecimalV3Type) sigType;
+ if (path.isEmpty()) {
+ // a top-level scalar slot linked with a MAP leaf keeps the
type of that
+ // leaf (e.g. map_contains_value's probe / element_at's lookup
must match
+ // the MAP value/key type). The link is resolved by the
original Any/Follow
+ // identity, falling back to the resolved type when the
template can not be
+ // recovered.
+ if (sigDecimal.getPrecision() > 0) {
+ String linkedGroup = null;
+ int index = anyFollowIndex(templateType);
+ if (index >= 0) {
+ linkedGroup = indexToMapLeafGroup.get(index);
+ } else {
+ linkedGroup = mapLeafGroupByType.get(sigDecimal);
+ }
+ if (linkedGroup != null) {
+ DecimalV3Type linkedWider =
groupWider.get(linkedGroup);
+ if (linkedWider != null) {
+ return linkedWider;
+ }
+ }
+ // independent logical Any groups (e.g. the key/value
arguments of
+ // map_agg) keep their own precision/scale instead of
being merged
+ // into one wider type
+ DecimalV3Type scalarWider =
scalarGroupWider.get(sigDecimal);
+ if (scalarWider != null) {
+ return scalarWider;
+ }
+ }
+ return widerType;
+ }
+ if (containerIndex >= 0) {
+ DecimalV3Type groupType = groupWider.get(
+ MAP_CONTAINER_GROUP + containerIndex + "/" + path +
":" + sigDecimal);
+ return groupType != null ? groupType : widerType;
+ }
+ if (isMapNested(path)) {
+ int index = anyFollowIndex(templateType);
+ String groupKey = index >= 0 ? ANY_INDEX_GROUP + index : path
+ ":" + sigDecimal;
+ DecimalV3Type groupType = groupWider.get(groupKey);
+ return groupType != null ? groupType : widerType;
+ }
+ // non-MAP ARRAY leaf (e.g. the item of ARRAY<Any(index)>): keep
the original
+ // Any/Follow identity so it stays promoted together with the
linked scalar
+ // slot of the same group, otherwise the array and the probe
diverge
+ int index = anyFollowIndex(templateType);
+ if (index >= 0) {
+ DecimalV3Type groupType = groupWider.get(ANY_INDEX_GROUP +
index);
+ if (groupType != null) {
+ return groupType;
+ }
+ }
+ // other leaves (e.g. ARRAY items not nested in a MAP) keep the
original
+ // behavior of the single wider type
+ return widerType;
Review Comment:
[P1] Preserve independent non-MAP ARRAY argument types
This fallback still merges ARRAY slots that the function signatures
deliberately keep independent. For example:
```text
Project[array_sortby(src, keys)]
Values[src ARRAY<DECIMAL(76,76)>, keys ARRAY<DECIMAL(76,70)>]
```
Both items contribute only to `widerType`, so this rewrites both expected
inputs to `DECIMAL(76,70)` and truncates six digits from the source values that
BE returns; the sort keys never needed the source type. The same root corrupts
independent `array_zip` fields (and leaves its Struct return type inconsistent)
and expanded `array_enumerate_uniq` varargs, where truncation can collapse
distinct composite keys. Please preserve a separate Decimal group for each
`INSTANCE_WITHOUT_INDEX` occurrence (including expanded varargs) and
`array_zip`'s independent fixed slots, while retaining common exact-vararg and
indexed Any/Follow groups, with cross-range/cross-scale regressions for these
paths.
--
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]
