You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "jacktengg (via GitHub)" <gi...@apache.org> on 2026/04/20 16:02:20 UTC

[PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

jacktengg opened a new pull request, #62651:
URL: https://github.com/apache/doris/pull/62651

   
   When a SQL query fails before reaching Coordinator.exec() (e.g., "Database does not exist" during Nereids planning), the workload_group field in the audit_log is empty. This is because workload group name is only set on ConnectContext inside Coordinator.exec(), which is never reached for queries that fail during planning/analysis.
   
   The workload group name was also never reset between queries on the same connection, so the bug only manifests on fresh connections where no previous query has reached Coordinator.exec(). On reused connections, the stale value from a prior query masks the issue.
   
   Fix: Pre-resolve the workload group name at the start of StmtExecutor.execute() using the same resolution logic as WorkloadGroupMgr (session variable -> user property -> default). Coordinator.exec() will later overwrite this with the authoritative value after privilege checking and validation.
   
   ### What problem does this PR solve?
   
   Issue Number: close #xxx
   
   Related PR: #xxx
   
   Problem Summary:
   
   ### Release note
   
   None
   
   ### Check List (For Author)
   
   - Test <!-- At least one of them must be included. -->
       - [ ] Regression test
       - [ ] Unit Test
       - [ ] Manual test (add detailed scripts or steps below)
       - [ ] No need to test or manual test. Explain why:
           - [ ] This is a refactor/code format and no logic has been changed.
           - [ ] Previous test can cover this change.
           - [ ] No code files have been changed.
           - [ ] Other reason <!-- Add your reason?  -->
   
   - Behavior changed:
       - [ ] No.
       - [ ] Yes. <!-- Explain the behavior change -->
   
   - Does this need documentation?
       - [ ] No.
       - [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
   
   ### Check List (For Reviewer who merge this PR)
   
   - [ ] Confirm the release note
   - [ ] Confirm test cases
   - [ ] Confirm document
   - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
   
   


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62651:
URL: https://github.com/apache/doris/pull/62651#discussion_r3112087646


##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -646,6 +653,35 @@ public void execute(TUniqueId queryId) throws Exception {
         }
     }
 
