Skip to content

branch-4.2: [feat](inverted-index) Support configurable ngram size difference #67917 - #68774

Merged
airborne12 merged 1 commit into
apache:branch-4.2from
airborne12:pick-67917-branch-4.2
Oct 15, 2026
Merged

branch-4.2: [feat](inverted-index) Support configurable ngram size difference #67917#68774
airborne12 merged 1 commit into
apache:branch-4.2from
airborne12:pick-67917-branch-4.2

Conversation

@airborne12

@airborne12 airborne12 commented Oct 8, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: close #67916

Related PR: #67917

Problem Summary:

Cherry-pick of #67917 (commit 6a79f95, a squash merge) to branch-4.2. Custom ngram tokenizers can now set max_ngram_diff (0 to 255, default 1) instead of the hard-coded difference of 1. FE and BE validate the same range, and newly created policies cap absolute gram sizes at 1024. A persisted max_ngram_diff marker keeps marker-less policies from before the change valid. max_ngram_diff does not change the analyzer identity.

branch-4.2 also has the gram mode tokenizer, which the source change does not know about. Adaptations:

  • max_ngram_diff and the size limits apply to the legacy (no mode) path only.
  • max_ngram_diff together with mode is rejected.
  • The compatibility marker is added only to new legacy ngram policies.

Release note

Allow custom ngram tokenizers to configure the maximum difference between max_gram and min_gram with max_ngram_diff values from 0 through 255.

Check List (For Author)

  • Test

    • Regression test (the case is included; it was not run locally and is left to CI)
    • Unit Test
    • Manual test

  • Behavior changed:

    • No.
    • Yes. New custom ngram tokenizers can opt into a wider gram-size range (difference up to 255, absolute size up to 1024).

  • Does this need documentation?

    • No.
    • Yes. Same documentation as the source PR.

Validation

  • FE: run-fe-ut.sh --run PolicyValidatorTests,AnalyzerIdentityBuilderTest: 117 tests, 0 failures; Checkstyle passed.
  • BE: NGramTokenizerTest.*: 21/21 passed (includes the 7 new tests), ASAN.
  • clang-format 16 and Checkstyle merge-tree preflight: PASS.
  • Local-environment notes (not part of this PR): origin/branch-4.2 currently declares thrift field id 229 twice in PaloInternalService.thrift (enable_paimon_rust_reader, enable_prune_nested_column), so a clean branch-4.2 build fails; I bumped one id locally only to run the tests. For the BE UT I also stubbed the unrelated paimon_rust sources locally and linked with --unresolved-symbols=ignore-all. None of this is in the commit.

