github-actions[bot] commented on code in PR #67154:
URL: https://github.com/apache/doris/pull/67154#discussion_r3863354589
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -567,53 +569,211 @@ 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
+ // the key/value of a MAP (keeping the original behavior), for the
placeholder
+ // return type, and for MAP leaves whose group has no concrete type
information.
+ DecimalV3Type widerType = null;
+
+ // The KEY leaves and the VALUE leaves of 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 resolved leaf
type, so the
+ // corresponding leaves of different (or repeated) MAP arguments
aggregate while
+ // the key leaves and the value leaves stay independent.
+ Map<DecimalV3Type, DecimalV3Type> mapKeyWider = Maps.newHashMap();
+ Map<DecimalV3Type, DecimalV3Type> mapValueWider = Maps.newHashMap();
+
+ // Top-level scalar decimal leaves with a concrete resolved type. After
+ // Any/Follow resolution such a slot carries the same concrete type as
the MAP
+ // key/value leaf it was resolved from (e.g. the lookup argument of
element_at
+ // follows the MAP key type). It must be promoted together with that
MAP leaf,
+ // otherwise the BE compares columns of different concrete decimal
classes.
+ 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),
+ null, true, mapKeyWider, mapValueWider, 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 key/value and the
scalar slot
+ // linked with it are promoted to one type.
+ for (DecimalLeaf scalarLeaf : scalarLeaves) {
+ DecimalV3Type linkedWider =
mapKeyWider.get(scalarLeaf.resolvedType);
+ if (linkedWider == null) {
+ linkedWider = mapValueWider.get(scalarLeaf.resolvedType);
+ if (linkedWider != null) {
+ mapValueWider.put(scalarLeaf.resolvedType,
+ mergeDecimalV3Type(linkedWider,
scalarLeaf.promotedType));
}
+ } else {
+ mapKeyWider.put(scalarLeaf.resolvedType,
+ mergeDecimalV3Type(linkedWider,
scalarLeaf.promotedType));
}
}
- 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), null,
true,
+ mapKeyWider, mapValueWider, 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 mapSide} is {@link #MAP_KEY} or {@link
#MAP_VALUE}
+ * when the leaf is directly the key/value of a MAP (possibly nested in a
MAP),
+ * otherwise {@code null}. {@code linkable} indicates the leaf is a
top-level scalar
+ * slot (not nested in any MAP or ARRAY), which may be linked to a MAP
key/value leaf
+ * of the same resolved type. {@code widerHolder} accumulates the wider
type across
+ * all decimal leaves.
+ */
+ private static void collectDecimalLeaf(DataType sigType, DataType argType,
Expression arg,
+ String mapSide, boolean linkable, Map<DecimalV3Type,
DecimalV3Type> mapKeyWider,
+ Map<DecimalV3Type, DecimalV3Type> mapValueWider, 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 (mapSide == null) {
+ if (linkable && promoted != null && sigDecimal.getPrecision()
> 0) {
+ // top-level scalar slot with a concrete resolved type may
be linked
+ // with a MAP key/value leaf of the same type below
+ scalarLeaves.add(new DecimalLeaf(sigDecimal, promoted));
+ }
+ } else if (promoted != null) {
+ if (mapSide == MAP_KEY) {
+ mapKeyWider.merge(sigDecimal, promoted,
ComputeSignatureHelper::mergeDecimalV3Type);
+ } else {
+ mapValueWider.merge(sigDecimal, promoted,
ComputeSignatureHelper::mergeDecimalV3Type);
+ }
+ }
+ return;
+ } else if (sigType instanceof MapType) {
+ MapType mapType = (MapType) sigType;
+ if (argType instanceof MapType) {
+ MapType argMapType = (MapType) argType;
+ collectDecimalLeaf(mapType.getKeyType(),
argMapType.getKeyType(), arg,
+ MAP_KEY, false, mapKeyWider, mapValueWider,
scalarLeaves, widerHolder);
+ collectDecimalLeaf(mapType.getValueType(),
argMapType.getValueType(), arg,
+ MAP_VALUE, false, mapKeyWider, mapValueWider,
scalarLeaves, widerHolder);
+ } else if (argType instanceof NullType) {
+ collectDecimalLeaf(mapType.getKeyType(), argType, arg,
+ MAP_KEY, false, mapKeyWider, mapValueWider,
scalarLeaves, widerHolder);
+ collectDecimalLeaf(mapType.getValueType(), argType, arg,
+ MAP_VALUE, false, mapKeyWider, mapValueWider,
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;
+ }
+ // ARRAY items are not MAP key/value leaves, keep the original
behavior
Review Comment:
[P1] Preserve the enclosing MAP path through nested containers
For a nonconstant `m MAP<DECIMAL(38,0), ARRAY<DECIMAL(38,18)>>`, this
recursion drops the incoming `MAP_VALUE`. The array leaf is therefore merged
globally with the key and becomes `DECIMAL(38,6)` under the default overflow
scale, so `expectedInputTypes()` inserts a lossy cast and discards low-order
fractional digits before `map_values(m)` executes. Nested MAP recursion
similarly overwrites the outer branch and can apply an outer-key lookup
promotion to an unrelated inner key. Please carry the full structural/logical
group path through ARRAY/MAP recursion and cover a nonconstant nested-container
case. This is distinct from the existing direct key/value and scalar-link
threads.
--
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]
