Skip to content

perf(readutil): preserve early filtering across fallback paths#25853

Merged
XuPeng-SH merged 3 commits into
mainfrom
codex/fix-25588-preserve-early-filters
Jul 18, 2026
Merged

perf(readutil): preserve early filtering across fallback paths#25853
XuPeng-SH merged 3 commits into
mainfrom
codex/fix-25588-preserve-early-filters

Conversation

@XuPeng-SH

Copy link
Copy Markdown
Contributor

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #25588

What this PR does / why we need it:

This PR preserves early filtering across three safe but expensive fallback paths:

  • represents OR-ed primary-key filters as atomic specs and heap-merges partition-state iterators, including tombstones, with overlap deduplication;
  • replaces the PREFIX_IN > 4 pass-through with an immutable, sorted prefix index and eagerly builds the regular IN lookup;
  • preserves compilable conjuncts in top-level and nested AND filters while keeping OR compilation all-or-nothing.

It also exposes disjunct/key counts for observability and adds BVT coverage for committed partition-state rows, persisted mixed filters, large compound-PK prefix lists, NULLs/duplicates, overlapping OR branches, and workspace rows.

Testing

  • owning-package tests: readutil, logtailreplay, and disttae
  • focused race tests (-count=10) and stress tests (-count=100)
  • full owning-package race tests
  • go vet and go build for modified packages
  • make build
  • local mo-tester BVT against the branch-built server: 23/23 passed (100%)
  • benchmark with 8,192 encoded keys and 1,024 prefixes: about 0.37-0.48 ms/op, 0 alloc/op

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@LeftHandCold LeftHandCold 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 OR union iterator, tombstone path, immutable IN/PREFIX_IN lookup, partial-AND compilation, fallback semantics, iterator ownership, and observability changes. The subset compilation remains fail-open/safe because residual expressions still evaluate the full predicate; OR remains all-or-nothing. Child iterators are closed on all owning paths, prefix/disjunct state is bounded by filter input, and the new immutable lookup is race-safe. Owning-package tests, focused race tests (10x), and a 10,000-case PREFIX_IN differential check all passed. No blocking issues found.

@gouhongshen gouhongshen 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.

Codex automated review

No blocking or non-blocking correctness findings in b7fa79d relative to origin/main. Focused Go tests could not run because this checkout lacks the required MatrixOne CGo library (cgo/libmo.dylib).

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