Skip to content

branch-4.0: [test](nereids) Replace flaky null stats case with unit test - #65655

Open
shuke987 wants to merge 1 commit into
apache:branch-4.0from
shuke987:codex/backport-65445-branch-4.0
Open

branch-4.0: [test](nereids) Replace flaky null stats case with unit test#65655
shuke987 wants to merge 1 commit into
apache:branch-4.0from
shuke987:codex/backport-65445-branch-4.0

Conversation

@shuke987

@shuke987 shuke987 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Related PR: #65445

Problem Summary:

Backport #65445 to branch-4.0.

The regression case test_scale_num_nulls depends 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

  • This is a branch backport of merged PR [test](nereids) Replace flaky null stats case with unit test #65445. The same planner-level coverage concern was raised in the upstream review thread, then explicitly resolved by a maintainer before the PR was approved and merged.
  • The independent behavior introduced with [opt](nereids) scale num_nulls in col stats when partition pruned #62265 is selected-partition statistics scaling. The replacement test invokes the real StatsCalculator.computeOlapScan() path and verifies row count, NDV, min/max, count, and numNulls. SQL predicate-to-partition selection remains covered by PruneOlapScanPartitionTest on this release branch.
  • The removed cluster case sampled the strict physical partition row count immediately after insert/analyze. In Cloud P0 that value can remain 0 for up to two minutes even when table-level column statistics are correct. Retaining the case or waiting for that report would preserve the flake or add up to two minutes to the suite.
  • SHOW COLUMN STATS was only failure diagnostics, and memo text formatting was not an independent assertion target of this case.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

    Local validation:

    • StatsCalculatorTest#testComputeOlapScanScalesNumNullsForSelectedPartitions: passed (1 test)
    • StatsCalculatorTest: passed (17 tests)
    • mvn -pl fe-core -am checkstyle:check: passed (0 violations)
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

…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.
@shuke987
shuke987 requested a review from morningman as a code owner July 15, 2026 10:18
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@shuke987

Copy link
Copy Markdown
Contributor Author

/review

@shuke987

Copy link
Copy Markdown
Contributor Author

run buildall

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 numNulls scaling in StatsCalculator.computeOlapScan and deletes the timing-dependent Groovy regression.
  • Test correctness: the new unit test installs an initialized ConnectContext, mocks Env/StatisticsCache/OlapTable/LogicalOlapScan, forces one selected partition out of three, and verifies table row count 12 plus selected row count 4 scales table-level numNulls=3 to 1. 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-1 and dismissed/duplicated with evidence. All three subagents then returned NO_NEW_VALUABLE_FINDINGS for 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.md was 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 --check on 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants