Skip to content

[branch-4.1][fix](fe) Disable expression ZoneMap filtering by default#65526

Merged
yiguolei merged 1 commit into
apache:branch-4.1from
mrhhsg:fix/disable-expr-zonemap-by-default
Jul 14, 2026
Merged

[branch-4.1][fix](fe) Disable expression ZoneMap filtering by default#65526
yiguolei merged 1 commit into
apache:branch-4.1from
mrhhsg:fix/disable-expr-zonemap-by-default

Conversation

@mrhhsg

@mrhhsg mrhhsg commented Jul 13, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: None

Related PR: #65263

Problem Summary:

Expression ZoneMap filtering was enabled by default when it was introduced on branch-4.1. This change disables enable_expr_zonemap_filter for regular new sessions and enables it in fuzzy mode when Config.pull_request_id % 2 == 0, so the fuzzy setting is deterministic for each PR while covering both enabled and disabled paths across PRs.

The dedicated expression ZoneMap regression suites now enable the session variable explicitly, keeping their profile-counter assertions deterministic regardless of the global default or fuzzy-mode PR parity.

For mixed-version compatibility, the absent Thrift-field behavior remains legacy-enabled. New FE sessions always serialize the session value explicitly through SessionVariable.toThrift().

Release note

Disable expression ZoneMap filtering by default on branch-4.1 and enable it for fuzzy-mode sessions with even pull request IDs.

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: the fuzzy value follows the existing PR-parity assignment pattern.

Validation:

  • git diff --check passed.

  • Behavior changed:

    • No.
    • Yes. Expression ZoneMap filtering is disabled by default for new sessions and enabled in fuzzy mode for even pull request IDs.
  • 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

@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?

@mrhhsg mrhhsg force-pushed the fix/disable-expr-zonemap-by-default branch from f437031 to d477dce Compare July 13, 2026 08:35
@mrhhsg

mrhhsg commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 0.00% (0/1) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/23) 🎉
Increment coverage report
Complete coverage report

@mrhhsg

mrhhsg commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

/review

@mrhhsg mrhhsg marked this pull request as ready for review July 14, 2026 01:30
@mrhhsg mrhhsg requested a review from yiguolei as a code owner July 14, 2026 01:30

@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 the default flip for enable_expr_zonemap_filter, the fuzzy-mode PR-parity assignment, and the two regression-suite opt-ins. I did not find a correctness issue worth an inline comment.

Critical checkpoints:

  • Existing review context: no prior inline comments or replies in the prepared review bundle.
  • Session/default behavior: new FE-created sessions serialize the explicit false default through SessionVariable.toThrift(), while the BE absent-thrift-field path remains legacy-enabled through expr_zonemap::is_expr_zonemap_filter_enabled().
  • Scan gating: OLAP segment/page pruning and Parquet row-group/page pruning all use the centralized BE helper.
  • Regression coverage: both changed expression-ZoneMap suites explicitly enable enable_expr_zonemap_filter and keep the required common-expression/profile settings before their profile-counter assertions.
  • Fuzzy mode: the PR-id parity assignment is deterministic; non-PR fuzzy mode with pull_request_id = 0 enables the feature, which I do not see as a regression in this PR.
  • User focus: no additional user-provided focus was present.
  • Review instruction note: the repository-local code-review skill required by the prompt was not discoverable in this checkout, so I followed the explicit prompt and ledger workflow.

Validation was static only; I did not run builds or regression tests in this review runner.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jul 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/175) 🎉
Increment coverage report
Complete coverage report

@yiguolei yiguolei merged commit 5e6f47d into apache:branch-4.1 Jul 14, 2026
33 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants