Skip to content

[improvement](be) Prune sparse IN predicates with full-set zone map checks - #70559

Open
HappenLee wants to merge 1 commit into
apache:masterfrom
HappenLee:pr/in-list-zonemap
Open

HappenLee wants to merge 1 commit into
apache:masterfrom
HappenLee:pr/in-list-zonemap

Conversation

@HappenLee

Copy link
Copy Markdown
Contributor

Background

Doris uses segment and page zone maps to exclude data ranges before reading rows. A storage IN predicate currently compares the zone's bounds with the minimum and maximum IN values. Row-level membership checks still use the full IN set.

What problem does this PR solve?

Issue Number: None
Related PR: None
PR Type: Enhancement

Problem Summary:

Sparse IN lists can retain zones containing no candidate values. For example, IN (10, 30) retains a zone bounded by [11, 29] because the bounding ranges overlap. Check the existing typed HybridSet for a value inside the zone after the cheap bounds check. Most of the diff adds tests for pruning boundaries and conservative fallbacks.

Details

  • Before and after: Segment and page zone maps can now reject gaps within an IN list's bounding range. Inclusive endpoints remain candidates; row-level membership checks remain unchanged.
  • Configs and defaults: Reuse in_zonemap_point_check_threshold (default 8192). Sets above the threshold retain bounds-only checks. Dense-domain containers also retain bounds-only checks to avoid scanning an entire domain for each zone.
  • Impact on existing users: Sparse IN predicates may read fewer segments and pages. Eligible checks take O(N) per zone, bounded by the configured threshold, and use O(1) additional space. No workload latency benchmark has been run.
  • Behavior change: More selective metadata pruning. Negated predicates and IN sets containing NaN retain conservative handling.

Testing

  • Local ASAN on the development baseline: ./run-be-ut.sh -j 48 --run --filter='InListPredicateZoneMapTest.*:HybridSetTest.*:ExprZonemapFilterTest.*' passed all 78 tests, including 11 new tests and an exhaustive comparison against set intersection for 741 integer ranges.
  • Coverage includes fixed and dynamic sets, inclusive endpoints, NULL/empty sets, negative predicates, threshold boundaries, dense-domain fallback, binary strings, predicate cloning, decimals, datetime fractions, NaNs, signed zero and infinities. SparseIntegerSet exercises the gap that bounds-only checking retains.
  • On the rebased master branch: clang-format 16, build hygiene and git diff --check passed. The ASAN suite has not been rerun on this rebased baseline.
  • Local clang-tidy on the development baseline was blocked by a pre-existing unmatched NOLINTEND in core/types.h.

Notes for Reviewers

  • Scope and reuse: Only the storage IN predicate and its unit tests change. The implementation reuses the existing HybridSet::contains_any_in_range interface.
  • Concurrency and lifetime: The scan reads the predicate's existing set; the change adds no mutable predicate state, ownership changes or locks.
  • Parallel paths: Both native segment and page pruning call this method. Expression zone-map filtering already uses the range helper. Parquet statistics and delete predicates are outside this change.
  • Preconditions and fallbacks: Readers already bypass unusable zone maps. Comments and tests explain the negative-predicate, dense-domain and NaN cases.
  • Compatibility: No storage-format, protocol, persistence or write-path changes; no new configuration or FE/BE variables.
  • Observability: Existing scan pruning counters expose skipped data; no new per-zone logging is introduced.

Release note

Improve segment and page pruning for sparse IN predicates.

Check List (For Author)

  • Behavior changed: Yes (see Details).
  • Does this need documentation: No.
  • Pre-submission status: Partially verified: local ASAN passed on the development baseline; the rebased master branch needs CI validation. See Testing for the local clang-tidy limitation.

AI Disclosure

  • Tool: Codex (GPT-6).
  • Generated: Implementation, unit tests and this description.
  • Human review: Pending maintainer review.

…hecks

### Background

Doris uses segment and page zone maps to exclude data ranges before reading rows. A storage IN predicate currently compares the zone's bounds with the minimum and maximum IN values. Row-level membership checks still use the full IN set.

### What problem does this PR solve?

Issue Number: None
Related PR: None
PR Type: Enhancement

Problem Summary:

Sparse IN lists can retain zones containing no candidate values. For example, `IN (10, 30)` retains a zone bounded by `[11, 29]` because the bounding ranges overlap. Check the existing typed `HybridSet` for a value inside the zone after the cheap bounds check. Most of the diff adds tests for pruning boundaries and conservative fallbacks.

### Details

- **Before and after:** Segment and page zone maps can now reject gaps within an IN list's bounding range. Inclusive endpoints remain candidates; row-level membership checks remain unchanged.
- **Configs and defaults:** Reuse `in_zonemap_point_check_threshold` (default `8192`). Sets above the threshold retain bounds-only checks. Dense-domain containers also retain bounds-only checks to avoid scanning an entire domain for each zone.
- **Impact on existing users:** Sparse IN predicates may read fewer segments and pages. Eligible checks take O(N) per zone, bounded by the configured threshold, and use O(1) additional space. No workload latency benchmark has been run.
- **Behavior change:** More selective metadata pruning. Negated predicates and IN sets containing NaN retain conservative handling.

### Testing

- Local ASAN on the development baseline: `./run-be-ut.sh -j 48 --run --filter='InListPredicateZoneMapTest.*:HybridSetTest.*:ExprZonemapFilterTest.*'` passed all 78 tests, including 11 new tests and an exhaustive comparison against set intersection for 741 integer ranges.
- Coverage includes fixed and dynamic sets, inclusive endpoints, NULL/empty sets, negative predicates, threshold boundaries, dense-domain fallback, binary strings, predicate cloning, decimals, datetime fractions, NaNs, signed zero and infinities. `SparseIntegerSet` exercises the gap that bounds-only checking retains.
- On the rebased master branch: clang-format 16, build hygiene and `git diff --check` passed. The ASAN suite has not been rerun on this rebased baseline.
- Local clang-tidy on the development baseline was blocked by a pre-existing unmatched `NOLINTEND` in `core/types.h`.

