github-actions[bot] commented on code in PR #67917: URL: https://github.com/apache/doris/pull/67917#discussion_r4000105184
########## regression-test/suites/inverted_index_p0/analyzer/test_ngram_max_diff_custom_analyzer.groovy: ########## @@ -0,0 +1,72 @@ +// 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}") + + def ngramTokens = sql """SELECT TOKENIZE('abcdefgh', '"analyzer"="${ngramAnalyzer}"')""" + def ngramTokenString = ngramTokens[0][0].toString() + assertTrue(ngramTokenString.contains('"token": "abcdefgh"')) Review Comment: [P2] Check the complete deterministic n-gram result For `abcdefgh` and sizes 1..8 the result has 36 deterministic tokens, but these two substring checks still pass if all size-5/6 tokens, the other size-7 token, duplicates, or most of the output are wrong. The BE unit case uses only `abcd`, so it never exercises sizes 5..8 either. Please record this query with `qt_*` and its generated `.out` result (as required for determined regression results) so the whole sequence is verified. ########## 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] Bound the newly enabled token fan-out This accepts ranges such as `min_gram=1`, `max_gram=50000`, and `max_ngram_diff=49999`. A 50,000-code-point value then emits `1 + ... + 50000 = 1,250,025,000` n-grams; `TOKENIZE` materializes them and index writers consume the same factory, with no token-count, resource, or cancellation guard. The old fixed difference rejected this low-minimum range. Please define and enforce a practical resource bound in both FE and BE, and add boundary tests, before persisting the policy. -- 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]
