github-actions[bot] commented on code in PR #68311:
URL: https://github.com/apache/doris/pull/68311#discussion_r4061211947
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnBE.java:
##########
@@ -174,6 +174,21 @@ public static Expression
foldByBE(ExpressionMatchingContext<Expression> context)
return root;
}
+ /** Evaluate a semantic constant whose value is required for argument
validation. */
+ public static Expression evaluateConstant(Expression expression,
ConnectContext context) {
+ // Required evaluation must also honor exclusions such as Sleep, which
can outlive
+ // the RPC timeout. Leave excluded expressions unevaluated for the
caller to reject.
+ if (expression.anyMatch(e -> shouldSkipFold((Expression) e))) {
+ return expression;
+ }
+ Expr legacyExpr = ExpressionTranslator.translate(expression, null);
+ Map<String, Expression> constants = Collections.singletonMap("0",
expression);
+ Map<String, TExpr> thriftExpressions = Collections.singletonMap(
+ "0", ExprToThriftVisitor.treeToThrift(legacyExpr));
+ return evalOnBE(Collections.singletonMap("0", thriftExpressions),
constants, context)
Review Comment:
[P2] Retry another healthy peer before rejecting the gram
This required path turns `evalOnBE`'s best-effort miss into an analysis
error, but `evalOnBE` shuffles the heartbeat-alive IDs and tries only
`backendIds.get(0)`. A selected BE whose BRPC endpoint is restarting, whose
light pool rejects the request, or whose transport fails returns the original
expression; `withFoldedGramNumber` then rejects a valid safe gram such as
`crc32('abc') % 3 + 1` even when other BEs can answer. Same-address channel
retries do not provide peer fallback. Please retry remaining compatible peers
for retryable endpoint/transport/overload failures within one shared overall
deadline (not five seconds per peer, and not deterministic expression
failures), and cover first-peer failure followed by second-peer success.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/NgramSearch.java:
##########
@@ -57,21 +63,47 @@ private NgramSearch(ScalarFunctionParams functionParams) {
@Override
public void checkLegalityBeforeTypeCoercion() {
- if (!child(1).isConstant()) {
+ if (!getArgument(1).isConstant()) {
throw new AnalysisException(
"ngram_search(text,pattern,gram_num): pattern support
const value only.");
}
- Expression gramNum = child(2);
- if (!(gramNum instanceof IntegerLikeLiteral)) {
+ Expression gramNum = getArgument(2);
+ if (!gramNum.isConstant() || !gramNum.getDataType().isIntegralType()) {
throw new AnalysisException(
"ngram_search(text,pattern,gram_num): gram_num support
const value only.");
}
- if (((IntegerLikeLiteral) gramNum).getIntValue() <= 0) {
+ gramNum = FoldConstantRuleOnFE.evaluateWithoutContext(gramNum);
+ if (gramNum instanceof NullLiteral) {
+ throw new AnalysisException(
+ "ngram_search(text,pattern,gram_num): gram_num support
const value only.");
+ }
+ if (gramNum instanceof IntegerLikeLiteral && ((IntegerLikeLiteral)
gramNum).getIntValue() <= 0) {
throw new AnalysisException(
"ngram_search(text,pattern,gram_num): gram_num must be a
positive constant.");
}
}
+ /** Resolve the required constant before rewrites can discard the function
call. */
+ public NgramSearch withFoldedGramNumber() {
+ Expression gramNum = getArgument(2);
+ if (!gramNum.getDataType().equals(IntegerType.INSTANCE)) {
+ gramNum = new Cast(gramNum, IntegerType.INSTANCE);
+ }
+ gramNum = FoldConstantRuleOnFE.evaluateWithoutContext(gramNum);
+ // Argument validation is independent of the optional BE folding
setting. Keep the
+ // evaluated value in the plan so CSE and execution use exactly the
value we validate.
+ if (!(gramNum instanceof Literal)) {
Review Comment:
[P1] Batch required folds before binding under table locks
For a cold plan over an internal table, `collectAndLockTable` acquires read
locks before `analyze()`, and those locks are released only after planning.
Every BE-only gram reaching this line then performs its own synchronous
singleton RPC. A single projection expression containing several safe
`ngram_search(..., crc32(...) % 3 + 1)` calls therefore creates N serialized
RPCs and N fresh BE fold executors/runtime states before CSE or ordinary
batched folding can run. Successful-but-slow calls can multiply the same
metadata-lock interval, blocking DDL; even healthy calls add repeated
network/setup cost. This is distinct from the already disclosed single planning
RPC. Please collect/batch these required constants outside table-lock ownership
(or evaluate them locally) and add multi-expression/lock-ownership coverage.
--
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]
