Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<size_t>(max_gram) * 4 + 1024;
_buffer.resize(buffer_size);
}

void NGramTokenizer::update_last_non_token_char() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,4 @@ class NGramTokenizer : public DorisTokenizer {
std::string _utf8_buffer;
};

} // namespace doris::segment_v2::inverted_index
} // namespace doris::segment_v2::inverted_index
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Comment thread
airborne12 marked this conversation as resolved.
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);
Expand Down Expand Up @@ -80,4 +103,4 @@ CharMatcherPtr NGramTokenizerFactory::parse_token_chars(const Settings& settings
return builder.build();
}

} // namespace doris::segment_v2::inverted_index
} // namespace doris::segment_v2::inverted_index
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -65,4 +70,4 @@ class NGramTokenizerFactory : public TokenizerFactory {
CharMatcherPtr _matcher;
};

}; // namespace doris::segment_v2::inverted_index
}; // namespace doris::segment_v2::inverted_index
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,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;
Expand Down Expand Up @@ -202,4 +283,4 @@ TEST(NGramTokenizerTest, WhitespaceTokenization) {
ASSERT_EQ(tokens, expected);
}

} // namespace doris::segment_v2
} // namespace doris::segment_v2
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
}

Expand Down Expand Up @@ -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<String, String> props = policy.getProperties();
if (props == null || props.isEmpty()) {
Expand All @@ -177,6 +182,11 @@ private static String resolveComponentIdentity(String name, IndexPolicyTypeEnum

// Build identity from sorted properties
TreeMap<String, String> 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);
Comment thread
airborne12 marked this conversation as resolved.
}
return sortedProps.toString();
} catch (RuntimeException e) {
return name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,13 @@ public List<String> 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);
Comment thread
airborne12 marked this conversation as resolved.
return hasUnsupportedTokenFilter || hasInvalidNgramTokenizer;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> 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;
}
Expand Down Expand Up @@ -186,8 +196,16 @@ public void createIndexPolicy(boolean ifNotExists, String policyName,

writeLock();
try {
validatePolicyProperties(type, properties);
IndexPolicy indexPolicy = IndexPolicy.create(policyName, type, properties);
Map<String, String> 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) {
Expand Down Expand Up @@ -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<String, String> properties) throws DdlException {
Expand Down Expand Up @@ -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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<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");
Comment thread
airborne12 marked this conversation as resolved.

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
Expand Down Expand Up @@ -76,6 +100,35 @@ protected void validateSpecific(Map<String, String> 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");
Expand Down
Loading
Loading