This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 95ec58f7e91 [feat](inverted-index) Backport configurable ngram size
difference to branch-4.1 (#68561)
95ec58f7e91 is described below
commit 95ec58f7e91d4bb2c05c113be69346928c74cade
Author: Jack <[email protected]>
AuthorDate: Tue Sep 29 15:37:22 2026 +0800
[feat](inverted-index) Backport configurable ngram size difference to
branch-4.1 (#68561)
### What problem does this PR solve?
Related PRs: #67917 (source feature), #68550 (merged prerequisite).
Target: `branch-4.1`.
Source squash commit: `6a79f954e2617c807173aad9796cfd6138c14055`.
Problem Summary:
Backport configurable `max_ngram_diff` for custom ngram tokenizers. The
default is 1. Explicit values must be ASCII integers from 0 through 255,
and newly created policies limit `min_gram` and `max_gram` to 1024.
Policies persisted before this change keep their former absolute-size
behavior. Invalid replayed policies cannot block valid replacements. The
admission-only `max_ngram_diff` setting is excluded from analyzer
identity.
After #68550 merged, this PR conflicted in six files. The resolution
preserves its analyzer identity, invalid-component, policy-name, and
malformed UTF-8 behavior, then adds the remaining ngram admission and
replay semantics. The PR now includes the updated `branch-4.1` through a
merge commit, so it can be pushed without rewriting its published
history. All source hunks are accounted for below;
`AnalyzerIdentityBuilder.java` needs no PR diff because #68550 already
supplies its source behavior. The range-diff pairs the source squash
commit `6a79f954e26` with the original backport `b668aca4154`; merge
commit `eb4f0e89330` integrates the updated target.
### Release note
Allow custom ngram tokenizers to configure the maximum difference
between `max_gram` and `min_gram` with `max_ngram_diff` values from 0
through 255.
### Check List (For Author)
- Test
- [x] FE build and Checkstyle passed with zero violations.
- [x] FE `PolicyValidatorTests` 44/44 and `AnalyzerIdentityBuilderTest`
64/64 passed (108/108 total).
- [x] ASAN BE build passed with `-j192`; clang-format 16 checked all
five changed C++ files.
- [x] ASAN BE `NGramTokenizerTest` passed 21/21, including the two
malformed UTF-8 cases from #68550.
- [x] Regression `test_ngram_max_diff_custom_analyzer` passed 1/1.
- [x] Regression `test_analyzer_identity_semantics` from #68550 passed
1/1.
- [x] Changed-line clang-tidy passed on all five changed C++ files.
- [x] Merged-tree format preflight passed against the latest fetched
`branch-4.1` (clang-format and FE Checkstyle).
- Behavior changed:
- [x] Yes. New custom ngram policies can opt into a wider gram-size
difference; old persisted policies retain their prior absolute-size
behavior.
- Does this need documentation?
- [x] Yes. The custom analyzer documentation should describe
`max_ngram_diff`, its 0 through 255 range, and the 1024 absolute-size
limit for newly created policies. The source PR did not include a
documentation change.
The local BE link used a task-owned compatible Lance dependency copy
with an uncommitted shim for missing Lance symbols; this work did not
validate Lance behavior. The dependency copy and shim are outside the PR
diff. The isolated regression instance disabled Arrow Flight SQL after
its local flight port bind failed; the tested NGram path does not use
Arrow Flight SQL.
### Source hunk audit
# PR #67917 source hunk audit after merging #68550
Source squash commit: `6a79f954e2617c807173aad9796cfd6138c14055`. Status
is relative to current `branch-4.1`; every row refers to one source `@@`
hunk unless file bytes match exactly.
### `be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.cpp`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -106 +106,2 @@ void NGramTokenizer::init(int32_t min_gram, int32_t
max_gram, bool edges_only) {` | Ported | Retain the configured ngram
size in the 4.1 tokenizer layout. |
### `be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.h`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -78 +78 @@ private:` | Ported | Retain the source field change and
the 4.1 compile-check bracketing. |
###
`be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.cpp`
| Source hunk | Status | Evidence |
|---|---|---|
| All hunks | Ported (verbatim) | Source and final merged file bytes
match. |
###
`be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer_factory.h`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -28,0 +29,5 @@ public:` | Ported | The configurable difference is
exposed alongside the current factory interface. |
| `@@ -68 +73 @@ private:` | Ported | The configurable difference is
exposed alongside the current factory interface. |
### `be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -89,0 +90,81 @@ TEST(NGramTokenizerTest, InvalidMinMaxDifference)
{` | Ported | All seven difference and size-limit cases remain beside
the malformed UTF-8 tests from #68550. |
| `@@ -205 +286 @@ TEST(NGramTokenizerTest, WhitespaceTokenization) {` |
Already in target | The target already has the source EOF normalization.
|
###
`fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -30,0 +31,2 @@ public final class AnalyzerIdentityBuilder {` |
Already in target | Merged #68550 already contains this ngram identity
change. |
| `@@ -171,0 +174,3 @@ public final class AnalyzerIdentityBuilder {` |
Already in target | Merged #68550 already contains this ngram identity
change. |
| `@@ -179,0 +185,5 @@ public final class AnalyzerIdentityBuilder {` |
Already in target | Merged #68550 already contains this ngram identity
change. |
###
`fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicy.java`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -132 +132 @@ public class IndexPolicy implements Writable,
GsonPostProcessable {` | Already in target | Merged #68550 supplies the
common_grams invalidation path. |
| `@@ -134,0 +135,5 @@ public class IndexPolicy implements Writable,
GsonPostProcessable {` | Adapted | Use isValidPolicy for ngram replay
while retaining common_grams invalidation. |
###
`fe/fe-core/src/main/java/org/apache/doris/indexpolicy/IndexPolicyMgr.java`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -113 +113 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {` | Already in target | Merged #68550 uses shared
component validation. |
| `@@ -120,4 +120,3 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {` | Already in target | Merged #68550 has the
shared replay-validation comment. |
| `@@ -125 +124 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {` | Already in target | Merged #68550 has the
shared component-validation helper. |
| `@@ -127,2 +126,13 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {` | Already in target | Merged #68550 rejects
analyzers referencing invalid tokenizers. |
| `@@ -189,2 +199,10 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {` | Adapted | Persist max_ngram_diff after
validation and before policy creation under the current manager lock. |
| `@@ -339,0 +358,3 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {` | Already in target | Merged #68550 rejects
invalid referenced policies. |
| `@@ -670,4 +691,4 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {` | Already in target | Merged #68550 logs invalid
policies on image and edit-log replay. |
###
`fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java`
| Source hunk | Status | Evidence |
|---|---|---|
| All hunks | Ported (verbatim) | Source and final merged file bytes
match. |
###
`fe/fe-core/src/test/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilderTest.java`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -19,0 +20,2 @@ package org.apache.doris.analysis.invertedindex;` |
Already in target | Merged #68550 supplies this test import. |
| `@@ -20,0 +23,2 @@ import org.apache.doris.indexpolicy.IndexPolicy;` |
Already in target | Merged #68550 supplies this test import. |
| `@@ -23,0 +28,2 @@ import org.junit.jupiter.api.Test;` | Already in
target | Merged #68550 supplies this test import. |
| `@@ -103,0 +110,101 @@ public class AnalyzerIdentityBuilderTest {` |
Adapted | Retain the source difference-limit identity and replay cases
beside the target default-bound, malformed-policy, and legacy-size
cases. |
###
`fe/fe-core/src/test/java/org/apache/doris/indexpolicy/PolicyValidatorTests.java`
| Source hunk | Status | Evidence |
|---|---|---|
| `@@ -19,0 +20 @@ package org.apache.doris.indexpolicy;` | Already in
target | Merged #68550 supplies this test import. |
| `@@ -20,0 +22 @@ import org.apache.doris.common.DdlException;` |
Already in target | Merged #68550 supplies this test import. |
| `@@ -23,0 +26,2 @@ import org.junit.jupiter.api.Test;` | Already in
target | Merged #68550 supplies this test import. |
| `@@ -132,0 +137 @@ public class PolicyValidatorTests {` | Already in
target | Merged #68550 already covers basic ngram validation. |
| `@@ -135,0 +141,124 @@ public class PolicyValidatorTests {` | Adapted
| Keep ten source validation/replay cases and the policy round-trip
helper; adjust the target default-valid case to a difference of one. |
###
`regression-test/data/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.out`
| Source hunk | Status | Evidence |
|---|---|---|
| All hunks | Ported (verbatim) | Source and final merged file bytes
match. |
###
`regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy`
| Source hunk | Status | Evidence |
|---|---|---|
| All hunks | Ported (verbatim) | Source and final merged file bytes
match. |
---
.../inverted/tokenizer/ngram/ngram_tokenizer.cpp | 3 +-
.../inverted/tokenizer/ngram/ngram_tokenizer.h | 2 +-
.../tokenizer/ngram/ngram_tokenizer_factory.cpp | 29 ++++-
.../tokenizer/ngram/ngram_tokenizer_factory.h | 7 +-
.../inverted/tokenizer/ngram_tokenizer_test.cpp | 81 +++++++++++++
.../org/apache/doris/indexpolicy/IndexPolicy.java | 6 +-
.../apache/doris/indexpolicy/IndexPolicyMgr.java | 5 +
.../doris/indexpolicy/NGramTokenizerValidator.java | 55 ++++++++-
.../invertedindex/AnalyzerIdentityBuilderTest.java | 75 ++++++++++++
.../doris/indexpolicy/PolicyValidatorTests.java | 132 ++++++++++++++++++++-
.../test_ngram_max_diff_custom_analyzer.out | 4 +
.../test_ngram_max_diff_custom_analyzer.groovy | 69 +++++++++++
12 files changed, 455 insertions(+), 13 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 e8fcbea9d79..d3d63b7a885 100644
--- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.cpp
+++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.cpp
@@ -115,7 +115,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<size_t>(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 55515b2a273..72b6110ea86 100644
--- a/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.h
+++ b/be/src/storage/index/inverted/tokenizer/ngram/ngram_tokenizer.h
@@ -77,4 +77,4 @@ private:
};
#include "common/compile_check_end.h"
-} // 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 c5b6c5a9c73..982ba376d29 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<std::string, CharMatcherPtr>
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 d064749d9d5..00334e303cb 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;
@@ -61,4 +66,4 @@ private:
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 8529b6b081d..a82e8b8b56b 100644
--- a/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp
+++ b/be/test/storage/index/inverted/tokenizer/ngram_tokenizer_test.cpp
@@ -110,6 +110,87 @@ TEST(NGramTokenizerTest, InvalidMinMaxDifference) {
ASSERT_TRUE(exception_thrown);
}
+TEST(NGramTokenizerTest, ConfiguredMinMaxDifference) {
+ NGramTokenizerFactory factory;
+ std::unordered_map<std::string, std::string> 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<std::string> 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<std::string, std::string> args;
+ args["max_ngram_diff"] = "-1";
+ Settings settings(args);
+
+ EXPECT_THROW(factory.initialize(settings), Exception);
+}
+
+TEST(NGramTokenizerTest, ExcessiveConfiguredDifferenceLimit) {
+ NGramTokenizerFactory factory;
+ std::unordered_map<std::string, std::string> 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<std::string, std::string> 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<std::string, std::string> 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<std::string, std::string> 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<std::string, std::string> 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<std::string, std::string> args;
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 b4f149c650f..0158abb6e56 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
@@ -135,11 +135,7 @@ public class IndexPolicy implements Writable,
GsonPostProcessable {
}
if (type == IndexPolicyTypeEnum.TOKENIZER
&& "ngram".equals(properties.get(PROP_TYPE))) {
- try {
- new NGramTokenizerValidator().validate(properties);
- } catch (DdlException | RuntimeException e) {
- return true;
- }
+ return !NGramTokenizerValidator.isValidPolicy(properties);
}
return false;
}
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 f00914dc2c1..29838730406 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
@@ -332,6 +332,11 @@ public class IndexPolicyMgr implements Writable,
GsonPostProcessable {
Map<String, String> storedProperties = properties == null
? null : Maps.newHashMap(properties);
validatePolicyProperties(type, storedProperties);
+ if (type == IndexPolicyTypeEnum.TOKENIZER
+ &&
"ngram".equals(storedProperties.get(IndexPolicy.PROP_TYPE))) {
+ // Persist the marker so replay applies the size limit to
newly created policies.
+ storedProperties.putIfAbsent("max_ngram_diff", "1");
+ }
IndexPolicy indexPolicy = IndexPolicy.create(policyName, type,
storedProperties);
if (nameToIndexPolicy.containsKey(normalizedName)) {
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 03c08cbda6c..df16c633e8a 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.Map;
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<String> 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<String> 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<String, String> 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 @@ public class NGramTokenizerValidator extends
BasePolicyValidator {
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 818b834da5b..32b5f668cef 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
@@ -150,6 +150,42 @@ public class AnalyzerIdentityBuilderTest {
}
}
+ @Test
+ public void testNgramValidationLimitDoesNotChangeAnalyzerIdentity() {
+ IndexPolicyMgr policyMgr = Mockito.mock(IndexPolicyMgr.class);
+ Env env = Mockito.mock(Env.class);
+ Mockito.when(env.getIndexPolicyMgr()).thenReturn(policyMgr);
+
+ Map<String, String> 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<String, String> 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<Env> 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();
@@ -189,6 +225,45 @@ public class AnalyzerIdentityBuilderTest {
}
}
+ @Test
+ public void testReplayedNgramDifferenceLimitDoesNotBlockValidReplacement()
throws Exception {
+ IndexPolicyMgr policyMgr = new IndexPolicyMgr();
+ Env env = Mockito.mock(Env.class);
+ Mockito.when(env.getIndexPolicyMgr()).thenReturn(policyMgr);
+
+ Map<String, String> 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<String, String> 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<Env> 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();
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 0cc01113262..0a99eaf52f3 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
@@ -127,10 +127,134 @@ public class PolicyValidatorTests {
NGramTokenizerValidator validator = new NGramTokenizerValidator();
Map<String, String> props = new HashMap<>();
props.put("min_gram", "3");
- props.put("max_gram", "5");
+ props.put("max_gram", "4");
+ validator.validate(props); // Should not throw
+ }
+
+ @Test
+ public void testNGramValidator_DefaultDifferenceLimit() {
+ NGramTokenizerValidator validator = new NGramTokenizerValidator();
+ Map<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> props = new HashMap<>();
+ props.put(IndexPolicy.PROP_TYPE, "ngram");
+
+ try (MockedStatic<Env> 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 {
@@ -236,6 +360,12 @@ public class PolicyValidatorTests {
Assertions.assertTrue(exception.getMessage().contains("enclosed in
square brackets"));
}
+ private static IndexPolicy roundTrip(IndexPolicy policy) throws Exception {
+ ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+ policy.write(new DataOutputStream(bytes));
+ return IndexPolicy.read(new DataInputStream(new
ByteArrayInputStream(bytes.toByteArray())));
+ }
+
private static IndexPolicyMgr roundTrip(IndexPolicyMgr manager) throws
Exception {
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
manager.write(new DataOutputStream(bytes));
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 00000000000..07b08b46c6e
--- /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 }, [...]
+
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 00000000000..5e9a84e86d9
--- /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}"')"""
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
