Repository navigation
[improvement](be) Skip dictionary entry bodies of non-matching terms in SNII term expansion - #69600
Merged
airborne12 merged 2 commits intoOct 26, 2026
Merged
Conversation
…in SNII term expansion ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: MATCH_REGEXP, wildcard, suffix and contains patterns without a literal prefix make SniiIndexSource::expand_terms enumerate the whole dictionary of every segment. LogicalIndexReader::visit_prefix_terms always passed an empty accept_key to DictBlockReader::visit_prefix_range, so the body of every dictionary entry was decoded (decode_dict_entry_rest) and the caller then rejected almost all of them with the required-text test and the pattern match. DictBlockReader already supports a key-only accept_key that skips the body of a rejected term. visit_prefix_terms now takes an optional accept_key, and expand_terms passes its term test (required text, then pattern match) as that predicate. Terms are tested exactly as before, only earlier, and the visitor still sees only accepted terms, so max_expansions and early stop count the same terms. Other callers keep the default (accept all). Measured on 99M AgentLogs rows (722 segments, tenant and status filters, token regexp 'transi.*', hot median): SNII 6028 ms -> 945 ms, V3 unchanged at about 590 ms. Result counts are identical to V3 for regexp, leading-wildcard regexp and phrase-prefix queries. Patterns with a literal prefix (for example '^transi.*') seek into the dictionary and are unchanged: SNII and V3 are level at about 100 ms before and after. Gdb sampling showed 58% of the time in decode_dict_entry_rest before and about 5% after. ### Release note Faster MATCH_REGEXP / wildcard term expansion on SNII inverted indexes. ### Check List (For Author) - Test: Unit Test (SniiRegexpQueryTest.AcceptKeySkipsBodiesOfRejectedTerms, SniiRegexpQueryTest.UnanchoredRegexpDecodesOnlyMatchedBodies; SniiB2T07Dict* and SniiRegexpQueryTest* pass on branch-4.2). Manual test (count equality against V3 on 99M rows, before/after timing). - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
Member
Author
|
/review |
airborne12
marked this pull request as ready for review
October 26, 2026 03:10
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Static review of PR #69600 at a22e758 found no actionable issue in the four changed files or their related dictionary/query/posting paths. The existing required-text and pattern checks now run before body decoding, while accepted terms retain their ordering, metadata and expansion limits. No existing inline findings require carrying forward; no additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and proof: the change reuses the dictionary's existing key predicate. The two new tests check accept-all/selective/reject-all/early-stop decoding counts and an unanchored regexp's expanded term and document results. Their assertions support the intended work reduction by inspection.
- Scope and clarity: focused optional-argument forwarding, relocation of the existing predicate, and two related tests. No unrelated production change.
- Concurrency and locks: callbacks remain synchronous; matcher state is query-local, shared reader data remains immutable, and no thread, lock or lock-order change is introduced.
- Lifecycle and static initialization: captured references and term views remain valid through matching; resident ownership or a shared block pin covers each scan. Accepted entries own their strings/payloads. No new ownership cycle or static initialization dependency.
- Configuration: no configuration item or dynamic-update behavior changed.
- Compatibility: the internal reader argument has an accept-all default for existing callers. No persisted format or external protocol change affecting rolling upgrades.
- Thrift/protobuf: no definitions or field/enum identifiers changed.
- Parallel paths: scored/unscored expansion and phrase-edge consumers use the updated source. CLucene retains equivalent matching logic; prefix collection and compaction probes retain their default predicate. Full dictionary and range scans remain appropriate for their callers.
- Special conditions: required-text rejection still precedes pattern matching. Existing key skipping preserves front coding and entry boundaries; prefix exhaustion and accepted-hit stopping remain separate. The new comments explain why matching moves earlier.
- Test coverage: inspected new tests and existing pattern/source/T07/phrase tests covering the five pattern kinds, limits, no-match/invalid patterns, namespace bypass, mixed entry layouts, anchors and resident/on-demand enumeration. Tests were not executed. A single combined selective-predicate/multiple-on-demand-block/cap test is absent; the component tests and static control-flow trace cover those transitions separately.
- Expected results: the fixture has 11 terms; order/ordinal are the two ord-prefixed terms, and only ordinal contains rdin and has document IDs 0 through 8999. New expected lists/counts agree with the fixture. No generated result file changed.
- Observability: existing decoder counters directly expose the intended work reduction; existing dictionary errors and query instrumentation remain. No new logging or metric is necessary for this local change.
- Transactions and persistence: no transaction, EditLog, visible-version, replay or failover behavior changed.
- Data writes and crash behavior: no write path, atomicity boundary or crash-cleanup ownership changed. Existing visibility and delete-bitmap filtering remain outside this term predicate.
- FE/BE variables: no new variable or transport path.
- Performance and memory: keys still enumerate as needed, with the same matcher sequence up to the accepted-term cap. Rejected bodies avoid payload materialization and hit copies; accepted entries retain existing caching. No significant new retained allocation or memory-accounting scope.
- Other correctness/error checks: block CRC validation still precedes scanning, requested-body decoding and Status propagation remain intact, and SQL null/bitmap handling is unchanged. No further substantiated issue remained after the risk recheck.
Review converged in 2 rounds with no unresolved correctness candidates. All conclusions are from static inspection: no builds, tests, format/static-analysis checks, runtime validation or benchmark reproduction were performed. The PR's reported timing improvements were not independently measured.
Review converged after 2 round(s).
Contributor
|
run buildall |
Contributor
TPC-H: Total hot run time: 28959 ms |
Contributor
TPC-DS: Total hot run time: 151173 ms |
Contributor
ClickBench: Total hot run time: 24.27 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
csun5285
approved these changes
Oct 26, 2026
Contributor
|
PR approved by anyone and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 26, 2026
…in SNII term expansion (#69600) ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: MATCH_REGEXP, wildcard, suffix and contains patterns without a literal prefix make SniiIndexSource::expand_terms enumerate the whole dictionary of every segment. LogicalIndexReader::visit_prefix_terms always passed an empty accept_key to DictBlockReader::visit_prefix_range, so the body of every dictionary entry was decoded (decode_dict_entry_rest) and the caller then rejected almost all of them with the required-text test and the pattern match. DictBlockReader already supports a key-only accept_key that skips the body of a rejected term. visit_prefix_terms now takes an optional accept_key, and expand_terms passes its term test (required text, then pattern match) as that predicate. Terms are tested exactly as before, only earlier, and the visitor still sees only accepted terms, so max_expansions and early stop count the same terms. Other callers keep the default (accept all). Measured on 99M AgentLogs rows (722 segments, tenant and status filters, token regexp 'transi.*', hot median): SNII 6028 ms -> 945 ms, V3 unchanged at about 590 ms. Result counts are identical to V3 for regexp, leading-wildcard regexp and phrase-prefix queries. Patterns with a literal prefix (for example '^transi.*') seek into the dictionary and are unchanged: SNII and V3 are level at about 100 ms before and after. Gdb sampling showed 58% of the time in decode_dict_entry_rest before and about 5% after.
yiguolei
pushed a commit
that referenced
this pull request
Oct 27, 2026
…ching terms in SNII term expansion #69600 (#69637) ### What problem does this PR solve? Issue Number: None Related PR: #69600 Problem Summary: Backport of #69600 to branch-4.2. MATCH_REGEXP, wildcard, suffix and contains patterns without a literal prefix make `SniiIndexSource::expand_terms` enumerate the whole dictionary of every segment. `LogicalIndexReader::visit_prefix_terms` passed an empty `accept_key`, so the body of every dictionary entry was decoded before the caller rejected almost all of them. `visit_prefix_terms` now takes an optional `accept_key`, and `expand_terms` passes its term test as that predicate, so a rejected term is skipped before its body is decoded. Terms are tested exactly as before, only earlier. Measured on master's parent line (99M AgentLogs rows, 722 segments): token regexp `transi.*` on SNII 6028 ms -> 945 ms, V3 about 590 ms. ### Branch adaptations None. `git cherry-pick -x 6979f7f` applied cleanly to the current branch-4.2 and touches the same four files as the original: `logical_index_reader.{h,cpp}`, `snii_index_source.cpp` and `snii_query_test.cpp`. ### Validation - The same change (before the one-line clang-format fix of the declaration in `logical_index_reader.h`) was built and run on an earlier branch-4.2 base (`ecbc5feab9`): `SniiB2T07Dict*` and `SniiRegexpQueryTest*`, 31 tests passed, including the two new tests. - On master, `Snii*` passed (1373 tests) and the master CI passed all checks. - Not rebuilt locally on the current branch-4.2 tip; this PR's BE UT run covers that. ### Release note Faster MATCH_REGEXP / wildcard term expansion on SNII inverted indexes. ### Check List (For Author) - Test: Unit Test - Behavior changed: No - Does this need documentation: No 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: martin.foo838 <martin.foo838@gmail.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
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: None
Problem Summary: MATCH_REGEXP, wildcard, suffix and contains patterns without a literal
prefix make SniiIndexSource::expand_terms enumerate the whole dictionary of every segment.
LogicalIndexReader::visit_prefix_terms always passed an empty accept_key to
DictBlockReader::visit_prefix_range, so the body of every dictionary entry was decoded
(decode_dict_entry_rest) and the caller then rejected almost all of them with the
required-text test and the pattern match. DictBlockReader already supports a key-only
accept_key that skips the body of a rejected term.
visit_prefix_terms now takes an optional accept_key, and expand_terms passes its term test
(required text, then pattern match) as that predicate. Terms are tested exactly as before, only
earlier, and the visitor still sees only accepted terms, so max_expansions and early stop count
the same terms. Other callers keep the default (accept all).
Measured on 99M AgentLogs rows (722 segments, tenant and status filters, token regexp
'transi.', hot median): SNII 6028 ms -> 945 ms, V3 unchanged at about 590 ms. Result counts
are identical to V3 for regexp, leading-wildcard regexp and phrase-prefix queries. Patterns with
a literal prefix (for example '^transi.') seek into the dictionary and are unchanged: SNII and
V3 are level at about 100 ms before and after. Gdb sampling showed 58% of the time in
decode_dict_entry_rest before and about 5% after.
Release note
Faster MATCH_REGEXP / wildcard term expansion on SNII inverted indexes.
Check List (For Author)
🤖 Generated with Claude Code