github-actions[bot] commented on code in PR #67154:
URL: https://github.com/apache/doris/pull/67154#discussion_r3860558097
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -567,53 +566,121 @@ 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
whose argument
+ // type is unknown (NULL) and for the placeholder return type.
+ DecimalV3Type widerType = null;
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);
- }
+ DataType targetType = getSignatureArgumentType(signature, i);
List<DataType> argTypes =
extractArgumentTypeBySignature(DecimalV3Type.class, targetType,
arguments.get(i).getDataType());
- if (argTypes.isEmpty()) {
- continue;
- }
-
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;
+ DecimalV3Type decimalV3Type =
promotedDecimalV3Type(arguments.get(i), argType);
+ if (widerType == null) {
+ widerType = decimalV3Type;
} else {
- finalType = (DecimalV3Type)
DecimalV3Type.widerDecimalV3Type(finalType, decimalV3Type, false);
+ widerType = (DecimalV3Type)
DecimalV3Type.widerDecimalV3Type(widerType, decimalV3Type, false);
}
}
}
- DecimalV3Type argType = finalType;
- if (finalType == null) {
+ if (widerType == null) {
return signature;
}
- List<DataType> newArgTypes = signature.argumentsTypes.stream()
- .map(at -> TypeCoercionUtils.replaceDecimalV3WithTarget(at,
argType))
- .collect(Collectors.toList());
+
+ // Promote each decimal slot independently. Decimal slots inside a
complex type
+ // (e.g. the key and the value of a MAP) are independent type
variables and must
+ // keep their own precision/scale instead of being merged into one
wider type,
+ // otherwise widening one slot may overflow another slot (e.g. a big
integral key).
+ List<DataType> newArgTypes =
Lists.newArrayListWithCapacity(signature.argumentsTypes.size());
+ for (int i = 0; i < signature.argumentsTypes.size(); i++) {
+ DataType sigType = signature.argumentsTypes.get(i);
+ if (signature.hasVarArgs && i == signature.argumentsTypes.size() -
1) {
+ // keep the previous behavior for the vararg slot
+
newArgTypes.add(TypeCoercionUtils.replaceDecimalV3WithTarget(sigType,
widerType));
+ } else {
+ newArgTypes.add(promoteDecimalV3Slot(sigType,
arguments.get(i).getDataType(),
Review Comment:
[P1] Keep linked Decimal slots on one promoted type
This now promotes each declared argument separately after Any/Follow
resolution has already tied logical slots together. For `ElementAt`'s
`MAP<Any(0), Any(1)>, Follow(0)` signature, a nonliteral `MAP<DECIMAL(9,2),
...>` column and an equal `DECIMAL(10,3)` lookup are first resolved to the
map-key type, but this call leaves the key as Decimal32 while independently
widening the lookup to Decimal64. `expectedInputTypes()` preserves that split,
and BE's map lookup calls `ColumnDecimal32::compare_at` on the Decimal64 RHS
even though it requires the same concrete column class. The `NullType` fallback
has the same identity loss because it can replace a resolved Follow slot with
`widerType` from an unrelated MAP leaf. Please promote by logical Any/Follow
type-variable group, and cover storage-width boundaries plus NULL.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignatureHelper.java:
##########
@@ -567,53 +566,121 @@ 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
whose argument
+ // type is unknown (NULL) and for the placeholder return type.
+ DecimalV3Type widerType = null;
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);
- }
+ DataType targetType = getSignatureArgumentType(signature, i);
List<DataType> argTypes =
extractArgumentTypeBySignature(DecimalV3Type.class, targetType,
arguments.get(i).getDataType());
- if (argTypes.isEmpty()) {
- continue;
- }
-
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;
+ DecimalV3Type decimalV3Type =
promotedDecimalV3Type(arguments.get(i), argType);
+ if (widerType == null) {
+ widerType = decimalV3Type;
} else {
- finalType = (DecimalV3Type)
DecimalV3Type.widerDecimalV3Type(finalType, decimalV3Type, false);
+ widerType = (DecimalV3Type)
DecimalV3Type.widerDecimalV3Type(widerType, decimalV3Type, false);
}
}
}
- DecimalV3Type argType = finalType;
- if (finalType == null) {
+ if (widerType == null) {
return signature;
}
- List<DataType> newArgTypes = signature.argumentsTypes.stream()
- .map(at -> TypeCoercionUtils.replaceDecimalV3WithTarget(at,
argType))
- .collect(Collectors.toList());
+
+ // Promote each decimal slot independently. Decimal slots inside a
complex type
+ // (e.g. the key and the value of a MAP) are independent type
variables and must
+ // keep their own precision/scale instead of being merged into one
wider type,
+ // otherwise widening one slot may overflow another slot (e.g. a big
integral key).
+ List<DataType> newArgTypes =
Lists.newArrayListWithCapacity(signature.argumentsTypes.size());
+ for (int i = 0; i < signature.argumentsTypes.size(); i++) {
+ DataType sigType = signature.argumentsTypes.get(i);
+ if (signature.hasVarArgs && i == signature.argumentsTypes.size() -
1) {
Review Comment:
[P1] Model logical Decimal groups inside vararg signatures
This blanket last-slot replacement breaks both relationships that the new
promotion needs to preserve:
1. `Field` declares `.varArgs(DecimalV3Type.WILDCARD,
DecimalV3Type.WILDCARD)`, so its fixed first operand and repeated tail are one
comparison type. With a nonconstant `DECIMAL(3,2)` first operand equal to a
`DECIMAL(4,3)` search operand, they instead remain split; BE dispatches from
argument 0 and compares raw values 120/1200, returning 0 (and cross-width cases
hit an incompatible column cast).
2. Variadic user functions can declare a repeated MAP type and use this
default precision chain. For an exactly typed `MAP<DECIMAL(76,0),
DECIMAL(76,76)>` argument, the scan computes `DECIMAL(76,6)` globally and this
line rewrites both independent leaves to it before execution, overflowing the
key and truncating the value.
Please preserve common scalar families such as `field` while aggregating
each corresponding nested leaf independently across repeated complex arguments,
with regressions for both shapes.
--
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]
