[fix](case) Relax show data compaction size check#65387
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed PR 65387.
I did not find a correctness issue worth an inline review comment. The patch only relaxes physical SHOW DATA size equality in test_show_data_with_compaction to a bounded relative comparison. The compared values are still checked for wait_timeout before the helper is called, full compaction is still explicitly awaited, and nearby inverted-index show-data tests already use relative tolerance for physical size comparisons.
Validation was static/GitHub-patch based in this runner: the checkout is shallow, local base lookup for the prompt base SHA is incomplete, and I did not run the p2 regression suite.
User focus: no additional user-provided review focus was supplied.
Subagent conclusions: optimizer-rewrite reported no optimizer/rewrite, semantic equivalence, join, aggregate, or parallel-path issue. tests-session-config reported no regression-test, expected-output, session/config propagation, compatibility, or style issue. Final convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same empty inline-comment set.
a5ff6b2 to
736406c
Compare
|
run buildall |
|
/review |
There was a problem hiding this comment.
Reviewed PR 65387 at 736406c.
I did not find a correctness issue worth an inline review comment.
Critical checkpoints:
- Changed-file scope: GitHub reports one modified file,
regression-test/suites/inverted_index_p2/test_show_data.groovy. - Existing review context: the prepared inline-thread context has no existing inline review comments or replies, so there was no duplicate inline issue to suppress.
- Test behavior: the patch only relaxes
SHOW DATAphysical-size assertions intest_show_data_with_compaction; the table definitions, stream loads,sync, full-compaction waits,inverted_index_compaction_enablesave/restore path, and SQL/query semantics are unchanged. - Expected outputs: this suite has no
.outartifact to update. - User focus: no additional user-provided review focus was supplied.
- Validation limits: review was static/GitHub-patch based in this runner. The checkout does not contain the prompt base SHA, and
thirdparty/installed,thirdparty/installed/bin/protoc, and builtoutput/artifacts are missing, so I did not run the p2 regression suite locally.
Subagent conclusions:
optimizer-rewritefound no optimizer/rewrite, semantic-equivalence, join, aggregate, or parallel-path issue.tests-session-configfound no regression-test, expected-output, session/config propagation, compatibility, generated-output, or basic CI/style issue.- Final convergence round 1 ended with both live subagents replying
NO_NEW_VALUABLE_FINDINGSfor the same empty proposed inline-comment set.
Repository review skill note: I searched the checkout for a repo-local SKILL.md / code-review skill before reviewing, but no such file was present in this runner, so I followed the explicit .code-review.ECQuo4/review_prompt.txt workflow.
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
skip buildall |
|
/review |
|
Address review feedback by keeping the relative size comparison between the documents table paths while preserving the cloud/non-cloud sanity ranges.\n\nrun buildall |
There was a problem hiding this comment.
Automated review summary for PR #65387.
I reviewed the single changed file, regression-test/suites/inverted_index_p2/test_show_data.groovy, including the surrounding test_show_data_with_compaction flow, the SHOW DATA output shape, compaction helper behavior, cloud-mode split, and comparable inverted-index size/compaction tests.
Opinion: I found no blocking issue and no substantiated inline comment to submit. The change keeps the documents-table enabled/disabled compaction comparison, adds absolute range checks for the observed cloud/non-cloud SHOW DATA sizes, and relaxes both equality checks into a bounded relative-difference assertion.
Critical checkpoints:
- Review prompt, prepared review context, changed-file list, existing review threads/comments, user focus, required-agents file, and shared ledger were read before diff review.
- No AGENTS.md files were required for the changed-file ancestors.
- The expected repo-local
.claude/skills/code-review/SKILL.mdwas not present after hidden-path search, so I followed the explicit review prompt and loaded workflow contract. - GitHub PR metadata confirmed head
a490d6e00261113c5d745f55e2d020cb2e1d5923is still current and the PR scope is exactly one modified file. - Existing inline review context was empty, so there were no prior threads to suppress.
- GitHub patch whitespace/trailing-space checks found no issue. Local ancestry-based
git diff --checkwas not usable because this checkout is shallow, so I used GitHub patch metadata for scope and validation. - I did not run the full regression suite locally in this runner.
Subagent conclusions:
optimizer-rewrite: no optimizer/rewrite, semantic-equivalence, or parallel join/aggregate issue found.tests-session-config: no regression-test, expected-output, session/config propagation, compatibility, or basic CI/style issue found.- Convergence round 1 ended with both live subagents replying
NO_NEW_VALUABLE_FINDINGSfor the same ledger state and empty proposed inline comment set.
User focus: no additional user-provided review focus was supplied.
|
skip buildall |
Summary
Testing