github-actions[bot] commented on code in PR #67887:
URL: https://github.com/apache/doris/pull/67887#discussion_r4003463873
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputeSignature.java:
##########
@@ -117,6 +118,19 @@ default FunctionSignature
computeSignature(FunctionSignature signature) {
.get();
}
+ /**
+ * Refresh argument and return metadata that is derived directly from the
current children after reusing a
+ * previously resolved signature. The immediate origin arguments are
supplied with that signature so an override
+ * can distinguish an unchanged raw child from an unrelated replacement
without eagerly recomputing either type.
+ * The default is intentionally identity: overload selection, coercion,
and value-dependent precision decisions
+ * remain frozen across equivalent rewrites. An override must only rebuild
metadata that the function definition
+ * derives from its children; it must not search overloads or rerun
generic signature computation.
+ */
+ default FunctionSignature refreshDerivedSignature(
+ FunctionSignature signature, List<Expression>
immediateOriginArguments) {
+ return signature;
Review Comment:
[P1] Refresh unchanged complex passthrough signatures too
This identity default leaves functions such as ArrayPopBack, ArrayPopFront,
ArraySlice, ArrayConcat, and DeduplicateMap on their old reused return
metadata. A reduced production path is:
```text
LeftOuterJoin(other = length(to_json(array_popback(
array(struct(l.number, r.number), struct(l.number, r.number)))))) > 0)
```
The final AdjustNullable pass rebuilds the edited struct/array nodes with
the right-side field nullable, but ArrayPopBack.withChildren still reaches this
default and keeps its pre-join retArgType(0) result. ExpressionTranslator then
emits the current argument type with that stale return, while BE derives and
produces array_popback's result from argument 0, so return/column type
validation can fail with the same nested-nullability mismatch this PR is
fixing. Please audit all complex retArgType/Follow passthroughs and add an
outer-join regression with one of them between array(struct(...)) and its
consumer.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Array.java:
##########
@@ -131,4 +142,79 @@ public List<FunctionSignature> getSignatures() {
.map(dataType -> FunctionSignature.ret(ArrayType.of(new
FollowToArgumentType(0))).varArgs(dataType))
.collect(ImmutableList.toImmutableList());
}
+
+ @Override
+ public FunctionSignature computeSignature(FunctionSignature signature) {
+ if (isFullyResolved(signature.returnType)
+ &&
signature.argumentsTypes.stream().allMatch(Array::isFullyResolved)) {
+ // findWiderCommonTypeByVariable already produced the exact common
item type. Running generic precision
+ // promotion again could use one visible nested leaf to overwrite
independent fields inside that type.
+ return signature;
+ }
+ return ExplicitlyCastableSignature.super.computeSignature(signature);
+ }
+
+ @Override
+ public FunctionSignature deriveSignatureFromChildren(
+ FunctionSignature resolvedSignature, List<Expression>
immediateOriginArguments) {
+ if (children.isEmpty()) {
+ if (!resolvedSignature.hasVarArgs &&
resolvedSignature.argumentsTypes.isEmpty()) {
+ return resolvedSignature;
+ }
+ throw new AnalysisException(
+ "Cannot safely reuse a non-empty ARRAY signature for an
empty ARRAY");
+ }
+ if (!resolvedSignature.hasVarArgs ||
resolvedSignature.argumentsTypes.isEmpty()
+ || !(resolvedSignature.returnType instanceof ArrayType)) {
+ throw new AnalysisException(
+ "Cannot safely reuse an empty or fixed-arity ARRAY
signature for a non-empty ARRAY");
+ }
+ List<DataType> currentTypes = children.stream()
+ .map(ExpressionTrait::getDataType)
+ .collect(Collectors.toList());
+ List<DataType> originTypes = new ArrayList<>(currentTypes.size());
+ for (int i = 0; i < currentTypes.size(); i++) {
+ DataType resolvedType = resolvedSignature.getArgType(i);
+ DataType originType = i < immediateOriginArguments.size()
+ ? immediateOriginArguments.get(i).getDataType() :
resolvedType;
+ ChildDerivedSignature.refreshNestedTypeMetadata(
+ resolvedType, currentTypes.get(i), originType);
+ originTypes.add(originType);
+ }
+ DataType itemType = ChildDerivedSignature.mergeNestedTypeMetadata(
Review Comment:
[P1] Retype typed NULL children to the refreshed array item shape
A reduced path is:
```text
LeftOuterJoin(other = length(to_json(
array(struct(l.number, r.number), NULL)))) > 0)
```
Initial coercion types the NULL as the original required-field struct.
AdjustNullable then makes `r.number` nullable, and this merge publishes that
refreshed struct as every signature argument and as the array item, but it does
not change the typed NullLiteral child. ExpressionTranslator sends the actual
old child type. BE consequently creates the array result with a nullable-field
Struct while the NULL source is `ColumnNullable(ColumnStruct<required field>)`;
array construction materializes it and recursively copies the nested payload
even when its null bit is set, so the destination nullable field receives a
non-nullable source column. Please retype typed-NULL children (and parallel
multi-input constructors such as CreateMap) to the refreshed common type, and
add an executing outer-join regression; the current FE-only typed-NULL test
checks only the signature.
--
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]
