github-actions[bot] commented on code in PR #67256:
URL: https://github.com/apache/doris/pull/67256#discussion_r3877984253
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java:
##########
@@ -395,6 +395,53 @@ private StatementContext(ConnectContext connectContext,
OriginStatement originSt
}
}
+ /**
+ * Create a fresh StatementContext for the next EXECUTE of a prepared
statement.
+ *
+ * <p>A prepared statement keeps its StatementContext inside {@code
PreparedStatementContext}
+ * for the whole lifetime of the connection. Reusing the same object
across executions makes
+ * its per-statement state (bound tables, CTE maps, statistics, snapshots,
connector scope,
+ * ...) accumulate and it is only released when the connection closes,
which can OOM
+ * long-lived connections. Instead of clearing in place, allocate a
brand-new context per
+ * EXECUTE and copy over only the state that must survive between
executions, so the previous
+ * context becomes unreachable and is promptly GC'd.
+ *
+ * <p>Carried over:
+ * <ul>
+ * <li>id generator positions, so ids generated during this execution
never collide with
+ * ids already present in the cached analyzed plan from PREPARE;</li>
+ * <li>the placeholder real expressions bound by this EXECUTE (the
protocol layer fills
+ * them on the previous context before this method runs) and the
placeholder list;</li>
+ * <li>the placeholder to comparison-slot registry used by the
short-circuit fast path;</li>
+ * <li>the short-circuit and nondeterministic flags that gate the
short-circuit fast path
+ * before this execution re-plans.</li>
+ * </ul>
+ * Everything else (tables, CTEs, statistics, snapshots, planner
resources, connector
+ * scope, ...) starts empty/fresh on the new context.
+ */
+ public StatementContext createNextExecuteContext() {
+ // Continue the id generators from the previous context. The cached
analyzed plan from
+ // PREPARE (and every prior execution) already consumed ids from them,
so a fresh
+ // generator starting at 0 would collide with those ids during this
execution's planning.
+ StatementContext next = new StatementContext(connectContext,
originStatement,
+ exprIdGenerator.getCurrentId());
+ next.objectIdGenerator.resetId(objectIdGenerator.getCurrentId());
+ next.relationIdGenerator.resetId(relationIdGenerator.getCurrentId());
+ next.cteIdGenerator.resetId(cteIdGenerator.getCurrentId());
+ next.talbeIdGenerator.resetId(talbeIdGenerator.getCurrentId());
+
next.placeHolderIdGenerator.resetId(placeHolderIdGenerator.getCurrentId());
+ // Placeholder bindings of this EXECUTE, and the comparison-slot
registry used to replace
+ // conjuncts on the cached short-circuit plan without re-planning.
+ next.idToPlaceholderRealExpr.putAll(idToPlaceholderRealExpr);
+ next.idToComparisonSlot.putAll(idToComparisonSlot);
+ next.placeholders = new ArrayList<>(placeholders);
+ // Short-circuit gating flags are computed by the previous execution's
planning and gate
+ // the fast path of this execution before any re-planning happens.
+ next.isShortCircuitQuery = isShortCircuitQuery;
Review Comment:
Blocking: the fresh context carries isShortCircuitQuery=true but drops the
cached ShortCircuitQueryContext. On the second reusable EXECUTE, ExecuteCommand
enters directExecuteShortCircuitQuery without planning this executor.
StmtExecutor.executeAndSendResult then sees a null statement-level cache and
falls back to new ShortCircuitQueryContext(planner, ...); planner is null on
this direct path, and the constructor immediately dereferences it via
planner.getDescTable(), so the prepared point query fails with an NPE. Please
install the already-validated prepared cache on the fresh context before direct
execution (or preserve it with equivalent invalidation semantics) and cover two
executions of the fast path.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/PreparedStatementContext.java:
##########
@@ -54,6 +54,23 @@ public void setStatementContext(StatementContext
statementContext) {
this.statementContext = statementContext;
}
+ /**
+ * Allocate a fresh StatementContext for this EXECUTE and replace the
previous one, so the
+ * old context (with the per-statement state accumulated by prior
executions: bound tables,
+ * CTE maps, statistics, snapshots, ...) becomes unreachable and is
promptly GC'd.
+ *
+ * <p>A prepared statement lives as long as its connection. Reusing one
StatementContext
+ * across all executions would keep growing those maps and could OOM
long-lived connections,
+ * so we create a new object per execution and carry over only the state
that must survive
+ * (placeholder bindings, comparison slots, id generator positions,
short-circuit flags).
+ *
+ * @return the fresh StatementContext to use for the current execution
+ */
+ public StatementContext nextStatementContext() {
+ statementContext = statementContext.createNextExecuteContext();
Review Comment:
Blocking: this assignment discards the outgoing context without closing its
connector scope. COM_STMT_EXECUTE does not run the ConnectProcessor.handleQuery
StatementContext.close finally, and ConnectContext.clear only nulls the
connection reference. Coordinated scans register a query-finish close, but
connector commands and failures before scan registration have no callback and
relied on the removed next-execution reset. Repeated executions can therefore
abandon closeable metadata or an active connector transaction without closeAll;
GC will not finalize them. Please close/reset the outgoing scope before
replacing it, and make the wiring test assert that a seeded closeable or
transaction is finalized.
--
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]
