github-actions[bot] commented on code in PR #67154:
URL: https://github.com/apache/doris/pull/67154#discussion_r3870593902
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -567,53 +570,247 @@ 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();
+
+ // 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();
- } 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, scalarGroupWider,
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, scalarGroupWider,
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, 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, 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));
+ scalarGroupWider.merge(sigDecimal, promoted,
+ ComputeSignatureHelper::mergeDecimalV3Type);
+ }
+ } 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);
Review Comment:
[P1] Preserve the logical Any group when resolved types collide
A reduced failing tree is:
```text
Project[map_contains_value(m, x)]
Scan[m MAP<DECIMAL(10,3), DECIMAL(9,2)>, x DECIMAL(10,3)]
```
Any resolution correctly widens the value/`x` group to `DECIMAL(10,3)`,
which now happens to equal the independent key type. Because collection visits
the key first, this `putIfAbsent` records only `DECIMAL(10,3) -> key`; `x` is
folded into that key group and replacement regresses the value to
`DECIMAL(9,2)`. `expectedInputTypes()` therefore sends a Decimal32 value array
and Decimal64 probe to BE's array-contains dispatch, which returns a runtime
type error; same-width cases compare different raw scales. `map_contains_entry`
has the same collision when its key/value Any groups resolve equally. Please
carry the original Any/Follow group identity instead of reconstructing it from
the resolved type, and add nonconstant value/entry regressions.
--
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]
