branch-4.0: [test](nereids) Replace flaky null stats case with unit test - #65655
Open
shuke987 wants to merge 1 commit into
Open
branch-4.0: [test](nereids) Replace flaky null stats case with unit test#65655shuke987 wants to merge 1 commit into
shuke987 wants to merge 1 commit into
Conversation
…65445) Related PR: apache#62265 Problem Summary: `test_scale_num_nulls` checks that table-level `numNulls=3` is scaled to `1` when partition pruning selects 4 of 12 rows. The regression case executes `EXPLAIN MEMO PLAN` immediately after insert and analyze, but Cloud P0 reports the physical partition row count asynchronously. The planner can therefore observe a transient selected-partition row count of zero, while waiting for the real row count can take up to two minutes. This change: - adds deterministic FE unit coverage that invokes `StatsCalculator.computeOlapScan()` with a three-partition OLAP scan; - fixes the table row count at 12, selected-partition row count at 4, and cached table-level column statistics at `ndv=1`, `min=max=2`, and `numNulls=3`; - verifies the final scan statistics have `ndv=1`, `min=max=2`, `count=4`, and scaled `numNulls=1`; - removes the timing-dependent Groovy regression case. The unit test exercises the production partition-pruning statistics path using fixed in-memory inputs. It has no cluster, physical row-count reporting, polling, sleep, network, or storage dependency. Production behavior is unchanged.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
Reviewed PR 65655 at head 122607d0cecf2a0cbdb1487193e0ae154d85e9ef.
I did not find a substantiated, non-duplicative issue that needs an inline review comment.
Critical checkpoints:
- Scope: this is a test-only branch-4.0 backport of #65445. It adds deterministic FE unit coverage for selected-partition
numNullsscaling inStatsCalculator.computeOlapScanand deletes the timing-dependent Groovy regression. - Test correctness: the new unit test installs an initialized
ConnectContext, mocksEnv/StatisticsCache/OlapTable/LogicalOlapScan, forces one selected partition out of three, and verifies table row count 12 plus selected row count 4 scales table-levelnumNulls=3to1. The assertions are tied to the production branch, not Mockito defaults. - Coverage tradeoff: the deleted Groovy case was the SQL-visible coverage for analyze + partition pruning + memo-plan scaled
numNulls. That concern was verified, then dismissed here because this is a backport of upstream #65445, where the same planner-level coverage concern had already been raised and dismissed, and the PR documents the real flake source: asynchronously visible physical partition row counts. - Runtime behavior: no production code, persisted format, config, FE/BE protocol, transaction, lifecycle, or concurrency path changes in this PR.
- Duplicate context: this PR currently has no existing inline review comments or review threads, and no inline comments are proposed.
- User focus: no additional user-provided review focus was supplied.
- Subagents: Full Review A found no branch-fidelity issue; Full Review B and the risk-focused review both found the same coverage-loss candidate, which was merged as
M-1and dismissed/duplicated with evidence. All three subagents then returnedNO_NEW_VALUABLE_FINDINGSfor the final ledger state with no inline comments.
Validation performed:
- Read the review prompt, required review context, existing comments/threads, changed-file list, focus file, and shared ledger before reviewing the diff. No required AGENTS.md files were listed for this PR, and
.claude/skills/code-review/SKILL.mdwas absent in this checkout. - Reconfirmed the current PR head/base by GitHub API and confirmed there are still no PR inline comments or reviews.
- Performed static review of the changed files and relevant
StatsCalculator,ColumnStatisticBuilder,ConnectContext,SessionVariable, FE test dependency, and statistics regression coverage paths. - Ran
git diff --checkon the touched files successfully.
I did not run builds or tests because the review prompt says this runner is only for review operations and not to attempt builds.
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?
Related PR: #65445
Problem Summary:
Backport #65445 to branch-4.0.
The regression case
test_scale_num_nullsdepends on asynchronously visible physical row counts. When the physical row count is still 0, Nereids scales the selected-partition column statistics to 0 even though the injected partition/table statistics are correct, making the case flaky.Replace the environment-dependent Groovy regression case with a deterministic FE unit test that directly constructs the partition/table statistics and verifies selected-partition scaling, including row count, NDV, min/max, count, and numNulls.
Coverage rationale
StatsCalculator.computeOlapScan()path and verifies row count, NDV, min/max, count, andnumNulls. SQL predicate-to-partition selection remains covered byPruneOlapScanPartitionTeston this release branch.SHOW COLUMN STATSwas only failure diagnostics, and memo text formatting was not an independent assertion target of this case.Release note
None
Check List (For Author)
Test
Local validation:
StatsCalculatorTest#testComputeOlapScanScalesNumNullsForSelectedPartitions: passed (1 test)StatsCalculatorTest: passed (17 tests)mvn -pl fe-core -am checkstyle:check: passed (0 violations)Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)