[test](fe) Cover internal query audit failure in FE unit test#65696
Merged
Conversation
Replace the flaky regression case that polls __internal_schema.audit_log with a deterministic FE unit test that verifies executeInternalQuery submits an ERR audit event when planning fails. Key changes: - mock NereidsPlanner planning failure in StmtExecutorInternalQueryTest - verify the submitted AuditEvent records ERR state and failure details - remove the unstable fault-injection regression case Unit Test: - ./run-fe-ut.sh --run org.apache.doris.qe.StmtExecutorInternalQueryTest
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29506 ms |
Contributor
TPC-DS: Total hot run time: 177094 ms |
Contributor
Author
|
run feut |
Contributor
ClickBench: Total hot run time: 25.01 s |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review completed. I found no blocking correctness issue and no inline finding to submit.
Critical checkpoint conclusions:
- Goal and proof: This test-only PR replaces the timing-sensitive audit-table polling case with a deterministic FE unit test. The new test proves that a failed internal query reaches
AuditLogHelper, submits anAFTER_QUERYevent, and preservesERR, the internal error code/message, internal/query/Nereids flags, SQL text, and SQL hash. - Scope and clarity: The change is small and focused. The added
getStatementContext()stubs are necessary for the mocked planner to pass through the audit metadata branch instead of being stopped by a mock-induced null dereference. - Parallel paths and conditions: Parse/plan, coordinator execution, and result-fetch failures currently converge on the same outer error-state catch and audit
finally. The workload-status queue and audit processor pass the same event object onward, changing only runtime statistics, and the audit loader serializes the reviewed state/error/internal fields directly. I found no distinct current failure branch left uncovered by the new state-to-event test. - Concurrency and lifecycle: No production concurrency or lifecycle behavior changes. The test restores the temporary
Env.workloadRuntimeStatusMgrreplacement infinally; FE test classes run in non-reused JVM forks, no method-parallel mode is configured, and this test does not start the workload-status daemon, so no race or singleton leak was substantiated. - Configuration, compatibility, persistence, data writes, and FE/BE variables: No production configuration, protocol, storage-format, transaction, persistence, data-write, or FE/BE variable-passing change is present.
- Performance and observability: No production hot path or observability behavior is modified.
- Test coverage and results: The FE unit test, compile check, NonConcurrent Regression, and P0 Regression checks pass. The separate FE incremental-coverage check is red because this PR changes no production Java lines: its bot report contains a blank incremental value and the incremental report URL is absent, while the complete coverage report exists. I treated that as reporting infrastructure rather than a code defect. No local build was run because the review contract prohibits builds in this runner.
- User focus: No additional user-provided focus point was supplied; the complete PR was reviewed.
Review opinion: ready from a code-review perspective.
morrySnow
approved these changes
Jul 17, 2026
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
The previous regression coverage for internal query audit failures depended on querying
__internal_schema.audit_logafter a fault-injection failure. That path is timing-sensitive because audit persistence is asynchronous, so the regression can fail even whenStmtExecutor.executeInternalQuery()already reports the error audit event correctly.This PR replaces that flaky coverage with a deterministic FE unit test. The new test forces a planning failure in
executeInternalQuery()and verifies that the submittedAuditEventis marked asERRand carries the failure details. The unstable fault-injection regression case is removed.Release note
None