[branch-4.1][fix](fe) Disable expression ZoneMap filtering by default#65526
Open
mrhhsg wants to merge 1 commit into
Open
[branch-4.1][fix](fe) Disable expression ZoneMap filtering by default#65526mrhhsg wants to merge 1 commit into
mrhhsg wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
f437031 to
d477dce
Compare
Member
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Member
Author
|
/review |
Contributor
There was a problem hiding this comment.
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 throughexpr_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_filterand 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 = 0enables 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.
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?
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_filterfor regular new sessions and enables it in fuzzy mode whenConfig.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)
Validation:
git diff --checkpassed.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)