github-actions[bot] commented on code in PR #67154:
URL: https://github.com/apache/doris/pull/67154#discussion_r3869326073
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -567,53 +570,228 @@ private static FunctionSignature
defaultTimePrecisionPromotion(FunctionSignature
private static FunctionSignature defaultDecimalV3PrecisionPromotion(
FunctionSignature signature, List<Expression> arguments) {
- DecimalV3Type finalType = null;
+ // 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 outermost MAP leaf group of each resolved type, used to link a
top-level
+ // scalar slot (e.g. element_at's lookup) with the MAP leaf it was
resolved from:
+ // after Any/Follow resolution both carry the same concrete 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();
+
+ 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();
- } else {
- targetType = signature.getArgType(i);
- }
- List<DataType> argTypes =
extractArgumentTypeBySignature(DecimalV3Type.class, targetType,
- arguments.get(i).getDataType());
- if (argTypes.isEmpty()) {
- continue;
- }
+ DataType targetType = getSignatureArgumentType(signature, i);
+ collectDecimalLeaf(targetType, arguments.get(i).getDataType(),
arguments.get(i),
+ "", mapLeafGroupByType, groupWider, scalarLeaves,
widerHolder);
+ }
+ widerType = widerHolder[0];
+ if (widerType == null) {
+ return signature;
+ }
- 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);
- }
- if (finalType == null) {
- finalType = decimalV3Type;
- } else {
- finalType = (DecimalV3Type)
DecimalV3Type.widerDecimalV3Type(finalType, decimalV3Type, false);
- }
+ // Fold the promoted type of every top-level scalar slot into the MAP
leaf group
+ // of the same resolved type (if any), so the MAP leaf and the scalar
slot linked
+ // with it are promoted to one type.
+ for (DecimalLeaf scalarLeaf : scalarLeaves) {
+ String linkedGroup =
mapLeafGroupByType.get(scalarLeaf.resolvedType);
+ if (linkedGroup != null) {
+ groupWider.merge(linkedGroup, scalarLeaf.promotedType,
+ ComputeSignatureHelper::mergeDecimalV3Type);
}
}
- DecimalV3Type argType = finalType;
- if (finalType == null) {
- return signature;
+
+ List<DataType> newArgTypes =
Lists.newArrayListWithCapacity(signature.argumentsTypes.size());
+ for (int i = 0; i < signature.argumentsTypes.size(); i++) {
+
newArgTypes.add(replaceDecimalV3Leaf(signature.argumentsTypes.get(i), "",
+ mapLeafGroupByType, groupWider, widerType));
}
- List<DataType> newArgTypes = signature.argumentsTypes.stream()
- .map(at -> TypeCoercionUtils.replaceDecimalV3WithTarget(at,
argType))
- .collect(Collectors.toList());
signature = signature.withArgumentTypes(signature.hasVarArgs,
newArgTypes);
if (signature.returnType instanceof DecimalV3Type
&& ((DecimalV3Type) signature.returnType).getPrecision() <= 0)
{
- signature = signature.withReturnType(argType);
+ 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 widerHolder} accumulates the wider type across all
decimal leaves.
+ */
+ private static void collectDecimalLeaf(DataType sigType, DataType argType,
Expression arg,
+ String path, Map<DecimalV3Type, String> mapLeafGroupByType,
+ Map<String, DecimalV3Type> groupWider, 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: keep the original behavior of the
single wider
+ // type, but a concrete resolved type may be linked with a MAP
leaf below
+ if (promoted != null && sigDecimal.getPrecision() > 0) {
+ scalarLeaves.add(new DecimalLeaf(sigDecimal, promoted));
+ }
+ } else if (isMapNested(path) && promoted != null) {
+ String groupKey = path + ":" + sigDecimal;
+ groupWider.merge(groupKey, promoted,
ComputeSignatureHelper::mergeDecimalV3Type);
+ // keep the outermost group (shortest path, key before value)
for linking
+ mapLeafGroupByType.putIfAbsent(sigDecimal, groupKey);
+ }
+ // other leaves (e.g. ARRAY items not nested in a MAP) keep the
original
+ // behavior of the single wider type
+ return;
+ } else if (sigType instanceof MapType) {
+ MapType mapType = (MapType) sigType;
+ if (argType instanceof MapType) {
+ MapType argMapType = (MapType) argType;
+ collectDecimalLeaf(mapType.getKeyType(),
argMapType.getKeyType(), arg,
+ appendPath(path, MAP_KEY), mapLeafGroupByType,
groupWider,
+ scalarLeaves, widerHolder);
+ collectDecimalLeaf(mapType.getValueType(),
argMapType.getValueType(), arg,
+ appendPath(path, MAP_VALUE), mapLeafGroupByType,
groupWider,
+ scalarLeaves, widerHolder);
+ } else if (argType instanceof NullType) {
+ collectDecimalLeaf(mapType.getKeyType(), argType, arg,
+ appendPath(path, MAP_KEY), mapLeafGroupByType,
groupWider,
+ scalarLeaves, widerHolder);
+ collectDecimalLeaf(mapType.getValueType(), argType, arg,
+ appendPath(path, MAP_VALUE), mapLeafGroupByType,
groupWider,
+ 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
+ collectDecimalLeaf(((ArrayType) sigType).getItemType(),
itemArgType, arg,
+ appendPath(path, ARRAY_ITEM), mapLeafGroupByType,
groupWider,
+ scalarLeaves, widerHolder);
+ }
+ // 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, all other leaves (scalar, ARRAY
item, etc.)
+ * keep the original behavior of using the single wider type across all
decimal
+ * slots.
+ */
+ private static DataType replaceDecimalV3Leaf(DataType sigType, String path,
+ Map<DecimalV3Type, String> mapLeafGroupByType, Map<String,
DecimalV3Type> groupWider,
+ 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. element_at's lookup must match the MAP key type)
+ if (sigDecimal.getPrecision() > 0) {
+ String linkedGroup = mapLeafGroupByType.get(sigDecimal);
+ if (linkedGroup != null) {
+ DecimalV3Type linkedWider =
groupWider.get(linkedGroup);
+ if (linkedWider != null) {
+ return linkedWider;
+ }
+ }
+ }
+ return widerType;
Review Comment:
[P1] Preserve independent scalar groups used to build MAPs
`map_agg` and `map_agg_v2` expose their MAP key/value as independent
top-level `Any(0)` and `Any(1)` arguments, but this fallback still replaces
every top-level Decimal slot with one global type. For example:
```text
Aggregate[map_agg(k, v)]
Scan(k DECIMAL(38,0), v DECIMAL(38,18))
```
After Any resolution the two inputs are correctly independent, but this pass
merges them to `DECIMAL(38,6)`. `expectedInputTypes()` then casts the 38-digit
key to a type with only 32 integral digits (so the entry can disappear as a
NULL key) and truncates twelve fractional digits from the value before
aggregation. `MapFromArrays` and `MapFromEntries` already opt out of default
precision for this exact key/value independence, whereas both MAP aggregates
still use it. Please preserve logical Any groups for top-level scalars (or give
these aggregates an equivalent precision override) and add a nonconstant
aggregate regression.
--
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]