### Notes for Reviewers

- **Scope and reuse:** Only the storage IN predicate and its unit tests change. The implementation reuses the existing `HybridSet::contains_any_in_range` interface.
- **Concurrency and lifetime:** The scan reads the predicate's existing set; the change adds no mutable predicate state, ownership changes or locks.
- **Parallel paths:** Both native segment and page pruning call this method. Expression zone-map filtering already uses the range helper. Parquet statistics and delete predicates are outside this change.
- **Preconditions and fallbacks:** Readers already bypass unusable zone maps. Comments and tests explain the negative-predicate, dense-domain and NaN cases.
- **Compatibility:** No storage-format, protocol, persistence or write-path changes; no new configuration or FE/BE variables.
- **Observability:** Existing scan pruning counters expose skipped data; no new per-zone logging is introduced.

### Release note

Improve segment and page pruning for sparse IN predicates.

### Check List (For Author)

- Behavior changed: Yes (see Details).
- Does this need documentation: No.
- Pre-submission status: Partially verified: local ASAN passed on the development baseline; the rebased master branch needs CI validation. See Testing for the local clang-tidy limitation.

### AI Disclosure

- Tool: Codex (GPT-6).
- Generated: Implementation, unit tests and this description.
- Human review: Pending maintainer review.
@HappenLee

Copy link
Copy Markdown
Contributor Author

/review

@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 exact head 10de6701e37c2a131a52f290dea2b38c52226cec. No actionable new findings or existing blocking inline findings were identified. The review converged in 1 round: the complete reviewer and both separate risk reviewers returned NO_NEW_VALUABLE_FINDINGS; both risk questions were independently verified and dismissed with source evidence.

Checkpoint conclusions:

  1. Goal and evidence: Positive storage IN predicates can reject gaps within overlapping bounds, with inclusive endpoints retained. SparseIntegerSet distinguishes the former behavior; the independent integer oracle covers all 741 inclusive ranges over [-6,31].
  2. Scope and reuse: The two-file change is focused and reuses HybridSet::contains_any_in_range; row membership remains unchanged.
  3. Concurrency and locks: The new branch reads existing predicate state through const container iteration. It adds no threads, mutation, or locks; runtime-filter publication retains its existing synchronization.
  4. Lifecycle and initialization: Existing shared ownership retains sets and clones; string predicates own copied bytes. The clone test exercises destruction of the original predicate. No new static initialization, retained state, or allocation is introduced.
  5. Configuration: The existing mutable in_zonemap_point_check_threshold (8192 by default) is read on each evaluation. Tests cover equal/above/zero thresholds; no configuration is added.
  6. Compatibility: No function interface, storage format, persistence representation, or dependency changes. SQL/delete/runtime-filter set construction and zone-map decoding preserve primitive types, including legacy types; string aliases are handled explicitly.
  7. Thrift/protobuf: No definitions or field/enum IDs change.
  8. Parallel paths: Both native segment and page pruning reach this override; constant readers supply actual constant bounds. Unusable maps retain reader bypasses. Expression zone-map filtering already uses the helper; Parquet statistics, delete evaluation, dictionary/Bloom/inverted-index membership and NOT_IN_LIST remain separate paths.
  9. Conditions and special cases: Comments explain negation, dense-domain cost, NaN and sparse-gap checks. Doris ordering makes any set NaN the maximum, so the NaN fallback covers mixed and NaN-only lists. Signed zero, infinities, NULLs, opposite predicates, binary/CHAR strings and truncated bounds remain conservative through the traced reader/helper contracts.
  10. Test coverage: All 11 new unit tests were inspected: fixed/dynamic sets, inclusive boundaries, NULL/empty sets, negative forms, thresholds, dense-domain fallback, binary/empty strings, clone lifetime, decimal, datetime fractions and floating special values. CMake discovers the new test automatically.
  11. Test results: No expected-result files change; new expectations agree with set intersection by inspection. Author-reported test passes were not reproduced during this review.
  12. Observability: Existing segment pruning counters and page diagnostics cover the refined pruning; no added per-zone logging is needed.
  13. Transactions and persistence: No EditLog, visible-version, replication, rowset or delete-bitmap mutation changes.
  14. Data writes and atomicity: This is read-side metadata filtering; no new write, commit, failover or crash-cleanup path.
  15. FE/BE variables: No new variable or transmission path.
  16. Performance: Cheap bounds remain first; oversized and dense-domain sets bypass exact scans. Eligible lookups short-circuit, use O(N) candidate work bounded by the configured threshold and O(1) additional space. No latency improvement is asserted without measurement.
  17. Error/memory/other checks: The range helper's primitive-type/order assertions were traced upstream. No new discarded Status, ownership cycle, memory allocation, PCH/instantiation change, or additional verified issue was found.

No additional user focus was supplied. Coverage is complete for this static review, with no unresolved candidate or risk question. Validation limits: no builds, tests, formatting/hygiene scripts, SQL execution, or benchmarks were run. Added tests exercise the predicate directly; end-to-end segment/page execution, truncated CHAR/constant-reader integration, and workload performance were assessed from source rather than executed.

Review converged after 1 round(s).

@HappenLee

Copy link
Copy Markdown
Contributor Author

run buildall

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.

1 participant