HappenLee commented on code in PR #66942:
URL: https://github.com/apache/doris/pull/66942#discussion_r3860144120
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java:
##########
@@ -856,6 +857,25 @@ private FunctionCallExpr convertToStateCombinator(String
name, FunctionCallExpr
return new FunctionCallExpr(fn, new
FunctionParams(fnCall.getChildren()), false);
}
+ @Override
+ public Expr visitCombineCombinator(CombineCombinator combinator,
PlanTranslatorContext context) {
+ List<Expr> arguments = combinator.children().stream()
+ .map(arg -> new SlotRef(arg.getDataType().toCatalogDataType(),
arg.nullable()))
+ .collect(ImmutableList.toImmutableList());
+ FunctionCallExpr functionCallExpr = new FunctionCallExpr(
+ visitAggregateFunction(combinator.getNestedFunction(),
context).getFn(),
+ new FunctionParams(false, arguments), false);
+ Function aggregateFunction = functionCallExpr.getFn();
+ aggregateFunction.setName(new FunctionName(combinator.getName()));
+
aggregateFunction.setArgs(arguments.stream().map(Expr::getType).collect(Collectors.toList()));
+ aggregateFunction.setBinaryType(Function.BinaryType.AGG_STATE);
Review Comment:
Thanks for raising this. This failure path requires a new FE that can plan
and emit `_combine` to coexist with an old, query-eligible BE.
Under the supported Doris upgrade contract, all BEs are upgraded before any
FE is upgraded:
- During the mixed-BE phase, the FE is still old and cannot resolve or emit
`_combine`.
- During the FE rolling-upgrade phase, all BEs have already been upgraded
and support `AggregateStateCombine`.
Therefore, `isSmoothUpgradeSrc()` being query-available by itself does not
make this path reachable. The documented Cloud upgrade procedure explicitly
requires confirming that all BE instances have been upgraded before upgrading
FE:
https://doris.apache.org/docs/4.x/compute-storage-decoupled/upgrade/
The general upgrade procedure states the same BE-before-FE ordering:
https://doris.apache.org/docs/dev/admin-manual/cluster-management/upgrade/
Keeping the same `be_exec_version` is also expected here because this change
does not alter the serialized block format; compatibility is provided by the
BE-first rollout order.
Unless there is a supported Cloud rollout workflow in which a new FE is
deployed before every old smooth-upgrade source BE in every compute group has
been retired, this mixed-version path is outside the supported upgrade sequence
and should not require a runtime fence. Please identify such a supported
workflow if one exists; otherwise I believe this finding can be resolved.
--
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]
