airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4075519682
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -150,47 +282,562 @@ private static String
buildIdentityFromPolicyProperties(IndexPolicyTypeEnum type
* Resolve a component (tokenizer) to its identity.
*/
private static String resolveComponentIdentity(String name,
IndexPolicyTypeEnum expectedType) {
+ return resolveComponentIdentity(name, expectedType, null);
+ }
+
+ /** {@code fold} is the case-folding context of a char filter, or null
without a downstream fold. */
+ private static String resolveComponentIdentity(
+ String name, IndexPolicyTypeEnum expectedType, FoldContext fold) {
if (Strings.isNullOrEmpty(name)) {
return "";
}
- // Check if it's a built-in component
- if (expectedType == IndexPolicyTypeEnum.TOKENIZER
- && IndexPolicy.BUILTIN_TOKENIZERS.contains(name)) {
- return name;
+ // Existing named policies take precedence over built-ins for upgrade
compatibility.
+ try {
+ Env env = Env.getCurrentEnv();
+ if (env != null && env.getIndexPolicyMgr() != null) {
+ IndexPolicy policy =
env.getIndexPolicyMgr().getPolicyByName(name);
+ if (policy != null && policy.getType() == expectedType) {
+ if (policy.isInvalid()) {
+ return "invalid-policy:" + policy.getId() + ":" +
policy.getName();
+ }
+ Map<String, String> props = policy.getProperties();
+ if (props != null && !props.isEmpty()) {
+ TreeMap<String, String> sortedProps = new
TreeMap<>(props);
+ String type = sortedProps.get(IndexPolicy.PROP_TYPE);
+ String normalizedType =
normalizeBuiltinComponentName(type, expectedType);
+ if (normalizedType != null) {
+ if ("empty".equals(normalizedType)) {
+ return "";
+ }
+ sortedProps.put(IndexPolicy.PROP_TYPE,
normalizedType);
+ canonicalizeEffectiveComponentProperties(
+ sortedProps, normalizedType, expectedType);
+ if (sortedProps.size() == 1) {
+ return normalizedType;
+ }
+ }
+ 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);
+ }
+ if (expectedType == IndexPolicyTypeEnum.CHAR_FILTER
+ &&
"char_replace".equals(sortedProps.get(IndexPolicy.PROP_TYPE))) {
+ String replacement =
sortedProps.getOrDefault("replacement", " ");
+ String pattern = canonicalizeCharReplacePattern(
+ sortedProps.get("pattern"), replacement,
fold);
+ if (pattern.isEmpty()) {
+ return "";
+ }
+ sortedProps.put("pattern", pattern);
+ sortedProps.put("replacement", replacement);
+ }
+ if (normalizedType != null && sortedProps.size() == 1)
{
+ return normalizedType;
+ }
+ return sortedProps.toString();
+ }
+ }
+ }
+ } catch (RuntimeException e) {
+ // Fall through to built-in resolution or the original name.
+ }
+
+ String normalizedName = normalizeBuiltinComponentName(name,
expectedType);
+ return "empty".equals(normalizedName) ? "" : normalizedName == null ?
name : normalizedName;
+ }
+
+ private static void canonicalizeEffectiveComponentProperties(
+ TreeMap<String, String> properties, String type,
IndexPolicyTypeEnum expectedType) {
+ if ("pinyin".equals(type)) {
+ removeBooleanDefaults(properties, true,
+ "keep_first_letter", "keep_full_pinyin",
"keep_none_chinese",
+ "keep_none_chinese_together",
"keep_none_chinese_in_first_letter",
+ "lowercase", "trim_whitespace", "ignore_pinyin_offset",
+ "none_chinese_pinyin_tokenize");
+ removeBooleanDefaults(properties, false,
+ "keep_separate_first_letter", "keep_joined_full_pinyin",
"keep_original",
+ "keep_none_chinese_in_joined_full_pinyin",
"remove_duplicated_term",
+ "fixed_pinyin_offset", "keep_separate_chinese");
+ removeIntegerDefault(properties, "limit_first_letter_length", 16);
+ canonicalizePinyinDependencies(properties, expectedType);
+ return;
}
- // For custom component, get its properties
+ if (expectedType == IndexPolicyTypeEnum.TOKEN_FILTER) {
+ if ("asciifolding".equals(type)) {
+ removeBooleanDefaults(properties, false, "preserve_original");
+ } else if ("word_delimiter".equals(type)) {
+ removeBooleanDefaults(properties, true, "generate_word_parts",
"generate_number_parts",
+ "split_on_case_change", "split_on_numerics",
"stem_english_possessive");
+ removeBooleanDefaults(properties, false, "catenate_words",
"catenate_numbers",
+ "catenate_all", "preserve_original");
+ canonicalizeWordSet(properties, "protected_words");
+ canonicalizeTypeTable(properties);
+ } else if ("icu_normalizer".equals(type)) {
+ canonicalizeIcuNormalizerDefaults(properties, false);
+ }
+ return;
+ }
+
+ if (expectedType == IndexPolicyTypeEnum.CHAR_FILTER) {
+ if ("icu_normalizer".equals(type)) {
+ canonicalizeIcuNormalizerDefaults(properties, true);
+ }
+ return;
+ }
+
+ if (expectedType != IndexPolicyTypeEnum.TOKENIZER) {
+ return;
+ }
+ switch (type) {
+ case "ngram":
+ case "edge_ngram":
+ removeIntegerDefault(properties, "min_gram", 1);
+ removeIntegerDefault(properties, "max_gram", 2);
+ canonicalizeWordSet(properties, "token_chars");
+ canonicalizeCustomTokenChars(properties);
+ break;
+ case "standard":
+ removeIntegerDefault(properties, "max_token_length", 255);
+ break;
+ case "char_group":
+ removeIntegerDefault(properties, "max_token_length", 255);
+ canonicalizeTokenizeOnChars(properties);
+ break;
+ case "keyword":
+ // BE only range-checks buffer_size; the emitted term is
always capped by a constant.
+ properties.remove("buffer_size");
+ break;
+ case "basic":
+ canonicalizeBasicExtraChars(properties);
+ break;
+ default:
+ break;
+ }
+ }
+
+ private static void removeBooleanDefaults(
+ TreeMap<String, String> properties, boolean defaultValue,
String... keys) {
+ for (String key : keys) {
+ String value = properties.get(key);
+ if (value == null || !("true".equalsIgnoreCase(value) ||
"false".equalsIgnoreCase(value))) {
+ continue;
+ }
+ boolean parsed = Boolean.parseBoolean(value);
+ if (parsed == defaultValue) {
+ properties.remove(key);
+ } else {
+ properties.put(key, Boolean.toString(parsed));
+ }
+ }
+ }
+
+ private static void removeIntegerDefault(
+ TreeMap<String, String> properties, String key, int defaultValue) {
+ String value = properties.get(key);
+ if (value == null) {
+ return;
+ }
try {
- Env env = Env.getCurrentEnv();
- if (env == null || env.getIndexPolicyMgr() == null) {
- return name;
+ int parsed = Integer.parseInt(value);
+ if (parsed == defaultValue) {
+ properties.remove(key);
+ } else {
+ properties.put(key, Integer.toString(parsed));
}
+ } catch (NumberFormatException e) {
+ // Invalid policies keep their original identity.
+ }
+ }
- IndexPolicy policy = env.getIndexPolicyMgr().getPolicyByName(name);
- if (policy == null || policy.getType() != expectedType) {
- return name;
+ private static void canonicalizeIcuNormalizerDefaults(
+ TreeMap<String, String> properties, boolean hasMode) {
+ String name = properties.get("name");
+ if (name != null) {
+ String normalizedName = name.trim().toLowerCase(Locale.ROOT);
+ if ("nfkc_cf".equals(normalizedName)) {
+ properties.remove("name");
+ } else {
+ properties.put("name", normalizedName);
}
- if (policy.isInvalid()) {
- return "invalid-policy:" + policy.getId() + ":" +
policy.getName();
+ }
+ String filter = properties.get("unicode_set_filter");
+ if (filter != null && filter.isEmpty()) {
+ // BE treats an explicit empty string like an absent filter.
+ properties.remove("unicode_set_filter");
+ } else if (filter != null) {
+ try {
+ UnicodeSet unicodeSet = new UnicodeSet(filter);
+ if (unicodeSet.isEmpty()) {
+ properties.remove("unicode_set_filter");
+ } else {
+ properties.put("unicode_set_filter",
unicodeSet.toPattern(false));
+ }
+ } catch (IllegalArgumentException e) {
+ // Invalid policies keep their original identity.
}
+ }
+ if (hasMode) {
+ canonicalizeIcuNormalizerMode(properties);
+ }
+ }
- Map<String, String> props = policy.getProperties();
- if (props == null || props.isEmpty()) {
- return name;
+ private static void canonicalizeIcuNormalizerMode(TreeMap<String, String>
properties) {
+ removeStringDefault(properties, "mode", "compose");
+ if (!"decompose".equals(properties.get("mode"))) {
+ return;
+ }
+ // BE ignores mode for nfd/nfkd, and nfc/nfkc in decompose mode are
the same ICU instances.
+ String name = properties.get("name");
+ if ("nfc".equals(name) || "nfd".equals(name)) {
+ properties.put("name", "nfd");
+ properties.remove("mode");
+ } else if ("nfkc".equals(name) || "nfkd".equals(name)) {
+ properties.put("name", "nfkd");
+ properties.remove("mode");
+ }
+ }
+
+ // BE reads these settings as unordered sets of trimmed, non-empty words.
+ private static void canonicalizeWordSet(TreeMap<String, String>
properties, String key) {
+ String value = properties.get(key);
+ if (value == null) {
+ return;
+ }
+ TreeSet<String> words = new TreeSet<>();
+ for (String word : value.split(",")) {
+ String trimmed = trimAsciiWhitespace(word);
+ if (!trimmed.isEmpty()) {
+ words.add(trimmed);
}
+ }
+ if (words.isEmpty()) {
+ properties.remove(key);
+ } else {
+ properties.put(key, String.join(",", words));
+ }
+ }
+
+ // BE matches custom token characters as a code point set ORed with the
named classes.
+ private static void canonicalizeCustomTokenChars(TreeMap<String, String>
properties) {
+ String value = properties.get("custom_token_chars");
+ if (value == null) {
+ return;
+ }
+ String tokenChars = properties.getOrDefault("token_chars", "");
+ Set<String> classes = new TreeSet<>(List.of(tokenChars.split(",")));
+ StringBuilder canonical = new StringBuilder();
+ value.codePoints().distinct().sorted()
+ .filter(codePoint -> !isCoveredByAsciiClass(codePoint,
classes))
+ .forEach(canonical::appendCodePoint);
+ if (canonical.length() == 0 && !value.isEmpty() &&
classes.remove("custom")) {
+ properties.remove("custom_token_chars");
+ properties.put("token_chars", String.join(",", classes));
+ return;
+ }
+ properties.put("custom_token_chars", canonical.toString());
+ }
- // 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);
+ // BE collects tokenize_on_chars entries into sets and checks the
categories before the literals.
+ private static void canonicalizeTokenizeOnChars(TreeMap<String, String>
properties) {
+ List<String> entries =
parseEntryList(properties.get("tokenize_on_chars"));
+ if (entries == null) {
+ return;
+ }
+ TreeSet<String> canonical = new TreeSet<>(entries);
+ Set<String> classes = new TreeSet<>(canonical);
+ classes.retainAll(CHAR_GROUP_TYPES);
+ canonical.removeIf(entry -> entry.indexOf('\\') < 0
+ && entry.codePointCount(0, entry.length()) == 1
+ && isCoveredByAsciiClass(entry.codePointAt(0), classes));
+ putEntryList(properties, "tokenize_on_chars", canonical);
+ }
+
+ /**
+ * Whether a named character class of the ngram or char_group tokenizer
already matches this
+ * code point. Only ASCII is judged: its categories never change between
the ICU versions FE
+ * and BE link against, and the class predicates agree there.
+ */
+ private static boolean isCoveredByAsciiClass(int codePoint, Set<String>
classes) {
+ if (codePoint >= 128) {
+ return false;
+ }
+ int type = UCharacter.getType(codePoint);
+ for (String name : classes) {
+ switch (name) {
+ case "letter":
+ if (UCharacter.isLetter(codePoint)) {
+ return true;
+ }
+ break;
+ case "digit":
+ if (UCharacter.isDigit(codePoint)) {
+ return true;
+ }
+ break;
+ case "whitespace":
+ if (UCharacter.isWhitespace(codePoint)) {
+ return true;
+ }
+ break;
+ case "punctuation":
+ if (type == UCharacter.START_PUNCTUATION || type ==
UCharacter.END_PUNCTUATION
+ || type == UCharacter.OTHER_PUNCTUATION || type ==
UCharacter.CONNECTOR_PUNCTUATION
+ || type == UCharacter.DASH_PUNCTUATION || type ==
UCharacter.INITIAL_PUNCTUATION
+ || type == UCharacter.FINAL_PUNCTUATION) {
+ return true;
+ }
+ break;
+ case "symbol":
+ if (type == UCharacter.CURRENCY_SYMBOL || type ==
UCharacter.MATH_SYMBOL
+ || type == UCharacter.OTHER_SYMBOL || type ==
UCharacter.MODIFIER_SYMBOL) {
+ return true;
+ }
+ break;
+ default:
+ break;
}
- return sortedProps.toString();
- } catch (RuntimeException e) {
- return name;
}
+ return false;
+ }
+
+ // BE builds a per-character type map where a later rule for the same
character wins.
+ private static void canonicalizeTypeTable(TreeMap<String, String>
properties) {
+ List<String> rules = parseEntryList(properties.get("type_table"));
+ if (rules == null) {
+ return;
+ }
+ TreeMap<Integer, String> types = new TreeMap<>();
+ for (String rule : rules) {
+ int arrow = rule.lastIndexOf("=>");
+ if (arrow < 0 || rule.indexOf('\n') >= 0 || rule.indexOf('\r') >=
0) {
+ return;
+ }
+ String character = trimAsciiWhitespace(rule.substring(0, arrow));
+ String type = trimAsciiWhitespace(rule.substring(arrow + 2));
+ // Escaped characters keep the original identity rather than
reproducing BE unescaping.
+ if (character.indexOf('\\') >= 0 || character.codePointCount(0,
character.length()) != 1
+ || !WORD_DELIMITER_TYPES.contains(type)) {
+ return;
+ }
+ types.put(character.codePointAt(0), type);
+ }
+ // A rule that restates BE's own classification changes nothing, but a
table made only of
+ // such rules still replaces BE's default table, which classifies
Latin-1 differently.
+ TreeMap<Integer, String> effectiveTypes = new TreeMap<>(types);
+ effectiveTypes.entrySet().removeIf(
+ entry ->
entry.getValue().equals(defaultWordDelimiterType(entry.getKey())));
+ if (effectiveTypes.isEmpty()) {
+ effectiveTypes = types;
+ }
+ List<String> canonicalRules = new ArrayList<>();
+ for (Map.Entry<Integer, String> entry : effectiveTypes.entrySet()) {
+ canonicalRules.add(new String(Character.toChars(entry.getKey())) +
"=>" + entry.getValue());
+ }
+ putEntryList(properties, "type_table", canonicalRules);
+ }
+
+ /** BE's u_charType classification of an ASCII code point, or null for
anything else. */
+ private static String defaultWordDelimiterType(int codePoint) {
+ if (codePoint >= 128) {
+ return null;
+ }
+ switch (UCharacter.getType(codePoint)) {
+ case UCharacter.UPPERCASE_LETTER:
+ return "UPPER";
+ case UCharacter.LOWERCASE_LETTER:
+ return "LOWER";
+ case UCharacter.DECIMAL_DIGIT_NUMBER:
+ return "DIGIT";
+ default:
+ return "SUBWORD_DELIM";
+ }
+ }
+
+ /** Parse a bracketed entry list as BE does, or return null for a
malformed list. */
+ private static List<String> parseEntryList(String value) {
+ if (value == null) {
+ return null;
+ }
+ List<String> entries = new ArrayList<>();
+ String trimmed = trimAsciiWhitespace(value);
+ if (trimmed.isEmpty()) {
+ return entries;
+ }
+ for (String item : ENTRY_SEPARATOR.split(trimmed)) {
+ String entry = trimAsciiWhitespace(item);
+ if (entry.length() < 2 || entry.charAt(0) != '[' ||
entry.charAt(entry.length() - 1) != ']') {
+ return null;
+ }
+ String content = entry.substring(1, entry.length() - 1);
+ if (!content.isEmpty()) {
+ entries.add(content);
+ }
+ }
+ return entries;
+ }
+
+ private static void putEntryList(TreeMap<String, String> properties,
String key, Collection<String> entries) {
+ if (entries.isEmpty()) {
+ properties.remove(key);
+ return;
+ }
+ StringBuilder canonical = new StringBuilder();
+ for (String entry : entries) {
+ if (canonical.length() > 0) {
+ canonical.append(",");
+ }
+ canonical.append("[").append(entry).append("]");
+ }
+ properties.put(key, canonical.toString());
+ }
+
+ // Trim the same ASCII whitespace that BE trims.
+ private static String trimAsciiWhitespace(String value) {
+ int begin = 0;
+ int end = value.length();
+ while (begin < end && isAsciiWhitespace(value.charAt(begin))) {
+ ++begin;
+ }
+ while (end > begin && isAsciiWhitespace(value.charAt(end - 1))) {
+ --end;
+ }
+ return value.substring(begin, end);
+ }
+
+ private static boolean isAsciiWhitespace(char value) {
+ return value == ' ' || (value >= '\t' && value <= '\r');
+ }
+
+ // BE consumes an ASCII alphanumeric run before it consults extra_chars.
+ private static void canonicalizeBasicExtraChars(TreeMap<String, String>
properties) {
+ String extraChars = properties.get("extra_chars");
+ if (extraChars == null) {
+ return;
+ }
+ boolean[] present = new boolean[128];
+ for (int i = 0; i < extraChars.length(); ++i) {
+ char value = extraChars.charAt(i);
+ if (value >= present.length) {
+ return;
+ }
+ boolean alphanumeric = (value >= '0' && value <= '9') || (value >=
'A' && value <= 'Z')
+ || (value >= 'a' && value <= 'z');
+ present[value] = !alphanumeric;
+ }
+ StringBuilder canonical = new StringBuilder();
+ for (int i = 0; i < present.length; ++i) {
+ if (present[i]) {
+ canonical.append((char) i);
+ }
+ }
+ if (canonical.length() == 0) {
+ properties.remove("extra_chars");
+ } else {
+ properties.put("extra_chars", canonical.toString());
+ }
+ }
+
+ private static void canonicalizePinyinDependencies(
Review Comment:
Confirmed and fixed in 79715d0b1b2.
`lowercase` is dropped from a pinyin identity only when `keep_first_letter`,
`keep_none_chinese`, `keep_original` and the joined output are all effectively
false. In that configuration every emitted candidate comes from
`TONELESS_PINYIN_FORMAT`, whose case is already fixed, so the two settings
tokenize identically. The removal is gated to TOKENIZER policies: the pinyin
token filter keeps the property because it falls back to the incoming token
when it would otherwise emit nothing, and that token can carry case.
Test: `testPinyinLowercaseIgnoredWhenOnlyFullPinyinIsEmitted`.
##########
be/src/storage/index/inverted/similarity/predicate_collector.cpp:
##########
@@ -61,14 +61,22 @@ InvertedIndexAnalyzerCtx analyzer_context_from_properties(
return analyzer_ctx;
}
-std::vector<TermInfo> analyze_plain_query(const std::string& value,
- const InvertedIndexAnalyzerCtx&
analyzer_ctx) {
+Result<std::vector<TermInfo>> analyze_plain_query(const std::string& value,
Review Comment:
Confirmed and fixed in 5843206b21c.
`create_analyzer_provider()` reaches `process_filter_configs()`, which
throws a `doris::Exception` for a component whose policy belongs to another
family, and `SearchPredicateCollector` built the context before entering the
guarded region, so that exception escaped the `Status`-returning scoring
collector.
`analyzer_context_from_properties()` is now declared in the header and
returns `Result<InvertedIndexAnalyzerCtx>`, wrapping context and provider
construction together and mapping both `CLuceneError` and `Exception` to
`INVERTED_INDEX_ANALYZER_ERROR`; the call site propagates that error instead.
Test:
`CollectionStatisticsTest.ScoringAnalyzerContextReportsWrongFamilyComponentAsStatus`
installs an `IndexPolicyMgr` that replays a CHAR_FILTER policy referenced as a
`token_filter` and asserts the call returns the status rather than throwing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