Hunk audit (against #67917)

File Hunk Result
ngram_tokenizer.cpp @@ NGramTokenizer::init buffer size Ported(verbatim)
ngram_tokenizer.h trailing newline Ported (4.2 keeps compile_check_end.h)
ngram_tokenizer_factory.cpp initialize limits and diff check, newline Ported(verbatim); the 4.2 gram branch returns earlier, so these apply to the legacy path only
ngram_tokenizer_factory.h MAX_NGRAM_DIFF/MAX_NGRAM_SIZE constants Ported(verbatim)
ngram_tokenizer_test.cpp 7 new tests Ported; 4.2's RejectsMalformedUtf8AndCanBeReset kept
NGramTokenizerValidator.java ALLOWED_PROPS, constants, isValidPolicy, size/diff checks Adapted: 4.2 mode props kept, max_ngram_diff rejected with mode
IndexPolicy.java @@ isInvalid Ported (uses isValidPolicy; same behavior as 4.2's try/catch)
IndexPolicyMgr.java @@ validateReferencedComponentsUsableLocked N-A: 4.2 already has it (tokenizer isInvalid check present)
IndexPolicyMgr.java @@ createIndexPolicy marker Adapted: only for non-mode ngram
IndexPolicyMgr.java @@ validateReferencedPolicy isInvalid check N-A: already present in 4.2
IndexPolicyMgr.java @@ warnIfUnsupported message Ported
AnalyzerIdentityBuilder.java PROP_MAX_NGRAM_DIFF, isInvalid identity, remove(max_ngram_diff) N-A: all three already present in 4.2 (verified with git grep on origin/branch-4.2)
AnalyzerIdentityBuilderTest.java 3 tests Ported; 4.2's testNgramDefaultBoundsDoNotChangeAnalyzerIdentity kept
PolicyValidatorTests.java difference/size/marker tests Ported; 4.2 gram-mode tests kept; 1 new test for max_ngram_diff + mode
regression-test .groovy/.out new files Ported(verbatim)

🤖 Generated with Claude Code

@airborne12
airborne12 requested a review from yiguolei as a code owner October 8, 2026 06:53
@hello-stephen

hello-stephen commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@airborne12

airborne12 commented Oct 12, 2026

Copy link
Copy Markdown
Member Author

run buildall

@hello-stephen

hello-stephen commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 51.28% (20/39) 🎉
Increment coverage report
Complete coverage report

csun5285 added a commit to csun5285/doris that referenced this pull request Oct 12, 2026
@csun5285 @claude
[Improvement](scan) Remove the common expr pushdown session variables
9963175 
### What problem does this PR solve?

Backport to branch-4.2 of how master dropped enable_common_expr_pushdown and
enable_common_expr_pushdown_for_inverted_index (apache#62222, apache#64285, apache#64479).

- BE: RuntimeState::enable_common_expr_pushdown() and
  enable_common_expr_pushdown_for_inverted_index() are gone. Tables not merged on
  read always push slot-based conjuncts into SegmentIterator, the OlapScanner check
  that every such conjunct was pushed runs whenever segment LIMIT can be pushed, and
  SegmentIterator no longer returns early from
  _check_all_conditions_passed_inverted_index_for_column.
- BE: validate_residual_scan_conjuncts from apache#62222 comes back. A residual SEARCH, a
  residual MATCH with enable_match_without_inverted_index off, and COUNT_ON_INDEX with
  residual scan conjuncts now fail with INVALID_ARGUMENT. The SEARCH and MATCH messages
  also say that tables merged on read do not push expressions on this branch.
- FE: the two session variables are removed (apache#64285). VariableMgr takes
  REMOVED_SESSION_VAR_NAMES from apache#63744, listing only these two names, so SET,
  SET GLOBAL, SET_VAR and SELECT @@ on them still succeed and do nothing.
- FE: fuzzy mode now randomizes enable_segment_limit_pushdown (apache#64479).
- thrift: field 65 is marked deprecated. No field id changes.
- Regression cases follow apache#64479: a setting of either removed variable becomes
  enable_segment_limit_pushdown with the same value; two settings in one SET_VAR or on
  two adjacent SET lines become one, taking the enable_common_expr_pushdown value when
  they differ. 214 files take apache#64479's hunks without its trailing-whitespace cleanup,
  57 files that apache#64479 misses or that differ here are rewritten by the same rules (one
  file gets both), and 12 files keep the removed names exactly as master has them today.

Upgrade BE before FE. An old BE behind a new FE no longer receives
enable_common_expr_pushdown, reads it as false, and keeps conjuncts in the scanner
until it is upgraded.

Verified on this branch (ASAN BE):
- FE and BE build, 0 checkstyle violations, BE changes clean under clang-format 16.
- FE UT SessionVariablesTest, VariableMgrTest and SessionVariableBackendSelectionTest
  (42 cases) pass. The 23 related BE UT suites (183 cases, including the SegmentIterator
  apply-index-expr, short-circuit, expr-zonemap, candidate-pushdown and no-need-read-data
  suites) pass.
- SET, SET GLOBAL, SET_VAR and SELECT @@ on both removed names succeed; SELECT @@ returns
  an empty string, SHOW VARIABLES lists neither, and an unknown name still fails.
- With enable_segment_limit_pushdown set globally to false and then true:
  query_p0/limit (18 suites), test_search_slash_in_term, test_variant_predefine_index_type,
  infer_predicate, and 69 more suites (the ones that failed on residual predicates in this
  PR's earlier CI, the ones that turned the old switch off, the ones that still set the
  removed names, and migrated SNII and gram cases) pass, except
  test_search_score_topn_predicates. That suite fails the same way in both runs, and CI
  mutes it on other branch-4.2 PRs (apache#68213, apache#68774): the FE reports the score() usage
  error before the LIMIT overflow error the case expects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
csun5285 added a commit to csun5285/doris that referenced this pull request Oct 13, 2026
@csun5285 @claude
[Improvement](scan) Remove the common expr pushdown session variables
4cb9d99 
### What problem does this PR solve?

Backport to branch-4.2 of how master dropped enable_common_expr_pushdown and
enable_common_expr_pushdown_for_inverted_index (apache#62222, apache#64285, apache#64479).

- BE: RuntimeState::enable_common_expr_pushdown() and
  enable_common_expr_pushdown_for_inverted_index() are gone. Tables not merged on
  read always push slot-based conjuncts into SegmentIterator, the OlapScanner check
  that every such conjunct was pushed runs whenever segment LIMIT can be pushed, and
  SegmentIterator no longer returns early from
  _check_all_conditions_passed_inverted_index_for_column.
- BE: validate_residual_scan_conjuncts from apache#62222 comes back. A residual SEARCH, a
  residual MATCH with enable_match_without_inverted_index off, and COUNT_ON_INDEX with
  residual scan conjuncts now fail with INVALID_ARGUMENT. The SEARCH and MATCH messages
  also say that tables merged on read do not push expressions on this branch.
- FE: the two session variables are removed (apache#64285). VariableMgr takes
  REMOVED_SESSION_VAR_NAMES from apache#63744, listing only these two names, so SET,
  SET GLOBAL, SET_VAR and SELECT @@ on them still succeed and do nothing.
- FE: fuzzy mode now randomizes enable_segment_limit_pushdown (apache#64479).
- thrift: field 65 is marked deprecated. No field id changes.
- Regression cases follow apache#64479: a setting of either removed variable becomes
  enable_segment_limit_pushdown with the same value; two settings in one SET_VAR or on
  two adjacent SET lines become one, taking the enable_common_expr_pushdown value when
  they differ. 214 files take apache#64479's hunks without its trailing-whitespace cleanup,
  57 files that apache#64479 misses or that differ here are rewritten by the same rules (one
  file gets both), and 12 files keep the removed names exactly as master has them today.

Upgrade BE before FE. An old BE behind a new FE no longer receives
enable_common_expr_pushdown, reads it as false, and keeps conjuncts in the scanner
until it is upgraded.

Verified on this branch (ASAN BE):
- FE and BE build, 0 checkstyle violations, BE changes clean under clang-format 16.
- FE UT SessionVariablesTest, VariableMgrTest and SessionVariableBackendSelectionTest
  (42 cases) pass. The 23 related BE UT suites (183 cases, including the SegmentIterator
  apply-index-expr, short-circuit, expr-zonemap, candidate-pushdown and no-need-read-data
  suites) pass.
- SET, SET GLOBAL, SET_VAR and SELECT @@ on both removed names succeed; SELECT @@ returns
  an empty string, SHOW VARIABLES lists neither, and an unknown name still fails.
- With enable_segment_limit_pushdown set globally to false and then true:
  query_p0/limit (18 suites), test_search_slash_in_term, test_variant_predefine_index_type,
  infer_predicate, and 69 more suites (the ones that failed on residual predicates in this
  PR's earlier CI, the ones that turned the old switch off, the ones that still set the
  removed names, and migrated SNII and gram cases) pass, except
  test_search_score_topn_predicates. That suite fails the same way in both runs, and CI
  mutes it on other branch-4.2 PRs (apache#68213, apache#68774): the FE reports the score() usage
  error before the LIMIT overflow error the case expects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yiguolei

yiguolei commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

skip buildall

@airborne12 @claude
branch-4.2: [feat](inverted-index) Support configurable ngram size di…
561f1a5 
…fference apache#67917

Cherry-picked from apache#67917

(cherry picked from commit 6a79f95)

Conflicts:
- NGramTokenizerValidator.java: branch-4.2 has the gram `mode` tokenizer.
  max_ngram_diff and the size limits apply to the legacy path only, and
  max_ngram_diff combined with `mode` is rejected.
- IndexPolicyMgr.java: the compatibility marker max_ngram_diff=1 is added
  only for new legacy ngram policies (not `mode` policies).
- IndexPolicy.java: isInvalid() uses NGramTokenizerValidator.isValidPolicy.
- AnalyzerIdentityBuilder.java: already contains the same logic on branch-4.2.
- AnalyzerIdentityBuilderTest.java, PolicyValidatorTests.java,
  ngram_tokenizer_test.cpp, ngram_tokenizer.h: keep the branch-4.2 tests and
  add the tests from the source change.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@airborne12
airborne12 force-pushed the pick-67917-branch-4.2 branch from 32da9bb to 561f1a5 Compare October 15, 2026 11:58
@airborne12

airborne12 commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

run buildall

@hello-stephen

hello-stephen commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 20.20% (20/99) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

hello-stephen commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 85.19% (23/27) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 76.01% (34141/44919)
Line Coverage 60.84% (386594/635472)
Region Coverage 57.61% (325817/565510)
Branch Coverage 58.43% (149477/255843)

@airborne12
airborne12 merged commit 7b457d6 into apache:branch-4.2 Oct 15, 2026
27 of 28 checks passed
@airborne12
airborne12 deleted the pick-67917-branch-4.2 branch October 15, 2026 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yiguolei yiguolei yiguolei approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@airborne12 @hello-stephen @yiguolei