Repository navigation
[fix](test) use Cloud Table Stream read-state hook - #67079
Merged
hello-stephen merged 1 commit intoAug 25, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review opinion: approve
No actionable issues found in PR #67079.
Critical checkpoint conclusions:
- Goal and test proof: the patch removes the stale test reference to the deleted
ResolveCloudTableStreamReadStaterule and exercises the production owner,CloudTableStreamReadStateHook.resolve. The existing assertions still prove that a missing-binding response raises the expected NereidsAnalysisException, installs no wrapper state, can be retried successfully, and issues two complete requests. - Scope and clarity: this is a minimal, focused test-only change (one file, five additions and three deletions) with a useful lifecycle comment and no production behavior change.
- Lifecycle and error handling:
BindRelationregisters the hook in Cloud mode,NereidsPlannerruns it after analysis, and lineage captures the same analyzed-plan object used here. The reflection helper rethrows runtime causes unchanged, while response membership validation finishes before wrapper installation for the failure under test, preserving atomic retry behavior. - Concurrency and locking: no thread, lock, shared-state access pattern, or concurrent lifecycle is introduced by this patch; the mocked calls and assertions are sequential.
- Parallel paths and coverage: neighboring tests cover hook registration/
afterAnalyze, CTE and rewrite behavior, snapshot scans, predicate elimination, and Cloud partition selection. This retry test remains correctly focused on partial-response atomicity. - Compatibility, configuration, persistence, writes, and FE-BE propagation: no configuration definition, protocol/storage format, journal, transaction, data-write path, or FE-BE variable changes are present.
- Performance and observability: the only overhead is two reflective calls in a unit test; no production hot path or observability requirement changes.
- Test status: the PR description reports all 13 tests in
InsertIntoTableCommandTableStreamTestpassed. I did not run builds or tests because the review runner explicitly prohibits builds. Live Checkstyle passed. The FE UT job failed before reachingfe-corebecause Maven Central connections reset while resolvingfe-kerberosdependencies (the 163 tests reached before that point passed), so that failure is unrelated to this test-only patch; the compile job was still pending during review. - User focus: no additional focus point was supplied; the complete PR was reviewed.
One normal full-review pass and a separate risk-focused pass both returned NO_NEW_VALUABLE_FINDINGS. The final changed-file, unresolved-candidate, and live duplicate sweeps are clean, so this review is complete with zero inline comments.
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 16588 ms |
Contributor
TPC-DS: Total hot run time: 81204 ms |
Contributor
ClickBench: Total hot run time: 14.49 s |
hello-stephen
approved these changes
Aug 25, 2026
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
35 of 69 tasks
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
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?
A Cloud Table Stream test still instantiated
ResolveCloudTableStreamReadStateafter that generic rewrite rule was removed, causing FE test compilation to fail.What is changed?
Invoke
CloudTableStreamReadStateHook.resolvethrough the test existing reflection utility. This keeps the partial-response retry coverage on the production post-analysis lifecycle that now owns read-state installation.Testing
InsertIntoTableCommandTableStreamTest: 13 tests passed