github-actions[bot] commented on code in PR #62201:
URL: https://github.com/apache/doris/pull/62201#discussion_r3051247106
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyConditionalFunction.java:
##########
@@ -60,9 +62,7 @@ private static Expression
rewriteCoalesce(ExpressionMatchingContext<Coalesce> ct
if (1 == coalesce.arity()) {
Review Comment:
`NullIf.customSignature()` has already resolved the result type before this
rewrite runs, so returning `nullIf.child(0)` directly here loses that type
information for typed NULLs. A concrete case is `NULLIF(NULL as DATETIME(0),
NULL as DATETIME(6))`: this branch returns `NullLiteral(DATETIME(0))`, while
the function result type is `DATETIME(6)`. The previous path went through
`ensureSameResultType(...)`, which preserved the widened result type.
```suggestion
return TypeCoercionUtils.ensureSameResultType(nullIf,
nullIf.child(0), ctx.rewriteContext);
```
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/CreateMap.java:
##########
@@ -109,12 +115,100 @@ public List<FunctionSignature> getSignatures() {
if (arity() == 0) {
return SIGNATURES;
} else {
- return ImmutableList.of(FunctionSignature.of(
- getDataType(),
- children.stream()
- .map(ExpressionTrait::getDataType)
- .collect(ImmutableList.toImmutableList())
- ));
+ // split children into keys and values
+ List<Expression> keyExpressions = new ArrayList<>();
+ List<Expression> valueExpressions = new ArrayList<>();
+ for (int i = 0; i < children.size(); i++) {
+ if (i % 2 == 0) {
+ keyExpressions.add(children.get(i));
+ } else {
+ valueExpressions.add(children.get(i));
+ }
+ }
+
+ // find common key type
+ Map<Boolean, List<Expression>> keyPartitioned =
keyExpressions.stream()
+ .collect(Collectors.partitioningBy(
+ e -> (e instanceof Literal && ((Literal)
e).isStringLikeLiteral())));
+ List<DataType> keyForFindCommon =
keyPartitioned.get(false).stream()
+
.map(ExpressionTrait::getDataType).collect(Collectors.toList());
+ Optional<DataType> commonKeyType =
TypeCoercionUtils.findWiderCommonTypeByVariable(
+ keyForFindCommon, false, true);
+ if (!commonKeyType.isPresent()) {
+
SearchSignature.throwCanNotFoundFunctionException(this.getName(),
getArguments());
+ } else {
+ for (Expression stringLiteral : keyPartitioned.get(true)) {
+ Optional<Expression> optLiteral =
TypeCoercionUtils.characterLiteralTypeCoercion(
+ ((Literal) stringLiteral).getStringValue(),
commonKeyType.get());
+ if (optLiteral.isPresent()) {
+ DataType parsedType = optLiteral.get().getDataType();
+ Optional<DataType> widened =
TypeCoercionUtils.findWiderTypeForTwo(
+ commonKeyType.get(), parsedType, false, true);
+ if (!widened.isPresent()) {
+
SearchSignature.throwCanNotFoundFunctionException(this.getName(),
getArguments());
+ }
+ if (!widened.get().equals(commonKeyType.get())) {
+ commonKeyType = widened;
+ break;
+ }
+ } else {
+ Optional<DataType> widened =
TypeCoercionUtils.findWiderTypeForTwo(
+ commonKeyType.get(),
stringLiteral.getDataType(), false, true);
+ if (!widened.isPresent()) {
+
SearchSignature.throwCanNotFoundFunctionException(this.getName(),
getArguments());
+ }
+ commonKeyType = widened;
+ break;
+ }
+ }
+ }
+
+ // find common value type
+ Map<Boolean, List<Expression>> valuePartitioned =
valueExpressions.stream()
Review Comment:
Stopping at the first widening here makes `map()` inference depend on
literal order. For example, `map(1, 1, 2, '2147483648', 3, '1.5')` will pick
`BIGINT` after the second value and then never look at the third value, even
though that later string literal forces a different common type.
`implicitCastInputTypes()` will then analyze the remaining values against the
wrong inferred type. The removed `processCreateMap()` path examined the whole
value list via `Array`, so this is a functional 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]