[fix](be) Match SNII wildcards by UTF-8 code point - #66861
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review status: converged after two rounds; requesting changes for one data-correctness issue.
The UTF-8 matcher is correct for valid inputs, and the added unit and integration coverage demonstrates SNII/V3 ? parity. The inline finding is a separate compatibility regression: legal raw keyword values containing malformed UTF-8 are silently omitted by nontrivial wildcards.
Critical checkpoints:
- Goal and proof: code-point matching for valid UTF-8 is implemented and exercised by matcher, logical-index, and ordered SNII/V3 SEARCH tests; malformed stored-term compatibility is not preserved.
- Scope and clarity: the production diff is focused on wildcard matching and invalid-pattern handling; no unrelated production change was found.
- Concurrency: the matcher is request-local; existing query-cache and single-flight keys include the raw normalized pattern, query type, and expansion cap, with no new lock or shared-state defect.
- Lifecycle and static initialization: the pattern view remains live for synchronous enumeration, scratch storage is request-scoped, and no cross-TU static dependency is added.
- Configuration: no configuration item is added or changed.
- Compatibility: there is no storage-format or protocol change, but existing and newly written raw keyword indexes can contain legal non-UTF-8 bytes; the inline issue changes their results and can publish/cache an incomplete bitmap.
- Parallel paths: standard and Lucene SEARCH parsing, analyzed and non-analyzed PREFIX/WILDCARD routing, SNII and V3 execution, raw and escaped dictionaries, cache hit/miss, null masking, and expansion caps were checked.
- Conditions and errors: valid UTF-8 boundary validation makes width jumps safe; invalid patterns fail before enumeration, but invalid stored terms are incorrectly converted to successful non-matches as described inline.
- Test coverage: valid 2-, 3-, and 4-byte code points, literals, star/question combinations, malformed matcher inputs, allocation reuse, union ordering, nulls, and end-to-end SNII/V3 results are covered. Add a writer-to-SEARCH regression for the accepted issue.
- Test results: the checked-in ordered output matches the suite, and the PR reports 22/22 targeted ASAN unit tests, the named regression, and a full BE build. This review did not run builds or tests because the review prompt forbids them.
- Observability: existing timers, cache counters, status propagation, and VLOGs are adequate for valid execution; the accepted silent omission is precisely the missing failure signal.
- Transactions and persistence: no transaction, EditLog, visibility-version, MoW, or metadata-persistence behavior is changed.
- Data writes and crash behavior: the production writer and on-disk bytes are unchanged; no new write atomicity or cleanup path is introduced.
- FE-BE variables and compatibility carriers: no query enum, thrift field, function symbol, or transmitted variable is added.
- Performance and other checks: prefix seeking and two reusable DP rows remain intact; validation is linear per visited term and introduces no per-term heap allocation. No second correctness, lifecycle, or performance issue survived verification.
User focus: no additional focus was provided. All candidates were independently verified, deduplicated, accepted, or dismissed; Round 2 returned NO_NEW_VALUABLE_FINDINGS on both full-review tracks and the separate risk track.
| : pattern_(pattern), pattern_valid_(is_valid_utf8(pattern)) {} | ||
|
|
||
| bool operator()(std::string_view text) { | ||
| if (!pattern_valid_ || !is_valid_utf8(text)) { |
There was a problem hiding this comment.
[P1] Preserve wildcard matches for raw keyword terms
SNII's non-analyzed writer persists raw VARCHAR bytes without UTF-8 validation, so a legal value such as cast(unhex('61FF') as string) creates a term containing bytes 0x61 0xff that reaches this guard. Returning false makes body:a* and body:** silently omit that row, even though the old byte matcher returned it and body:* still returns it through MATCH_ALL_DOCS. This changes results for existing indexes and makes equivalent wildcard forms disagree. Please preserve compatible handling where possible, or propagate a non-OK/bypass result for malformed dictionary terms instead of treating them as clean non-matches; add an end-to-end SNII keyword case covering these three patterns.
There was a problem hiding this comment.
Confirmed independently with a real writer-to-reader reproduction. Before the fix, a raw keyword term 61FF produced a* -> {valid-only}, ** -> {valid-only}, and * -> {valid-only} at the SNII query API, omitting the malformed term. Commit bd9a2b5 keeps query patterns strict UTF-8, uses code-point transitions for valid dictionary terms, and falls back to the legacy byte transitions only for malformed stored terms. The same corpus now returns the raw row for a*, **, and * while retaining the UTF-8 ? behavior. Added both BE writer/reader coverage and a real SNII regression case using CAST(UNHEX(61FF) AS STRING); 23 affected ASAN UTs and the generated-then-normal regression suite pass. This is query-only: no writer or on-disk format change, so existing SNII indexes are fixed by upgrade.
TPC-H: Total hot run time: 17392 ms |
TPC-DS: Total hot run time: 81270 ms |
### What problem does this PR solve? Issue Number: None Related PR: apache#66052 Problem Summary: SNII evaluated wildcard question marks byte by byte, so a pattern such as a?b could not match a猫b although the V3 inverted index treats the question mark as one Unicode character. Switching all terms to strict UTF-8 initially caused a compatibility regression: keyword indexes can contain arbitrary VARCHAR bytes, and already-written malformed terms such as 61FF stopped matching a*, **, and *. The matcher now advances valid terms by UTF-8 code point while falling back to the legacy byte semantics for malformed stored terms. Query patterns remain strict UTF-8 and invalid patterns return INVALID_ARGUMENT. The existing reusable DP scratch remains bounded. ### Release note SNII wildcard question marks now match one UTF-8 code point for valid text while preserving wildcard matches for existing raw keyword bytes. ### Check List (For Author) - Test: Unit Test and Regression test - ASAN BE unit tests: SniiWildcardQueryTest.* and SniiPatternQuery.* (23 passed) - Regression test: test_storage_format_snii_utf8_wildcard (generated output and normal run passed) - Full BE build with -j 192 - Behavior changed: Yes. SNII wildcard matching is code-point based for valid UTF-8 and retains legacy byte matching for malformed stored terms; the storage format and write path are unchanged. - Does this need documentation: No
73af19e to
bd9a2b5
Compare
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: All Codex review accounts are usage-limited; earliest retry is 2026-08-20T03:35:00Z. Please trigger /review again after that time. |
2 participants
What problem does this PR solve?
Issue Number: None
Related PR: #66052
Problem Summary:
SNII evaluated wildcard question marks byte by byte, so
a?bcould not matcha猫b, while V3 converts?to a RE2 dot and matches one Unicode code point. The SNII matcher now validates UTF-8 and advances its existing reusable dynamic-programming rows only at code-point boundaries. This preserves the bounded two-buffer scratch design, makes invalid patterns returnINVALID_ARGUMENT, and aligns SNII query results with V3 without changing the writer or stored bytes.Release note
SNII wildcard question marks now match one UTF-8 code point, consistent with V3.
Check List (For Author)
?now consumes exactly one UTF-8 code point. This is query-only; the storage format and write path are unchanged, so existing SNII indexes are upgrade-compatible.Tests
SniiWildcardQueryTest.*:SniiPatternQuery.*(22/22 passed)inverted_index_p0/storage_format/test_storage_format_snii_utf8_wildcard./build.sh --be -j 192Reviewer Checklist
type(scope): [Issue #xxx] summary