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 c5b6c5a9c733f4..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 @@ -26,12 +26,35 @@ 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"); + } + 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)); + } + 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"); + } + 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 > 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 +103,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/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h index 2ee428e32ff77f..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 @@ -26,6 +26,11 @@ 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; + // 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; @@ -65,4 +70,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 b9dca64838fe30..ac3d42cdcca11b 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,87 @@ 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, "abcdefgh"); + + 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); +} + +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, 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, 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); + args["max_ngram_diff"] = "1"; + 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); + 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; @@ -202,4 +283,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/analysis/invertedindex/AnalyzerIdentityBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java index 1e640fd7d22c86..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 @@ -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() { } @@ -169,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()) { @@ -177,6 +182,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/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..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 @@ -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; } @@ -186,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) { @@ -337,6 +355,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 +688,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 03c08cbda6c6fb..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 @@ -27,14 +27,38 @@ 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; + // 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", "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"); + private final boolean enforceAbsoluteSizeLimit; + public NGramTokenizerValidator() { + this(true); + } + + private NGramTokenizerValidator(boolean enforceAbsoluteSizeLimit) { super(ALLOWED_PROPS); + this.enforceAbsoluteSizeLimit = enforceAbsoluteSizeLimit; + } + + static boolean isValidPolicy(Map properties) { + try { + // 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; + } } @Override @@ -76,6 +100,35 @@ protected void validateSpecific(Map props) throws DdlException { throw new DdlException("max_gram [" + maxGram + "] " + "cannot be smaller than min_gram [" + minGram + "]"); } + 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); + } + + 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(value); + 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"); + } + } + + 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"); 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..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 @@ -17,10 +17,16 @@ 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; 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 +107,105 @@ 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", "2"); + 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); + } + } + + @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); + } + } + + @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); + 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 4418d6270a4952..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; @@ -130,9 +134,134 @@ 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")); + } + + @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")); + } + + @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)); + } + + @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")); + } + + @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 { 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 new file mode 100644 index 00000000000000..5e9a84e86d9740 --- /dev/null +++ b/regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy @@ -0,0 +1,69 @@ +// 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}") + + qt_ngram_tokens """SELECT TOKENIZE('abcdefgh', '"analyzer"="${ngramAnalyzer}"')""" +}