Skip to content

Conversation

@gouhongshen
Copy link
Contributor

@gouhongshen gouhongshen commented Nov 19, 2025

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue ##17994

What this PR does / why we need it:

PR Summary

  • Added OR-aware parsing in BasePKFilter: the parser now distributes AND over OR, builds Disjuncts, and invalidates the whole OR if any branch can’t be mapped to a PK predicate. This enables
    expressions such as (pk = 1 AND pk BETWEEN 5 AND 9) OR pk IN (20, 30) to produce multiple atomic filters.
  • ConstructBlockPKFilter understands those disjuncts, builds per-atom search functions, and unions their offsets so a single filter can cover mixed operators (e.g. pk prefix_eq 'ab' OR pk >= 1024). Mem
    filters remain single-op.
  • Strengthened unit tests (TestConstructBasePKFilterWithOr, TestConstructBlockPKFilterWithOr) to cover composite OR shapes and datatype coverage; vectors are freed properly after each test.
  • Updated BVT SQL suites (block_or_single_pk.sql, block_or_composite_pk.sql, block_or_no_pk.sql) to insert 8K rows with fault injection hooks, add rich OR queries (<10 rows per result, direct row
    outputs), and cover (a AND b) OR c plus long OR chains across all supported PK types.

Examples

  1. Query SELECT ... WHERE pk = 5 OR pk BETWEEN 100 AND 120 OR pk IN (512, 1024) now yields a BlockPKFilter whose SortedSearchFunc merges binary search offsets from all three predicates, ensuring every
    block overlap is found.
  2. Complex condition (pk >= 10 AND pk <= 20) OR pk prefix_eq 'ab' OR pk IN ('z1','z2') now expands into three disjuncts; blocks matching any disjunct are scanned, while MemPKFilter correctly deems
    multi-op filters invalid.

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

Labels

kind/enhancement size/XL Denotes a PR that changes [1000, 1999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants