github-actions[bot] commented on code in PR #66904:
URL: https://github.com/apache/doris/pull/66904#discussion_r3805014479
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -780,7 +800,7 @@ private void executeByNereids(TUniqueId queryId) throws
Exception {
profile.getSummaryProfile().setQueryBeginTime(TimeUtils.getStartTimeMs());
// short circuit query should not dump changed session var since it
will impact the performance.
- if (context.getSessionVariable().enableProfile &&
!statementContext.isShortCircuitQuery()) {
+ if (effectiveEnableProfile && !statementContext.isShortCircuitQuery())
{
Review Comment:
[P2] Capture session variables after the current hints are active
For `COM_STMT_EXECUTE`, this snapshot runs before `ExecuteCommand` re-enters
execution and again before analysis reapplies the retained `SET_VAR` map. If
the base group is disallowed but the hint selects an allowed group,
`effectiveEnableProfile` is false at both checks; the later refresh enables and
publishes the profile, but nothing ever fills `changedSessionVarCache`. It
therefore omits the nondefault `compute_group` override that selected it and
other nondefault hinted settings. Please snapshot/backfill at the shared
matching post-hint boundary, before rollback; that boundary should also cover
eager multi-statements, whose snapshot can observe another statement's hint.
Add a prepared false-to-true assertion on the rendered values.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -586,6 +594,18 @@ public void setStatementContext(StatementContext
statementContext) {
this.context.setStatementContext(statementContext);
}
+ /** Refresh the profile decision after a prepared statement reapplies its
SET_VAR hint. */
+ public void refreshEffectiveEnableProfileAfterSetVar(StatementContext
hintStatementContext) {
+ if (!isComStmtExecute || statementContext != hintStatementContext) {
+ return;
+ }
+ boolean enableProfile =
context.getSessionVariable().enableProfile(context);
+ if (enableProfile && !effectiveEnableProfile) {
+ profile.enable();
Review Comment:
[P2] Synchronize Profile settings when enabling after hints
This executor constructs a disabled `Profile` before a prepared statement
reapplies its retained `SET_VAR` map. When that hint moves the query to an
allowed group, this call enables the object, but `Profile.enable()` updates
neither `profileLevel` nor `autoProfileDurationMs`. A combined
`profile_level=3` or `auto_profile_threshold_ms=10000` hint therefore leaves FE
on constructor-time values: the coordinator carries the post-hint profile level
while FE can omit its distributed plan, and the FE-only threshold can retain a
profile that should be removed. Please synchronize both fields at the matching
post-hint boundary (covering every refresh, not only the boolean transition),
and test the false-to-true combined-hint case.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -586,6 +594,18 @@ public void setStatementContext(StatementContext
statementContext) {
this.context.setStatementContext(statementContext);
}
+ /** Refresh the profile decision after a prepared statement reapplies its
SET_VAR hint. */
+ public void refreshEffectiveEnableProfileAfterSetVar(StatementContext
hintStatementContext) {
+ if (!isComStmtExecute || statementContext != hintStatementContext) {
Review Comment:
[P2] Refresh the decision for ordinary multi-statements too
`parseMultiple` visits every statement before `ConnectProcessor` constructs
the first executor, and each parser-time `SET_VAR` mutates the shared
`SessionVariable`. For a packet whose statements hint `cg_b` and `cg_c`, the
first executor can therefore snapshot `cg_c`; its analyzer then reapplies
`cg_b` and `toThrift()` enables BE reporting for `cg_b`, but this guard rejects
the refresh because the command is `COM_QUERY`. The FE `Profile` and
coordinator can consequently make opposite decisions (and the first rollback
similarly affects the next snapshot), causing missing profiles or unnecessary
BE reporting. Please refresh whenever the hint context is the executor's
current context, or defer the snapshot until that statement's hint is active,
and cover distinct hints in one multi-statement request.
--
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]
