github-actions[bot] commented on code in PR #67917:
URL: https://github.com/apache/doris/pull/67917#discussion_r4000221500
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java:
##########
@@ -28,7 +28,7 @@
public class NGramTokenizerValidator extends BasePolicyValidator {
private static final Set<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");
Review Comment:
[P2] Keep the validation ceiling out of analyzer identity
This property is persisted in the tokenizer policy, and
`AnalyzerIdentityBuilder.resolveComponentIdentity` stringifies the full
property map. The BE, however, uses `max_ngram_diff` only while validating
min/max and never stores it in `NGramTokenizer`. Thus policies
`(min=1,max=2,max_ngram_diff=1)` and the same policy with a ceiling of `2` emit
identical tokens but receive different identities, so both CREATE TABLE and
ALTER duplicate-index checks allow redundant indexes on one column. Please
canonicalize tokenizer identity from runtime-effective properties (excluding
`max_ngram_diff`) and cover both duplicate-index paths.
##########
fe/fe-core/src/main/java/org/apache/doris/indexpolicy/NGramTokenizerValidator.java:
##########
@@ -77,6 +77,24 @@ protected void validateSpecific(Map<String, String> props)
throws DdlException {
+ "cannot be smaller than min_gram [" + minGram + "]");
}
+ int maxNgramDiff = 1;
+ if (props.containsKey("max_ngram_diff")) {
+ try {
+ maxNgramDiff = Integer.parseInt(props.get("max_ngram_diff"));
Review Comment:
[P2] Canonicalize numeric text before sending it to BE
[`Integer.parseInt`](https://docs.oracle.com/en/java/javase/23/docs/api/java.base/java/lang/Integer.html#parseInt(java.lang.String))
accepts Unicode decimal digits, so `min_gram=1,max_gram=8,max_ngram_diff=٧`
passes this check. The parsed value remains local: `IndexPolicy` persists the
original string, `PushIndexPolicyTask` forwards it unchanged, and BE
`Settings::get_int` then calls `std::stoi` on its UTF-8 bytes and fails during
analyzer construction. DDL therefore succeeds while TOKENIZE/index use fails
later. Please require ASCII decimal syntax or rewrite the stored value to
`Integer.toString(maxNgramDiff)` before persistence, and add an end-to-end
parity test.
--
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]
