Uh oh!
There was an error while loading. Please reload this page.
[fix](audit)Fixed an issue that the audit log would record the previous queryId when parseSQL fails. - #53107
Conversation
hello-stephen
commented
Jul 11, 2025
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
zddr
commented
Jul 11, 2025
run buildall |
doris-robot
commented
Jul 11, 2025
TPC-H: Total hot run time: 32913 ms |
doris-robot
commented
Jul 11, 2025
TPC-DS: Total hot run time: 185955 ms |
doris-robot
commented
Jul 11, 2025
ClickBench: Total hot run time: 29.49 s |
hello-stephen
commented
Jul 11, 2025
FE UT Coverage ReportIncrement line coverage |
zddr
commented
Jul 24, 2025
run buildall |
doris-robot
commented
Jul 24, 2025
TPC-H: Total hot run time: 33878 ms |
doris-robot
commented
Jul 24, 2025
TPC-DS: Total hot run time: 187418 ms |
doris-robot
commented
Jul 24, 2025
ClickBench: Total hot run time: 32.41 s |
zddr
commented
Jul 24, 2025
run p0 |
| exprIdGenerator = ExprId.createGenerator(initialId); | ||
| if (connectContext != null && connectContext.getSessionVariable() != null | ||
| && connectContext.queryId() != null | ||
| //&& connectContext.queryId() != null |
There was a problem hiding this comment.
The queryId retrieved here is always null—the current query's queryId is set in subsequent steps, while what was obtained earlier actually belonged to the previous query.
| protected void handleQuery(String originStmt) throws ConnectionException { | ||
| // Before executing the query, the queryId should be set to empty. | ||
| // Otherwise, if SQL parsing fails, the audit log will record the queryId from the previous query. | ||
| ctx.resetQueryId(); |
There was a problem hiding this comment.
now, what will be in the audit log? null?
There was a problem hiding this comment.
.setQueryId(ctx.queryId() == null ? "NaN" : DebugUtil.printId(ctx.queryId()))
NAN
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
zddr
commented
Jul 25, 2025
run buildall |
hello-stephen
commented
Jul 25, 2025
FE UT Coverage ReportIncrement line coverage |
zddr
commented
Jul 25, 2025
run performance |
doris-robot
commented
Jul 25, 2025
TPC-H: Total hot run time: 33898 ms |
doris-robot
commented
Jul 25, 2025
TPC-DS: Total hot run time: 187085 ms |
doris-robot
commented
Jul 25, 2025
ClickBench: Total hot run time: 32.63 s |
Uh oh!
There was an error while loading. Please reload this page.
…us queryId when parseSQL fails. (apache#53107) ### What problem does this PR solve? Issue: - When parsing SQL, the queryId has not yet been generated. However, the audit log retrieves the queryId from ConnectContext, which means it uses the queryId from the previous execution. - The queryId stored in queryCache is from the previous query, not the current one. Solution: - Since parsing may produce multiple SQL statements, each should have its own queryId. Therefore, the queryId should not be set during parsing. As a compromise, we clear the queryId before parsing, ensuring that the audit log records an empty queryId if parsing fails. - Set the queryId when adding the cache, not in the constructor. # Conflicts: # fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java
…us queryId when parseSQL fails. (apache#53107) ### What problem does this PR solve? Issue: - When parsing SQL, the queryId has not yet been generated. However, the audit log retrieves the queryId from ConnectContext, which means it uses the queryId from the previous execution. - The queryId stored in queryCache is from the previous query, not the current one. Solution: - Since parsing may produce multiple SQL statements, each should have its own queryId. Therefore, the queryId should not be set during parsing. As a compromise, we clear the queryId before parsing, ensuring that the audit log records an empty queryId if parsing fails. - Set the queryId when adding the cache, not in the constructor. # Conflicts: # fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java # fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java # fe/fe-core/src/test/java/org/apache/doris/qe/ConnectContextTest.java
…us queryId when parseSQL fails. (apache#53107) ### What problem does this PR solve? Issue: - When parsing SQL, the queryId has not yet been generated. However, the audit log retrieves the queryId from ConnectContext, which means it uses the queryId from the previous execution. - The queryId stored in queryCache is from the previous query, not the current one. Solution: - Since parsing may produce multiple SQL statements, each should have its own queryId. Therefore, the queryId should not be set during parsing. As a compromise, we clear the queryId before parsing, ensuring that the audit log records an empty queryId if parsing fails. - Set the queryId when adding the cache, not in the constructor. # Conflicts: # fe/fe-core/src/main/java/org/apache/doris/common/cache/NereidsSqlCacheManager.java # fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java # fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java # fe/fe-core/src/test/java/org/apache/doris/qe/ConnectContextTest.java
…us queryId when parseSQL fails. (apache#53107) ### What problem does this PR solve? Issue: - When parsing SQL, the queryId has not yet been generated. However, the audit log retrieves the queryId from ConnectContext, which means it uses the queryId from the previous execution. - The queryId stored in queryCache is from the previous query, not the current one. Solution: - Since parsing may produce multiple SQL statements, each should have its own queryId. Therefore, the queryId should not be set during parsing. As a compromise, we clear the queryId before parsing, ensuring that the audit log records an empty queryId if parsing fails. - Set the queryId when adding the cache, not in the constructor.
What problem does this PR solve?
Issue:
Solution:
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Fixed an issue that the audit log would record the previous queryId when parseSQL fails.
Release note
Fixed an issue that the audit log would record the previous queryId when parseSQL fails.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)