From f418b9582533510b8195e5e1b9077a6554f3e059 Mon Sep 17 00:00:00 2001 From: airborne12 Date: Sun, 13 Sep 2026 23:29:31 +0800 Subject: [PATCH 1/6] [feat](inverted-index) Support configurable ngram size difference ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Custom ngram tokenizers hard-code the allowed difference between `max_gram` and `min_gram` to 1. This prevents valid wider ngram ranges while offering no explicit override. This change adds the `max_ngram_diff` tokenizer property, keeps 1 as the backward-compatible default, validates non-negative values in FE and BE, and adds unit and regression coverage for a 1-to-8 tokenizer. ### Release note Allow custom ngram tokenizers to configure the maximum difference between `max_gram` and `min_gram` with `max_ngram_diff`. ### Check List (For Author) - Test - [ ] Regression test (coverage added; execution is pending CI) - [x] Unit Test - [ ] Manual test - Behavior changed: - [ ] No. - [x] Yes. Custom ngram tokenizers can opt into a wider gram-size range. - Does this need documentation? - [ ] No. - [x] Yes. The new tokenizer property should be added to the custom analyzer documentation. Validation: - `./build.sh --be -j48` (ASAN) - `./run-be-ut.sh --run --filter='NGramTokenizerTest.*' -j48` (14 tests passed) - `./run-fe-ut.sh --run org.apache.doris.indexpolicy.PolicyValidatorTests` (20 tests passed) - `build-support/run-clang-tidy.sh --base origin/master --build-dir be/build_ASAN` - C++ format and build-hygiene checks --- .../ngram/ngram_tokenizer_factory.cpp | 12 +++- .../tokenizer/ngram_tokenizer_test.cpp | 25 ++++++- .../indexpolicy/NGramTokenizerValidator.java | 20 +++++- .../indexpolicy/PolicyValidatorTests.java | 34 +++++++++ ...test_ngram_max_diff_custom_analyzer.groovy | 72 +++++++++++++++++++ 5 files changed, 158 insertions(+), 5 deletions(-) create mode 100644 regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy diff --git a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp index c5b6c5a9c733f4..53f2d17201c980 100644 --- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp +++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp @@ -26,12 +26,18 @@ std::unordered_map NGramTokenizerFactory::MATCHERS; void NGramTokenizerFactory::initialize(const Settings& settings) { _min_gram = settings.get_int("min_gram", NGramTokenizer::DEFAULT_MIN_NGRAM_SIZE); _max_gram = settings.get_int("max_gram", NGramTokenizer::DEFAULT_MAX_NGRAM_SIZE); + int32_t max_ngram_diff = settings.get_int("max_ngram_diff", 1); + if (max_ngram_diff < 0) { + throw Exception(ErrorCode::INVALID_ARGUMENT, + "max_ngram_diff must be greater than or equal to 0"); + } int32_t ngram_diff = _max_gram - _min_gram; - if (ngram_diff > 1) { + if (ngram_diff > max_ngram_diff) { throw Exception( ErrorCode::INVALID_ARGUMENT, "The difference between max_gram and min_gram in NGram Tokenizer must be less " - "than or equal to: [ 1 ] but was [" + + "than or equal to: [ " + + std::to_string(max_ngram_diff) + " ] but was [" + std::to_string(ngram_diff) + "]"); } _matcher = parse_token_chars(settings); @@ -80,4 +86,4 @@ CharMatcherPtr NGramTokenizerFactory::parse_token_chars(const Settings& settings return builder.build(); } -} // namespace doris::segment_v2::inverted_index \ No newline at end of file +} // namespace doris::segment_v2::inverted_index diff --git a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp index b9dca64838fe30..0d5bf3317b1af4 100644 --- a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp +++ b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp @@ -87,6 +87,29 @@ TEST(NGramTokenizerTest, InvalidMinMaxDifference) { ASSERT_TRUE(exception_thrown); } +TEST(NGramTokenizerTest, ConfiguredMinMaxDifference) { + NGramTokenizerFactory factory; + std::unordered_map args; + args["min_gram"] = "1"; + args["max_gram"] = "8"; + args["max_ngram_diff"] = "7"; + Settings settings(args); + factory.initialize(settings); + auto tokens = tokenize(factory, "abcd"); + + std::vector expected {"a", "ab", "abc", "abcd", "b", "bc", "bcd", "c", "cd", "d"}; + ASSERT_EQ(tokens, expected); +} + +TEST(NGramTokenizerTest, InvalidConfiguredDifferenceLimit) { + NGramTokenizerFactory factory; + std::unordered_map args; + args["max_ngram_diff"] = "-1"; + Settings settings(args); + + EXPECT_THROW(factory.initialize(settings), Exception); +} + TEST(NGramTokenizerTest, SymbolCharactersHandling) { NGramTokenizerFactory factory; std::unordered_map args; @@ -202,4 +225,4 @@ TEST(NGramTokenizerTest, WhitespaceTokenization) { ASSERT_EQ(tokens, expected); } -} // namespace doris::segment_v2 \ No newline at end of file +} // namespace doris::segment_v2 diff --git a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java index 03c08cbda6c6fb..45fd488998037e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java +++ b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java @@ -28,7 +28,7 @@ public class NGramTokenizerValidator extends BasePolicyValidator { private static final Set ALLOWED_PROPS = ImmutableSet.of( - "type", "min_gram", "max_gram", "token_chars", "custom_token_chars"); + "type", "min_gram", "max_gram", "max_ngram_diff", "token_chars", "custom_token_chars"); private static final Set VALID_TOKEN_CHARS = ImmutableSet.of( "letter", "digit", "whitespace", "punctuation", "symbol", "custom"); @@ -77,6 +77,24 @@ protected void validateSpecific(Map props) throws DdlException { + "cannot be smaller than min_gram [" + minGram + "]"); } + int maxNgramDiff = 1; + if (props.containsKey("max_ngram_diff")) { + try { + maxNgramDiff = Integer.parseInt(props.get("max_ngram_diff")); + if (maxNgramDiff < 0) { + throw new DdlException("max_ngram_diff must be greater than or equal to 0"); + } + } catch (NumberFormatException e) { + throw new DdlException("max_ngram_diff must be a non-negative integer"); + } + } + + int ngramDiff = maxGram - minGram; + if (ngramDiff > maxNgramDiff) { + throw new DdlException("The difference between max_gram and min_gram in NGram Tokenizer must be less " + + "than or equal to: [ " + maxNgramDiff + " ] but was [" + ngramDiff + "]"); + } + if (props.containsKey("token_chars")) { String tokenChars = props.get("token_chars"); if (!tokenChars.isEmpty()) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java index 4418d6270a4952..7854f78d906c6c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java +++ b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java @@ -130,9 +130,43 @@ public void testNGramValidator_ValidProperties() throws Exception { Map props = new HashMap<>(); props.put("min_gram", "3"); props.put("max_gram", "5"); + props.put("max_ngram_diff", "2"); validator.validate(props); // Should not throw } + @Test + public void testNGramValidator_DefaultDifferenceLimit() { + NGramTokenizerValidator validator = new NGramTokenizerValidator(); + Map props = new HashMap<>(); + props.put("min_gram", "1"); + props.put("max_gram", "8"); + + Exception exception = Assertions.assertThrows(DdlException.class, + () -> validator.validate(props)); + Assertions.assertTrue(exception.getMessage().contains("less than or equal to: [ 1 ]")); + } + + @Test + public void testNGramValidator_ConfiguredDifferenceLimit() throws Exception { + NGramTokenizerValidator validator = new NGramTokenizerValidator(); + Map props = new HashMap<>(); + props.put("min_gram", "1"); + props.put("max_gram", "8"); + props.put("max_ngram_diff", "7"); + validator.validate(props); // Should not throw + } + + @Test + public void testNGramValidator_InvalidDifferenceLimit() { + NGramTokenizerValidator validator = new NGramTokenizerValidator(); + Map props = new HashMap<>(); + props.put("max_ngram_diff", "-1"); + + Exception exception = Assertions.assertThrows(DdlException.class, + () -> validator.validate(props)); + Assertions.assertTrue(exception.getMessage().contains("greater than or equal to 0")); + } + // StandardTokenizerValidator Tests @Test public void testStandardTokenizerValidator_ValidProperties() throws Exception { diff --git a/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy b/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy new file mode 100644 index 00000000000000..50a1aa30f51a47 --- /dev/null +++ b/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy @@ -0,0 +1,72 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_ngram_max_diff_custom_analyzer", "p0") { + def defaultLimitTokenizer = "test_ngram_default_limit_tokenizer" + def ngramTokenizer = "test_ngram_1_8_tokenizer" + def ngramAnalyzer = "test_ngram_1_8_analyzer" + + try_sql "DROP INVERTED INDEX ANALYZER IF EXISTS ${ngramAnalyzer}" + try_sql "DROP INVERTED INDEX TOKENIZER IF EXISTS ${defaultLimitTokenizer}" + try_sql "DROP INVERTED INDEX TOKENIZER IF EXISTS ${ngramTokenizer}" + + test { + sql """ + CREATE INVERTED INDEX TOKENIZER ${defaultLimitTokenizer} + PROPERTIES ( + "type" = "ngram", + "min_gram" = "1", + "max_gram" = "8" + ) + """ + exception "less than or equal to: [ 1 ]" + } + + sql """ + CREATE INVERTED INDEX TOKENIZER IF NOT EXISTS ${ngramTokenizer} + PROPERTIES ( + "type" = "ngram", + "min_gram" = "1", + "max_gram" = "8", + "max_ngram_diff" = "7" + ) + """ + sql """ + CREATE INVERTED INDEX ANALYZER IF NOT EXISTS ${ngramAnalyzer} + PROPERTIES ("tokenizer" = "${ngramTokenizer}") + """ + + int maxRetry = 30 + Exception lastException = null + for (int i = 0; i < maxRetry; i++) { + try { + sql """SELECT TOKENIZE('probe', '"analyzer"="${ngramAnalyzer}"')""" + lastException = null + break + } catch (Exception e) { + lastException = e + sleep(1000) + } + } + assertTrue(lastException == null, + "Analyzer ${ngramAnalyzer} was not ready: ${lastException?.message}") + + def ngramTokens = sql """SELECT TOKENIZE('abcdefgh', '"analyzer"="${ngramAnalyzer}"')""" + def ngramTokenString = ngramTokens[0][0].toString() + assertTrue(ngramTokenString.contains('"token": "abcdefgh"')) + assertTrue(ngramTokenString.contains('"token": "bcdefgh"')) +} From e3f68c19ec66a9740e6435fc998879da10227a6d Mon Sep 17 00:00:00 2001 From: airborne12 Date: Mon, 14 Sep 2026 00:23:21 +0800 Subject: [PATCH 2/6] [test](inverted-index) Verify complete ngram output ### What problem does this PR solve? Problem Summary: The configured ngram range test covered only a short input, and the regression assertion sampled two tokens without detecting missing, duplicate, or reordered output. Compare the complete deterministic 36-token sequence for sizes 1 through 8 in both BE unit and regression coverage. ### Release note None ### Check List (For Author) - Test - [x] Unit Test - [ ] Regression test - Behavior changed: - [x] No. - Does this need documentation? - [x] No. --- .../inverted/tokenizer/ngram_tokenizer_test.cpp | 8 ++++++-- .../test_ngram_max_diff_custom_analyzer.groovy | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp index 0d5bf3317b1af4..a1fcd7afb94a5d 100644 --- a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp +++ b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp @@ -95,9 +95,13 @@ TEST(NGramTokenizerTest, ConfiguredMinMaxDifference) { args["max_ngram_diff"] = "7"; Settings settings(args); factory.initialize(settings); - auto tokens = tokenize(factory, "abcd"); + auto tokens = tokenize(factory, "abcdefgh"); - std::vector expected {"a", "ab", "abc", "abcd", "b", "bc", "bcd", "c", "cd", "d"}; + std::vector expected { + "a", "ab", "abc", "abcd", "abcde", "abcdef", "abcdefg", "abcdefgh", "b", + "bc", "bcd", "bcde", "bcdef", "bcdefg", "bcdefgh", "c", "cd", "cde", + "cdef", "cdefg", "cdefgh", "d", "de", "def", "defg", "defgh", "e", + "ef", "efg", "efgh", "f", "fg", "fgh", "g", "gh", "h"}; ASSERT_EQ(tokens, expected); } diff --git a/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy b/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy index 50a1aa30f51a47..18d5610112a8aa 100644 --- a/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy +++ b/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy @@ -66,7 +66,16 @@ suite("test_ngram_max_diff_custom_analyzer", "p0") { "Analyzer ${ngramAnalyzer} was not ready: ${lastException?.message}") def ngramTokens = sql """SELECT TOKENIZE('abcdefgh', '"analyzer"="${ngramAnalyzer}"')""" - def ngramTokenString = ngramTokens[0][0].toString() - assertTrue(ngramTokenString.contains('"token": "abcdefgh"')) - assertTrue(ngramTokenString.contains('"token": "bcdefgh"')) + def actualTokens = parseJson(ngramTokens[0][0].toString()).collect { it.token } + def expectedTokens = [ + "a", "ab", "abc", "abcd", "abcde", "abcdef", "abcdefg", "abcdefgh", + "b", "bc", "bcd", "bcde", "bcdef", "bcdefg", "bcdefgh", + "c", "cd", "cde", "cdef", "cdefg", "cdefgh", + "d", "de", "def", "defg", "defgh", + "e", "ef", "efg", "efgh", + "f", "fg", "fgh", + "g", "gh", + "h" + ] + assertEquals(expectedTokens, actualTokens) } From 89137eab9b7acc1b55534a6ea5dec8fc95304980 Mon Sep 17 00:00:00 2001 From: airborne12 Date: Mon, 14 Sep 2026 01:58:20 +0800 Subject: [PATCH 3/6] [fix](inverted-index) Keep ngram policy validation consistent Issue Number: close #67916 Related PR: #67917 Problem Summary: The max_ngram_diff creation limit incorrectly changed analyzer identity, allowing equivalent custom analyzers to bypass duplicate-index detection. FE also accepted non-ASCII digits that the BE integer parser rejects. Exclude max_ngram_diff from ngram tokenizer identity and require its value to use ASCII integer syntax. Release note: None Validation: - ./run-fe-ut.sh --run org.apache.doris.analysis.invertedindex.AnalyzerIdentityBuilderTest,org.apache.doris.indexpolicy.PolicyValidatorTests (27 tests passed) - FE Checkstyle passed as part of the targeted test run Behavior changed: Equivalent ngram analyzers now share an identity regardless of max_ngram_diff, and FE rejects non-ASCII max_ngram_diff values. Documentation impact: None --- .../AnalyzerIdentityBuilder.java | 7 +++ .../indexpolicy/NGramTokenizerValidator.java | 6 ++- .../AnalyzerIdentityBuilderTest.java | 47 +++++++++++++++++++ .../indexpolicy/PolicyValidatorTests.java | 11 +++++ 4 files changed, 70 insertions(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java index 1e640fd7d22c86..c70f3a42c6ab9b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java @@ -28,6 +28,8 @@ import java.util.TreeMap; public final class AnalyzerIdentityBuilder { + private static final String PROP_MAX_NGRAM_DIFF = "max_ngram_diff"; + private AnalyzerIdentityBuilder() { } @@ -177,6 +179,11 @@ private static String resolveComponentIdentity(String name, IndexPolicyTypeEnum // Build identity from sorted properties TreeMap sortedProps = new TreeMap<>(props); + if (expectedType == IndexPolicyTypeEnum.TOKENIZER + && "ngram".equals(sortedProps.get(IndexPolicy.PROP_TYPE))) { + // This setting only limits policy creation; it does not change emitted tokens. + sortedProps.remove(PROP_MAX_NGRAM_DIFF); + } return sortedProps.toString(); } catch (RuntimeException e) { return name; diff --git a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java index 45fd488998037e..754cc4aac2dc8f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java +++ b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java @@ -79,8 +79,12 @@ protected void validateSpecific(Map props) throws DdlException { int maxNgramDiff = 1; if (props.containsKey("max_ngram_diff")) { + String value = props.get("max_ngram_diff"); + if (!value.matches("-?[0-9]+")) { + throw new DdlException("max_ngram_diff must be a non-negative integer"); + } try { - maxNgramDiff = Integer.parseInt(props.get("max_ngram_diff")); + maxNgramDiff = Integer.parseInt(value); if (maxNgramDiff < 0) { throw new DdlException("max_ngram_diff must be greater than or equal to 0"); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java index e264b9831bb751..0910e3a71a1f05 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java @@ -17,10 +17,15 @@ package org.apache.doris.analysis.invertedindex; +import org.apache.doris.catalog.Env; import org.apache.doris.indexpolicy.IndexPolicy; +import org.apache.doris.indexpolicy.IndexPolicyMgr; +import org.apache.doris.indexpolicy.IndexPolicyTypeEnum; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import java.util.HashMap; import java.util.Iterator; @@ -101,4 +106,46 @@ public void testParserReturnsParserName() { null); Assertions.assertEquals("standard", identity); } + + @Test + public void testNgramValidationLimitDoesNotChangeAnalyzerIdentity() { + IndexPolicyMgr policyMgr = Mockito.mock(IndexPolicyMgr.class); + Env env = Mockito.mock(Env.class); + Mockito.when(env.getIndexPolicyMgr()).thenReturn(policyMgr); + + Map tokenizerProps = new HashMap<>(); + tokenizerProps.put(IndexPolicy.PROP_TYPE, "ngram"); + tokenizerProps.put("min_gram", "1"); + tokenizerProps.put("max_gram", "8"); + tokenizerProps.put("max_ngram_diff", "7"); + IndexPolicy tokenizerWithLimit = new IndexPolicy( + 1, "ngram_with_limit", IndexPolicyTypeEnum.TOKENIZER, tokenizerProps); + + Map equivalentTokenizerProps = new HashMap<>(tokenizerProps); + equivalentTokenizerProps.remove("max_ngram_diff"); + IndexPolicy tokenizerWithoutLimit = new IndexPolicy( + 2, "ngram_without_limit", IndexPolicyTypeEnum.TOKENIZER, equivalentTokenizerProps); + + IndexPolicy analyzerWithLimit = analyzerPolicy(3, "analyzer_with_limit", "ngram_with_limit"); + IndexPolicy analyzerWithoutLimit = analyzerPolicy(4, "analyzer_without_limit", "ngram_without_limit"); + Mockito.when(policyMgr.getPolicyByName("ngram_with_limit")).thenReturn(tokenizerWithLimit); + Mockito.when(policyMgr.getPolicyByName("ngram_without_limit")).thenReturn(tokenizerWithoutLimit); + Mockito.when(policyMgr.getPolicyByName("analyzer_with_limit")).thenReturn(analyzerWithLimit); + Mockito.when(policyMgr.getPolicyByName("analyzer_without_limit")).thenReturn(analyzerWithoutLimit); + + try (MockedStatic mockedEnv = Mockito.mockStatic(Env.class)) { + mockedEnv.when(Env::getCurrentEnv).thenReturn(env); + String identityWithLimit = AnalyzerIdentityBuilder.buildAnalyzerIdentity( + nonEmptyProperties(), "analyzer_with_limit", "", "__default__", "none", null); + String identityWithoutLimit = AnalyzerIdentityBuilder.buildAnalyzerIdentity( + nonEmptyProperties(), "analyzer_without_limit", "", "__default__", "none", null); + Assertions.assertEquals(identityWithoutLimit, identityWithLimit); + } + } + + private IndexPolicy analyzerPolicy(long id, String name, String tokenizer) { + Map properties = new HashMap<>(); + properties.put(IndexPolicy.PROP_TOKENIZER, tokenizer); + return new IndexPolicy(id, name, IndexPolicyTypeEnum.ANALYZER, properties); + } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java index 7854f78d906c6c..ffc63a8e08642a 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java +++ b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java @@ -167,6 +167,17 @@ public void testNGramValidator_InvalidDifferenceLimit() { Assertions.assertTrue(exception.getMessage().contains("greater than or equal to 0")); } + @Test + public void testNGramValidator_RejectsNonAsciiDifferenceLimit() { + NGramTokenizerValidator validator = new NGramTokenizerValidator(); + Map props = new HashMap<>(); + props.put("max_ngram_diff", "٧"); + + Exception exception = Assertions.assertThrows(DdlException.class, + () -> validator.validate(props)); + Assertions.assertTrue(exception.getMessage().contains("non-negative integer")); + } + // StandardTokenizerValidator Tests @Test public void testStandardTokenizerValidator_ValidProperties() throws Exception { From c52bb7f1e59cd37992f4f48ba08cf90e926c1232 Mon Sep 17 00:00:00 2001 From: airborne12 Date: Mon, 14 Sep 2026 03:17:19 +0800 Subject: [PATCH 4/6] [fix](inverted-index) Bound ngram analyzer expansion ### What problem does this PR solve? Issue Number: close #67916 Related PR: #67917 Problem Summary: An arbitrarily large max_ngram_diff could multiply token output without a hard fan-out bound, and the deterministic regression assertion was not stored as a runner-generated golden. Cap max_ngram_diff at 255 consistently in FE and BE, cover the accepted and rejected boundaries, and replace the manual token-list assertion with a named golden query generated by the regression runner. ### Release note The max_ngram_diff tokenizer setting accepts values from 0 through 255. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [x] Manual test - Behavior changed: - [ ] No. - [x] Yes. max_ngram_diff values above 255 are rejected to bound per-position token fan-out. - Does this need documentation? - [ ] No. - [x] Yes. Document the supported max_ngram_diff range. Validation: - ./build.sh --be -j8 (ASAN, Java extensions, build hygiene, and glibc compatibility) - ./build.sh --fe -j8 - NGramTokenizerTest: 16/16 passed - AnalyzerIdentityBuilderTest and PolicyValidatorTests: 29/29 passed - test_ngram_max_diff_custom_analyzer: runner-generated golden and clean comparison passed on an isolated local FE/BE - clang-format 16, clang-tidy, and Checkstyle passed --- .../ngram/ngram_tokenizer_factory.cpp | 5 +++++ .../tokenizer/ngram/ngram_tokenizer_factory.h | 5 ++++- .../tokenizer/ngram_tokenizer_test.cpp | 20 +++++++++++++++++ .../indexpolicy/NGramTokenizerValidator.java | 6 +++++ .../indexpolicy/PolicyValidatorTests.java | 22 +++++++++++++++++++ .../test_ngram_max_diff_custom_analyzer.out | 4 ++++ ...test_ngram_max_diff_custom_analyzer.groovy | 14 +----------- 7 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 regression-test/data/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.out diff --git a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp index 53f2d17201c980..199e0b7c2a9f46 100644 --- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp +++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp @@ -31,6 +31,11 @@ void NGramTokenizerFactory::initialize(const Settings& settings) { throw Exception(ErrorCode::INVALID_ARGUMENT, "max_ngram_diff must be greater than or equal to 0"); } + if (max_ngram_diff > MAX_NGRAM_DIFF) { + throw Exception( + ErrorCode::INVALID_ARGUMENT, + "max_ngram_diff must be less than or equal to " + std::to_string(MAX_NGRAM_DIFF)); + } int32_t ngram_diff = _max_gram - _min_gram; if (ngram_diff > max_ngram_diff) { throw Exception( diff --git a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h index 2ee428e32ff77f..37573237ab4791 100644 --- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h +++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h @@ -26,6 +26,9 @@ namespace doris::segment_v2::inverted_index { class NGramTokenizerFactory : public TokenizerFactory { public: + // A configured range can emit one token per gram size at every input position. + static constexpr int32_t MAX_NGRAM_DIFF = 255; + NGramTokenizerFactory() = default; ~NGramTokenizerFactory() override = default; @@ -65,4 +68,4 @@ class NGramTokenizerFactory : public TokenizerFactory { CharMatcherPtr _matcher; }; -}; // namespace doris::segment_v2::inverted_index \ No newline at end of file +}; // namespace doris::segment_v2::inverted_index diff --git a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp index a1fcd7afb94a5d..efb6c092570cac 100644 --- a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp +++ b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp @@ -114,6 +114,26 @@ TEST(NGramTokenizerTest, InvalidConfiguredDifferenceLimit) { EXPECT_THROW(factory.initialize(settings), Exception); } +TEST(NGramTokenizerTest, ExcessiveConfiguredDifferenceLimit) { + NGramTokenizerFactory factory; + std::unordered_map args; + args["max_ngram_diff"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_DIFF + 1); + Settings settings(args); + + EXPECT_THROW(factory.initialize(settings), Exception); +} + +TEST(NGramTokenizerTest, ConfiguredDifferenceLimitBoundary) { + NGramTokenizerFactory factory; + std::unordered_map args; + args["min_gram"] = "1"; + args["max_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_DIFF + 1); + args["max_ngram_diff"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_DIFF); + Settings settings(args); + + EXPECT_NO_THROW(factory.initialize(settings)); +} + TEST(NGramTokenizerTest, SymbolCharactersHandling) { NGramTokenizerFactory factory; std::unordered_map args; diff --git a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java index 754cc4aac2dc8f..b4c3c62ce44e29 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java +++ b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java @@ -27,6 +27,9 @@ import java.util.Set; public class NGramTokenizerValidator extends BasePolicyValidator { + // A configured range can emit one token per gram size at every input position. + static final int MAX_NGRAM_DIFF = 255; + private static final Set ALLOWED_PROPS = ImmutableSet.of( "type", "min_gram", "max_gram", "max_ngram_diff", "token_chars", "custom_token_chars"); @@ -88,6 +91,9 @@ protected void validateSpecific(Map props) throws DdlException { if (maxNgramDiff < 0) { throw new DdlException("max_ngram_diff must be greater than or equal to 0"); } + if (maxNgramDiff > MAX_NGRAM_DIFF) { + throw new DdlException("max_ngram_diff must be less than or equal to " + MAX_NGRAM_DIFF); + } } catch (NumberFormatException e) { throw new DdlException("max_ngram_diff must be a non-negative integer"); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java index ffc63a8e08642a..54ecc91ed69078 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java +++ b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java @@ -178,6 +178,28 @@ public void testNGramValidator_RejectsNonAsciiDifferenceLimit() { Assertions.assertTrue(exception.getMessage().contains("non-negative integer")); } + @Test + public void testNGramValidator_RejectsExcessiveDifferenceLimit() { + NGramTokenizerValidator validator = new NGramTokenizerValidator(); + Map props = new HashMap<>(); + props.put("max_ngram_diff", Integer.toString(NGramTokenizerValidator.MAX_NGRAM_DIFF + 1)); + + Exception exception = Assertions.assertThrows(DdlException.class, + () -> validator.validate(props)); + Assertions.assertTrue(exception.getMessage().contains("less than or equal to 255")); + } + + @Test + public void testNGramValidator_AcceptsDifferenceLimitBoundary() { + NGramTokenizerValidator validator = new NGramTokenizerValidator(); + Map props = new HashMap<>(); + props.put("min_gram", "1"); + props.put("max_gram", Integer.toString(NGramTokenizerValidator.MAX_NGRAM_DIFF + 1)); + props.put("max_ngram_diff", Integer.toString(NGramTokenizerValidator.MAX_NGRAM_DIFF)); + + Assertions.assertDoesNotThrow(() -> validator.validate(props)); + } + // StandardTokenizerValidator Tests @Test public void testStandardTokenizerValidator_ValidProperties() throws Exception { diff --git a/regression-test/data/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.out b/regression-test/data/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.out new file mode 100644 index 00000000000000..07b08b46c6eff3 --- /dev/null +++ b/regression-test/data/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.out @@ -0,0 +1,4 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !ngram_tokens -- +[{\n "token": "a"\n }, {\n "token": "ab"\n }, {\n "token": "abc"\n }, {\n "token": "abcd"\n }, {\n "token": "abcde"\n }, {\n "token": "abcdef"\n }, {\n "token": "abcdefg"\n }, {\n "token": "abcdefgh"\n }, {\n "token": "b"\n }, {\n "token": "bc"\n }, {\n "token": "bcd"\n }, {\n "token": "bcde"\n }, {\n "token": "bcdef"\n }, {\n "token": "bcdefg"\n }, {\n "token": "bcdefgh"\n }, {\n "token": "c"\n }, {\n "token": "cd"\n }, {\n "token": "cde"\n }, {\n "token": "cdef"\n }, {\n "token": "cdefg"\n }, {\n "token": "cdefgh"\n }, {\n "token": "d"\n }, {\n "token": "de"\n }, {\n "token": "def"\n }, {\n "token": "defg"\n }, {\n "token": "defgh"\n }, {\n "token": "e"\n }, {\n "token": "ef"\n }, {\n "token": "efg"\n }, {\n "token": "efgh"\n }, {\n "token": "f"\n }, {\n "token": "fg"\n }, {\n "token": "fgh"\n }, {\n "token": "g"\n }, {\n "token": "gh"\n }, {\n "token": "h"\n }] + diff --git a/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy b/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy index 18d5610112a8aa..5e9a84e86d9740 100644 --- a/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy +++ b/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy @@ -65,17 +65,5 @@ suite("test_ngram_max_diff_custom_analyzer", "p0") { assertTrue(lastException == null, "Analyzer ${ngramAnalyzer} was not ready: ${lastException?.message}") - def ngramTokens = sql """SELECT TOKENIZE('abcdefgh', '"analyzer"="${ngramAnalyzer}"')""" - def actualTokens = parseJson(ngramTokens[0][0].toString()).collect { it.token } - def expectedTokens = [ - "a", "ab", "abc", "abcd", "abcde", "abcdef", "abcdefg", "abcdefgh", - "b", "bc", "bcd", "bcde", "bcdef", "bcdefg", "bcdefgh", - "c", "cd", "cde", "cdef", "cdefg", "cdefgh", - "d", "de", "def", "defg", "defgh", - "e", "ef", "efg", "efgh", - "f", "fg", "fgh", - "g", "gh", - "h" - ] - assertEquals(expectedTokens, actualTokens) + qt_ngram_tokens """SELECT TOKENIZE('abcdefgh', '"analyzer"="${ngramAnalyzer}"')""" } From 8de9abb95d0c17da0588b2196ccf8ea135477801 Mon Sep 17 00:00:00 2001 From: airborne12 Date: Mon, 14 Sep 2026 04:38:41 +0800 Subject: [PATCH 5/6] [fix](inverted-index) Reject unsafe ngram policies ### What problem does this PR solve? Problem Summary: Large absolute ngram sizes could allocate an excessive tokenizer buffer even when max_ngram_diff was small. Persisted ngram policies that became invalid under current validation could also collide with a valid replacement analyzer identity. Cap custom ngram tokenizer sizes at 1024 in FE and BE, use overflow-safe buffer sizing, reject references to invalid replayed tokenizer policies, and give those policies stable policy-specific identities. ### Release note None ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - Behavior changed: - [x] Yes. Unsafe or currently invalid ngram tokenizer policies are rejected before analyzer construction. - Does this need documentation? - [x] No. --- .../tokenizer/ngram/ngram_tokenizer.cpp | 3 +- .../tokenizer/ngram/ngram_tokenizer.h | 2 +- .../ngram/ngram_tokenizer_factory.cpp | 11 +++++ .../tokenizer/ngram/ngram_tokenizer_factory.h | 2 + .../tokenizer/ngram_tokenizer_test.cpp | 21 ++++++++++ .../AnalyzerIdentityBuilder.java | 3 ++ .../apache/doris/indexpolicy/IndexPolicy.java | 7 +++- .../doris/indexpolicy/IndexPolicyMgr.java | 37 ++++++++++------ .../indexpolicy/NGramTokenizerValidator.java | 14 +++++++ .../AnalyzerIdentityBuilderTest.java | 42 ++++++++++++++++++- .../indexpolicy/PolicyValidatorTests.java | 22 ++++++++++ 11 files changed, 148 insertions(+), 16 deletions(-) diff --git a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.cpp b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.cpp index a0b253720b5a3f..33a39c1f0ae183 100644 --- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.cpp +++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.cpp @@ -103,7 +103,8 @@ void NGramTokenizer::init(int32_t min_gram, int32_t max_gram, bool edges_only) { _min_gram = min_gram; _max_gram = max_gram; _edges_only = edges_only; - _buffer.resize(4 * max_gram + 1024); + const size_t buffer_size = static_cast(max_gram) * 4 + 1024; + _buffer.resize(buffer_size); } void NGramTokenizer::update_last_non_token_char() { diff --git a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.h b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.h index 92267becbd72f3..ffc45bf3de9134 100644 --- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.h +++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.h @@ -75,4 +75,4 @@ class NGramTokenizer : public DorisTokenizer { std::string _utf8_buffer; }; -} // namespace doris::segment_v2::inverted_index \ No newline at end of file +} // namespace doris::segment_v2::inverted_index diff --git a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp index 199e0b7c2a9f46..b3d8bb4c596e2c 100644 --- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp +++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp @@ -26,6 +26,17 @@ std::unordered_map NGramTokenizerFactory::MATCHERS; void NGramTokenizerFactory::initialize(const Settings& settings) { _min_gram = settings.get_int("min_gram", NGramTokenizer::DEFAULT_MIN_NGRAM_SIZE); _max_gram = settings.get_int("max_gram", NGramTokenizer::DEFAULT_MAX_NGRAM_SIZE); + if (_min_gram <= 0 || _max_gram <= 0) { + throw Exception(ErrorCode::INVALID_ARGUMENT, "min_gram and max_gram must be positive"); + } + if (_min_gram > _max_gram) { + throw Exception(ErrorCode::INVALID_ARGUMENT, "min_gram must not be greater than max_gram"); + } + if (_min_gram > MAX_NGRAM_SIZE || _max_gram > MAX_NGRAM_SIZE) { + throw Exception(ErrorCode::INVALID_ARGUMENT, + "min_gram and max_gram must be less than or equal to " + + std::to_string(MAX_NGRAM_SIZE)); + } int32_t max_ngram_diff = settings.get_int("max_ngram_diff", 1); if (max_ngram_diff < 0) { throw Exception(ErrorCode::INVALID_ARGUMENT, diff --git a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h index 37573237ab4791..8b91369b51d4f2 100644 --- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h +++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h @@ -28,6 +28,8 @@ class NGramTokenizerFactory : public TokenizerFactory { public: // A configured range can emit one token per gram size at every input position. static constexpr int32_t MAX_NGRAM_DIFF = 255; + // Bound the per-stream buffer while retaining support for large application-specific grams. + static constexpr int32_t MAX_NGRAM_SIZE = 1024; NGramTokenizerFactory() = default; ~NGramTokenizerFactory() override = default; diff --git a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp index efb6c092570cac..8a11e048d072ca 100644 --- a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp +++ b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp @@ -134,6 +134,27 @@ TEST(NGramTokenizerTest, ConfiguredDifferenceLimitBoundary) { EXPECT_NO_THROW(factory.initialize(settings)); } +TEST(NGramTokenizerTest, AbsoluteSizeBoundary) { + NGramTokenizerFactory factory; + std::unordered_map args; + args["min_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_SIZE); + args["max_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_SIZE); + Settings settings(args); + + EXPECT_NO_THROW(factory.initialize(settings)); + EXPECT_NO_THROW(factory.create()); +} + +TEST(NGramTokenizerTest, ExcessiveAbsoluteSize) { + NGramTokenizerFactory factory; + std::unordered_map args; + args["min_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_SIZE); + args["max_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_SIZE + 1); + Settings settings(args); + + EXPECT_THROW(factory.initialize(settings), Exception); +} + TEST(NGramTokenizerTest, SymbolCharactersHandling) { NGramTokenizerFactory factory; std::unordered_map args; diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java index c70f3a42c6ab9b..310c442c3ce17b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java @@ -171,6 +171,9 @@ private static String resolveComponentIdentity(String name, IndexPolicyTypeEnum if (policy == null || policy.getType() != expectedType) { return name; } + if (policy.isInvalid()) { + return "invalid-policy:" + policy.getId() + ":" + policy.getName(); + } Map props = policy.getProperties(); if (props == null || props.isEmpty()) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicy.java b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicy.java index 31d51a06c9917d..12454b52c7083a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicy.java +++ b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicy.java @@ -129,8 +129,13 @@ public List getShowInfo() { ImmutableSet.of("common_grams"); public boolean isInvalid() { - return type == IndexPolicyTypeEnum.TOKEN_FILTER + boolean hasUnsupportedTokenFilter = type == IndexPolicyTypeEnum.TOKEN_FILTER && properties != null && LEGACY_UNSUPPORTED_TOKEN_FILTER_TYPES.contains(properties.get(PROP_TYPE)); + boolean hasInvalidNgramTokenizer = type == IndexPolicyTypeEnum.TOKENIZER + && properties != null + && "ngram".equals(properties.get(PROP_TYPE)) + && !NGramTokenizerValidator.isValidPolicy(properties); + return hasUnsupportedTokenFilter || hasInvalidNgramTokenizer; } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java index b547c482d81452..5f6ef7869d58fa 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java @@ -110,22 +110,32 @@ public void validateAnalyzerExists(String analyzerName) throws DdlException { if (policy.isInvalid()) { throw new DdlException("Analyzer '" + analyzerName + "' is invalid"); } - validateReferencedTokenFiltersUsableLocked(analyzerName, policy); + validateReferencedComponentsUsableLocked(analyzerName, policy); } finally { readUnlock(); } } /** - * Older metadata may retain token filter types no longer supported by BE, such as common_grams. - * Load these policies so a single obsolete policy cannot prevent FE startup, but reject any - * analyzer that references them at use time, before BE reports an unknown token filter during - * index construction or querying. + * Older metadata may retain components that current validation rejects. Load these policies so + * one obsolete policy cannot prevent FE startup, but reject analyzers that reference them before + * BE tries to construct the analyzer during index construction or querying. */ - private void validateReferencedTokenFiltersUsableLocked(String analyzerName, IndexPolicy analyzer) + private void validateReferencedComponentsUsableLocked(String analyzerName, IndexPolicy analyzer) throws DdlException { - String tokenFilterNames = analyzer.getProperties() == null - ? null : analyzer.getProperties().get(IndexPolicy.PROP_TOKEN_FILTER); + Map analyzerProperties = analyzer.getProperties(); + if (analyzerProperties == null) { + return; + } + String tokenizerName = analyzerProperties.get(IndexPolicy.PROP_TOKENIZER); + IndexPolicy tokenizer = tokenizerName == null + ? null : nameToIndexPolicy.get(normalizeKey(tokenizerName)); + if (tokenizer != null && tokenizer.isInvalid()) { + throw new DdlException("Analyzer '" + analyzerName + "' references invalid tokenizer '" + + tokenizerName + "'"); + } + + String tokenFilterNames = analyzerProperties.get(IndexPolicy.PROP_TOKEN_FILTER); if (tokenFilterNames == null || tokenFilterNames.isEmpty()) { return; } @@ -337,6 +347,9 @@ private void validatePolicyReference(String name, IndexPolicyTypeEnum expectedTy throw new DdlException("Referenced policy '" + name + "' is of type " + policy.getType() + " but expected " + expectedType); } + if (policy.isInvalid()) { + throw new DdlException("Referenced " + expectedType + " policy '" + name + "' is invalid"); + } } private void validateTokenizerProperties(Map properties) throws DdlException { @@ -667,10 +680,10 @@ public void gsonPostProcess() throws IOException { private static void warnIfUnsupported(IndexPolicy indexPolicy) { if (indexPolicy.isInvalid()) { - LOG.error("Index policy '{}' (id={}) uses token filter type '{}', which this version" - + " no longer supports; analyzers referencing it will be rejected. Drop the" - + " indexes and policies that depend on it.", indexPolicy.getName(), - indexPolicy.getId(), indexPolicy.getProperties().get(IndexPolicy.PROP_TYPE)); + LOG.error("Index policy '{}' (id={}, type={}) is not valid in this version; analyzers" + + " referencing it will be rejected. Drop the indexes and policies that depend" + + " on it.", indexPolicy.getName(), indexPolicy.getId(), + indexPolicy.getProperties().get(IndexPolicy.PROP_TYPE)); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java index b4c3c62ce44e29..63ccc86d9eab34 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java +++ b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java @@ -29,6 +29,8 @@ public class NGramTokenizerValidator extends BasePolicyValidator { // A configured range can emit one token per gram size at every input position. static final int MAX_NGRAM_DIFF = 255; + // NGramTokenizer keeps four code-point slots per configured gram plus a refill margin. + static final int MAX_NGRAM_SIZE = 1024; private static final Set ALLOWED_PROPS = ImmutableSet.of( "type", "min_gram", "max_gram", "max_ngram_diff", "token_chars", "custom_token_chars"); @@ -40,6 +42,15 @@ public NGramTokenizerValidator() { super(ALLOWED_PROPS); } + static boolean isValidPolicy(Map properties) { + try { + new NGramTokenizerValidator().validate(properties); + return true; + } catch (DdlException | RuntimeException e) { + return false; + } + } + @Override protected String getTypeName() { return "ngram tokenizer"; @@ -79,6 +90,9 @@ protected void validateSpecific(Map props) throws DdlException { throw new DdlException("max_gram [" + maxGram + "] " + "cannot be smaller than min_gram [" + minGram + "]"); } + if (minGram > MAX_NGRAM_SIZE || maxGram > MAX_NGRAM_SIZE) { + throw new DdlException("min_gram and max_gram must be less than or equal to " + MAX_NGRAM_SIZE); + } int maxNgramDiff = 1; if (props.containsKey("max_ngram_diff")) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java index 0910e3a71a1f05..46cba966175825 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java @@ -18,6 +18,7 @@ package org.apache.doris.analysis.invertedindex; import org.apache.doris.catalog.Env; +import org.apache.doris.common.DdlException; import org.apache.doris.indexpolicy.IndexPolicy; import org.apache.doris.indexpolicy.IndexPolicyMgr; import org.apache.doris.indexpolicy.IndexPolicyTypeEnum; @@ -116,7 +117,7 @@ public void testNgramValidationLimitDoesNotChangeAnalyzerIdentity() { Map tokenizerProps = new HashMap<>(); tokenizerProps.put(IndexPolicy.PROP_TYPE, "ngram"); tokenizerProps.put("min_gram", "1"); - tokenizerProps.put("max_gram", "8"); + tokenizerProps.put("max_gram", "2"); tokenizerProps.put("max_ngram_diff", "7"); IndexPolicy tokenizerWithLimit = new IndexPolicy( 1, "ngram_with_limit", IndexPolicyTypeEnum.TOKENIZER, tokenizerProps); @@ -143,6 +144,45 @@ public void testNgramValidationLimitDoesNotChangeAnalyzerIdentity() { } } + @Test + public void testReplayedInvalidNgramDoesNotBlockValidReplacement() throws Exception { + IndexPolicyMgr policyMgr = new IndexPolicyMgr(); + Env env = Mockito.mock(Env.class); + Mockito.when(env.getIndexPolicyMgr()).thenReturn(policyMgr); + + Map invalidProps = new HashMap<>(); + invalidProps.put(IndexPolicy.PROP_TYPE, "ngram"); + invalidProps.put("min_gram", "1"); + invalidProps.put("max_gram", "8"); + IndexPolicy invalidTokenizer = new IndexPolicy( + 10, "replayed_ngram", IndexPolicyTypeEnum.TOKENIZER, invalidProps); + + Map replacementProps = new HashMap<>(invalidProps); + replacementProps.put("max_ngram_diff", "7"); + IndexPolicy replacementTokenizer = new IndexPolicy( + 11, "replacement_ngram", IndexPolicyTypeEnum.TOKENIZER, replacementProps); + IndexPolicy invalidAnalyzer = analyzerPolicy(12, "replayed_analyzer", "replayed_ngram"); + IndexPolicy replacementAnalyzer = analyzerPolicy(13, "replacement_analyzer", "replacement_ngram"); + policyMgr.replayCreateIndexPolicy(invalidTokenizer); + policyMgr.replayCreateIndexPolicy(replacementTokenizer); + policyMgr.replayCreateIndexPolicy(invalidAnalyzer); + policyMgr.replayCreateIndexPolicy(replacementAnalyzer); + + Assertions.assertTrue(invalidTokenizer.isInvalid()); + Assertions.assertFalse(replacementTokenizer.isInvalid()); + Assertions.assertThrows(DdlException.class, + () -> policyMgr.validateAnalyzerExists("replayed_analyzer")); + + try (MockedStatic mockedEnv = Mockito.mockStatic(Env.class)) { + mockedEnv.when(Env::getCurrentEnv).thenReturn(env); + String invalidIdentity = AnalyzerIdentityBuilder.buildAnalyzerIdentity( + nonEmptyProperties(), "replayed_analyzer", "", "__default__", "none", null); + String replacementIdentity = AnalyzerIdentityBuilder.buildAnalyzerIdentity( + nonEmptyProperties(), "replacement_analyzer", "", "__default__", "none", null); + Assertions.assertNotEquals(invalidIdentity, replacementIdentity); + } + } + private IndexPolicy analyzerPolicy(long id, String name, String tokenizer) { Map properties = new HashMap<>(); properties.put(IndexPolicy.PROP_TOKENIZER, tokenizer); diff --git a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java index 54ecc91ed69078..e8ef0c0a31297b 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java +++ b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java @@ -200,6 +200,28 @@ public void testNGramValidator_AcceptsDifferenceLimitBoundary() { Assertions.assertDoesNotThrow(() -> validator.validate(props)); } + @Test + public void testNGramValidator_AcceptsAbsoluteSizeBoundary() { + NGramTokenizerValidator validator = new NGramTokenizerValidator(); + Map props = new HashMap<>(); + props.put("min_gram", Integer.toString(NGramTokenizerValidator.MAX_NGRAM_SIZE)); + props.put("max_gram", Integer.toString(NGramTokenizerValidator.MAX_NGRAM_SIZE)); + + Assertions.assertDoesNotThrow(() -> validator.validate(props)); + } + + @Test + public void testNGramValidator_RejectsExcessiveAbsoluteSize() { + NGramTokenizerValidator validator = new NGramTokenizerValidator(); + Map props = new HashMap<>(); + props.put("min_gram", Integer.toString(NGramTokenizerValidator.MAX_NGRAM_SIZE)); + props.put("max_gram", Integer.toString(NGramTokenizerValidator.MAX_NGRAM_SIZE + 1)); + + Exception exception = Assertions.assertThrows(DdlException.class, + () -> validator.validate(props)); + Assertions.assertTrue(exception.getMessage().contains("less than or equal to 1024")); + } + // StandardTokenizerValidator Tests @Test public void testStandardTokenizerValidator_ValidProperties() throws Exception { From 035a04947b30deb0e1dc7e72f18d0b3092399f07 Mon Sep 17 00:00:00 2001 From: airborne12 Date: Mon, 14 Sep 2026 05:28:48 +0800 Subject: [PATCH 6/6] [fix](inverted-index) Preserve legacy ngram policies ### What problem does this PR solve? Problem Summary: The absolute ngram size limit was applied during replay and BE reconstruction, so policies accepted before the limit existed could become unusable during a supported rolling upgrade. Persist an explicit compatibility marker on newly created ngram policies and enforce the absolute size limit only for marked policies. Preserve the former validation and construction behavior for marker-less legacy metadata. Cover serialized replay, dependent analyzer validation, marker persistence, and BE factory construction. ### Release note None ### Check List (For Author) - Test - [x] Unit Test - Behavior changed: - [x] Yes. Legacy ngram policies remain usable after upgrade while newly created policies retain the absolute size limit. - Does this need documentation? - [x] No. --- .../ngram/ngram_tokenizer_factory.cpp | 3 +- .../tokenizer/ngram_tokenizer_test.cpp | 13 ++++++ .../doris/indexpolicy/IndexPolicyMgr.java | 12 +++++- .../indexpolicy/NGramTokenizerValidator.java | 15 ++++++- .../AnalyzerIdentityBuilderTest.java | 20 ++++++++++ .../indexpolicy/PolicyValidatorTests.java | 40 +++++++++++++++++++ 6 files changed, 98 insertions(+), 5 deletions(-) diff --git a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp index b3d8bb4c596e2c..982ba376d29e24 100644 --- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp +++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp @@ -32,7 +32,8 @@ void NGramTokenizerFactory::initialize(const Settings& settings) { if (_min_gram > _max_gram) { throw Exception(ErrorCode::INVALID_ARGUMENT, "min_gram must not be greater than max_gram"); } - if (_min_gram > MAX_NGRAM_SIZE || _max_gram > MAX_NGRAM_SIZE) { + const bool has_max_ngram_diff = !settings.get_string("max_ngram_diff").empty(); + if (has_max_ngram_diff && (_min_gram > MAX_NGRAM_SIZE || _max_gram > MAX_NGRAM_SIZE)) { throw Exception(ErrorCode::INVALID_ARGUMENT, "min_gram and max_gram must be less than or equal to " + std::to_string(MAX_NGRAM_SIZE)); diff --git a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp index 8a11e048d072ca..ac3d42cdcca11b 100644 --- a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp +++ b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp @@ -139,6 +139,7 @@ TEST(NGramTokenizerTest, AbsoluteSizeBoundary) { std::unordered_map args; args["min_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_SIZE); args["max_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_SIZE); + args["max_ngram_diff"] = "1"; Settings settings(args); EXPECT_NO_THROW(factory.initialize(settings)); @@ -150,11 +151,23 @@ TEST(NGramTokenizerTest, ExcessiveAbsoluteSize) { std::unordered_map args; args["min_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_SIZE); args["max_gram"] = std::to_string(NGramTokenizerFactory::MAX_NGRAM_SIZE + 1); + args["max_ngram_diff"] = "1"; Settings settings(args); EXPECT_THROW(factory.initialize(settings), Exception); } +TEST(NGramTokenizerTest, LegacyFixedSizeAboveCurrentLimit) { + NGramTokenizerFactory factory; + std::unordered_map args; + args["min_gram"] = "2048"; + args["max_gram"] = "2048"; + Settings settings(args); + + EXPECT_NO_THROW(factory.initialize(settings)); + EXPECT_NO_THROW(factory.create()); +} + TEST(NGramTokenizerTest, SymbolCharactersHandling) { NGramTokenizerFactory factory; std::unordered_map args; diff --git a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java index 5f6ef7869d58fa..f7b5b81c108f10 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java @@ -196,8 +196,16 @@ public void createIndexPolicy(boolean ifNotExists, String policyName, writeLock(); try { - validatePolicyProperties(type, properties); - IndexPolicy indexPolicy = IndexPolicy.create(policyName, type, properties); + Map storedProperties = properties == null + ? null : Maps.newHashMap(properties); + validatePolicyProperties(type, storedProperties); + if (type == IndexPolicyTypeEnum.TOKENIZER + && "ngram".equals(storedProperties.get(IndexPolicy.PROP_TYPE))) { + // Presence distinguishes policies created with the absolute-size limit from + // compatible policies replayed from a version before max_ngram_diff existed. + storedProperties.putIfAbsent("max_ngram_diff", "1"); + } + IndexPolicy indexPolicy = IndexPolicy.create(policyName, type, storedProperties); if (nameToIndexPolicy.containsKey(normalizedName)) { if (ifNotExists) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java index 63ccc86d9eab34..df16c633e8add1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java +++ b/fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java @@ -38,13 +38,23 @@ public class NGramTokenizerValidator extends BasePolicyValidator { private static final Set VALID_TOKEN_CHARS = ImmutableSet.of( "letter", "digit", "whitespace", "punctuation", "symbol", "custom"); + private final boolean enforceAbsoluteSizeLimit; + public NGramTokenizerValidator() { + this(true); + } + + private NGramTokenizerValidator(boolean enforceAbsoluteSizeLimit) { super(ALLOWED_PROPS); + this.enforceAbsoluteSizeLimit = enforceAbsoluteSizeLimit; } static boolean isValidPolicy(Map properties) { try { - new NGramTokenizerValidator().validate(properties); + // Policies created before max_ngram_diff existed have no compatibility marker and + // must retain the absolute-size behavior accepted by the previous release. + boolean hasCompatibilityMarker = properties.containsKey("max_ngram_diff"); + new NGramTokenizerValidator(hasCompatibilityMarker).validate(properties); return true; } catch (DdlException | RuntimeException e) { return false; @@ -90,7 +100,8 @@ protected void validateSpecific(Map props) throws DdlException { throw new DdlException("max_gram [" + maxGram + "] " + "cannot be smaller than min_gram [" + minGram + "]"); } - if (minGram > MAX_NGRAM_SIZE || maxGram > MAX_NGRAM_SIZE) { + if (enforceAbsoluteSizeLimit + && (minGram > MAX_NGRAM_SIZE || maxGram > MAX_NGRAM_SIZE)) { throw new DdlException("min_gram and max_gram must be less than or equal to " + MAX_NGRAM_SIZE); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java index 46cba966175825..c65afc497a110a 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java @@ -183,6 +183,26 @@ public void testReplayedInvalidNgramDoesNotBlockValidReplacement() throws Except } } + @Test + public void testReplayedLegacyLargeNgramAnalyzerRemainsUsable() throws Exception { + IndexPolicyMgr policyMgr = new IndexPolicyMgr(); + Map legacyProps = new HashMap<>(); + legacyProps.put(IndexPolicy.PROP_TYPE, "ngram"); + legacyProps.put("min_gram", "2048"); + legacyProps.put("max_gram", "2048"); + IndexPolicy legacyTokenizer = new IndexPolicy( + 20, "legacy_large_ngram", IndexPolicyTypeEnum.TOKENIZER, legacyProps); + IndexPolicy legacyAnalyzer = analyzerPolicy( + 21, "legacy_large_analyzer", "legacy_large_ngram"); + + policyMgr.replayCreateIndexPolicy(legacyTokenizer); + policyMgr.replayCreateIndexPolicy(legacyAnalyzer); + + Assertions.assertFalse(legacyTokenizer.isInvalid()); + Assertions.assertDoesNotThrow( + () -> policyMgr.validateAnalyzerExists("legacy_large_analyzer")); + } + private IndexPolicy analyzerPolicy(long id, String name, String tokenizer) { Map properties = new HashMap<>(); properties.put(IndexPolicy.PROP_TOKENIZER, tokenizer); diff --git a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java index e8ef0c0a31297b..dba7ab0d5b50cb 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java +++ b/fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java @@ -17,10 +17,14 @@ package org.apache.doris.indexpolicy; +import org.apache.doris.catalog.Env; import org.apache.doris.common.DdlException; +import org.apache.doris.persist.EditLog; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; // import org.junit.jupiter.params.ParameterizedTest; // import org.junit.jupiter.params.provider.ValueSource; @@ -222,6 +226,42 @@ public void testNGramValidator_RejectsExcessiveAbsoluteSize() { Assertions.assertTrue(exception.getMessage().contains("less than or equal to 1024")); } + @Test + public void testLegacyNGramPolicyAboveCurrentLimitRemainsValidAfterReplay() throws Exception { + Map props = new HashMap<>(); + props.put(IndexPolicy.PROP_TYPE, "ngram"); + props.put("min_gram", "2048"); + props.put("max_gram", "2048"); + + IndexPolicy replayed = roundTrip(new IndexPolicy( + 1, "legacy_large_ngram", IndexPolicyTypeEnum.TOKENIZER, props)); + + Assertions.assertFalse(replayed.isInvalid()); + + props.put("max_ngram_diff", "1"); + IndexPolicy current = roundTrip(new IndexPolicy( + 2, "current_large_ngram", IndexPolicyTypeEnum.TOKENIZER, props)); + Assertions.assertTrue(current.isInvalid()); + } + + @Test + public void testNewNGramPolicyPersistsCompatibilityMarker() throws Exception { + Env env = Mockito.mock(Env.class); + Mockito.when(env.getNextId()).thenReturn(2L); + Mockito.when(env.getEditLog()).thenReturn(Mockito.mock(EditLog.class)); + IndexPolicyMgr policyMgr = new IndexPolicyMgr(); + Map props = new HashMap<>(); + props.put(IndexPolicy.PROP_TYPE, "ngram"); + + try (MockedStatic mockedEnv = Mockito.mockStatic(Env.class)) { + mockedEnv.when(Env::getCurrentEnv).thenReturn(env); + policyMgr.createIndexPolicy(false, "new_ngram", IndexPolicyTypeEnum.TOKENIZER, props); + } + + Assertions.assertEquals("1", + policyMgr.getPolicyByName("new_ngram").getProperties().get("max_ngram_diff")); + } + // StandardTokenizerValidator Tests @Test public void testStandardTokenizerValidator_ValidProperties() throws Exception {