+
+    /**
+     * Pre-resolve the workload group name and set it on ConnectContext.
+     * This ensures the workload group is available for audit logging even if the query
+     * fails before reaching Coordinator.exec() (e.g., "Database does not exist" during planning).
+     * Coordinator.exec() will later overwrite this with the authoritative value after

Review Comment:
   `executeInternalQuery()` still has the same lifecycle that motivated this fix: it writes the audit log in `finally`, but `workloadGroupName` is only populated inside `coord.exec()`. If parsing or planning throws before `coord.exec()`, internal SQL still records an empty `audit_log.workload_group`. Since this PR is fixing that audit-log invariant, please apply the same pre-resolution there as well, or centralize the initialization so both paths share it.



##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -646,6 +653,35 @@ public void execute(TUniqueId queryId) throws Exception {
         }
     }
 
+
+    /**
+     * Pre-resolve the workload group name and set it on ConnectContext.
+     * This ensures the workload group is available for audit logging even if the query
+     * fails before reaching Coordinator.exec() (e.g., "Database does not exist" during planning).
+     * Coordinator.exec() will later overwrite this with the authoritative value after
+     * privilege checking and workload group validation.
+     */
+    private void resolveWorkloadGroupName() {
+        if (!Config.enable_workload_group) {
+            context.setWorkloadGroupName("");
+            return;
+        }
+        try {
+            String groupName = context.getSessionVariable().getWorkloadGroup();
+            if (Strings.isNullOrEmpty(groupName)) {
+                groupName = Env.getCurrentEnv().getAuth().getWorkloadGroup(context.getQualifiedUser());
+            }
+            if (Strings.isNullOrEmpty(groupName)) {
+                groupName = WorkloadGroupMgr.DEFAULT_GROUP_NAME;

Review Comment:
   If pre-resolution throws here, we leave the previous statement's `workloadGroupName` on the reused `ConnectContext`, so the next planning failure can still be audited under a stale group. This is reachable in production: `Auth.getWorkloadGroup()` can hit the LDAP-backed `UserPropertyMgr.getPropertyIfNull()` path, and `LdapClient.getDn()` / `getUserDn()` throws `RuntimeException` on LDAP misconfiguration or duplicate users. Please clear `context.workloadGroupName` before the `try` (or in this `catch`) so failure falls back to empty instead of reusing the last query's value.



-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "jacktengg (via GitHub)" <gi...@apache.org>.
jacktengg commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4289889812

   run buildall


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #62651:
URL: https://github.com/apache/doris/pull/62651


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "jacktengg (via GitHub)" <gi...@apache.org>.
jacktengg commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4289887834

   /review


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4294093641

   PR approved by at least one committer and no changes requested.


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4282398869

   
   Thank you for your contribution to Apache Doris.
   Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
   
   Please clearly describe your PR:
   1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
   2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
   3. What features were added. Why was this function added?
   4. Which code was refactored and why was this part of the code refactored?
   5. Which functions were optimized and what is the difference before and after the optimization?
       


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62651:
URL: https://github.com/apache/doris/pull/62651#discussion_r3118130118


##########
fe/fe-core/src/main/java/org/apache/doris/qe/MysqlConnectProcessor.java:
##########
@@ -248,6 +248,7 @@ private void dispatch() throws IOException {
         }
         ctx.setCommand(command);
         ctx.setStartTime();
+        resolveWorkloadGroupName();

Review Comment:
   `resolveWorkloadGroupName()` only runs once per incoming packet here, but `ConnectProcessor.executeQuery()` can split a `COM_QUERY` text into multiple statements and execute them sequentially. If an earlier statement changes `@@workload_group`, later statements that fail before `Coordinator.exec()` will still write the stale pre-packet value to the audit log.
   
   A concrete case is `set workload_group = "wg_a"; select * from missing_db.t;`: the second statement is still audited with the old group, not `wg_a`. This needs to be refreshed at the per-statement boundary in `executeQuery()`, not only once in `dispatch()`. 



-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4285197866

   PR approved by at least one committer and no changes requested.


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "jacktengg (via GitHub)" <gi...@apache.org>.
jacktengg commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4282375919

   /review


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62651:
URL: https://github.com/apache/doris/pull/62651#discussion_r3114903666


##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -613,6 +615,11 @@ public void execute(TUniqueId queryId) throws Exception {
 
         try {
             try {
+                // Resolve workload group name early so it is available for audit logging even
+                // if the query fails before reaching Coordinator.exec() (e.g. during planning).
+                // Coordinator.exec() will overwrite this with the authoritative value later.
+                resolveWorkloadGroupName();

Review Comment:
   This only covers failures after `StmtExecutor.execute()` has already started. `ConnectProcessor.executeQuery()` still parses SQL before it constructs a `StmtExecutor`, and parse failures go straight to `handleQueryException()` -> `AuditLogHelper.logAuditLog()` (`ConnectProcessor.java` around the `parseWithFallback()` / `handleQueryException()` path). That means `SELEC 1` on a fresh connection still logs an empty `workload_group`, and the same syntax error on a reused connection still reuses the previous query's value because nothing cleared `ctx.workloadGroupName` before parse. Since the bug is connection-scoped stale state, the reset/resolve needs to happen before parse/audit in `ConnectProcessor` too, not only here.



-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4293276411

   # FE Regression Coverage Report
   Increment line coverage `  45.83% (11/24)` :tada:
   [Increment coverage report](http://coverage.selectdb-in.cc/coverage/62651_87c7e06df7475005b79f664562f86753889f2ddf_merge_fe/increment_report/index.html)
   [Complete coverage report](http://coverage.selectdb-in.cc/coverage/62651_87c7e06df7475005b79f664562f86753889f2ddf_merge_fe/report/index.html)
   


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "jacktengg (via GitHub)" <gi...@apache.org>.
jacktengg commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4285753419

   /review


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "jacktengg (via GitHub)" <gi...@apache.org>.
jacktengg commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4288916219

   /review


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


Re: [PR] [fix](audit log) fix empty workload_group value __internal_schema.audit_log [doris]

Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62651:
URL: https://github.com/apache/doris/pull/62651#issuecomment-4290488891

   # FE UT Coverage Report
   Increment line coverage `  100.00% (13/13)` :tada:
   [Increment coverage report](http://coverage.selectdb-in.cc/coverage/62651_87c7e06df7475005b79f664562f86753889f2ddf/fe_increment_report/index.html)
   [Complete coverage report](http://coverage.selectdb-in.cc/coverage/62651_87c7e06df7475005b79f664562f86753889f2ddf/fe_report/index.html)
   


-- 
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: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org