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]

Reply via email to