airborne12 commented on code in PR #67538:
URL: https://github.com/apache/doris/pull/67538#discussion_r4000025919
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/InvertedIndexUtil.java:
##########
@@ -353,12 +358,70 @@ private static void checkAnalyzerName(String
analyzerName, PrimitiveType colType
+ " is not supported for column of type " + colType);
}
try {
-
Env.getCurrentEnv().getIndexPolicyMgr().validateAnalyzerExists(analyzerName);
+ IndexPolicyMgr indexPolicyMgr =
Env.getCurrentEnv().getIndexPolicyMgr();
+ indexPolicyMgr.validateAnalyzerExists(analyzerName);
+ // Gram-family analyzer (an ngram tokenizer carrying mode, see
+ // IndexPolicyMgr#resolveGramTokenizerMode): BE builds
sparse/dense gram postings for it
+ // only on SNII, and those postings carry no positions, so phrase
queries are impossible.
+ Optional<String> gramMode =
indexPolicyMgr.resolveGramTokenizerMode(analyzerName);
+ if (gramMode.isPresent()) {
+ if (colType.isArrayType()) {
+ throw new AnalysisException("gram tokenizer (mode=" +
gramMode.get()
+ + ") analyzer '" + analyzerName + "' does not
support ARRAY columns");
+ }
+ if (!colType.isCharFamily()) {
+ throw new AnalysisException("gram tokenizer (mode=" +
gramMode.get()
+ + ") analyzer '" + analyzerName
+ + "' is supported only on scalar CHAR, VARCHAR, or
STRING columns");
+ }
+ if (storageFormat != TInvertedIndexFileStorageFormat.SNII) {
+ throw new AnalysisException("gram tokenizer (mode=" +
gramMode.get()
+ + ") requires inverted_index_storage_format =
SNII");
+ }
+ if ("true".equals(supportPhrase)) {
+ throw new AnalysisException(
+ "gram tokenizer index does not support phrase
(support_phrase must be false)");
+ }
+ }
} catch (DdlException e) {
throw new AnalysisException("Invalid custom analyzer: " +
e.getMessage());
}
}
+ /**
+ * Constraints a gram-family analyzer (an ngram tokenizer carrying mode)
enforces at the index
+ * property level:
+ * 1) an index-level char_filter (char_filter_type/pattern/replacement)
conflicts with the
+ * semantics of gram split boundaries (the character replacement
happens after the tokenizer
+ * has already split by the gram rule, which breaks reproducibility),
so it is rejected;
+ * 2) when support_phrase is not given explicitly it defaults to "false",
overriding the general
+ * rule of the {@link Index} constructor that "an analyzer implies
true" -- a gram index is
+ * forced to docs-only on the BE side and has no positions for a phrase
query to use.
+ *
+ * <p>The {@code properties} held by the caller ({@link
#checkInvertedIndexProperties}) and the
+ * {@link IndexDefinition} field are the same mutable Map reference, so
the defaults written
+ * here are seen when {@code IndexDefinition#translateToCatalogStyle}
builds the {@link Index}.
+ */
+ private static void applyGramFamilyIndexDefaults(String analyzerName,
Map<String, String> properties)
+ throws AnalysisException {
+ if (analyzerName == null || analyzerName.isEmpty()) {
+ return;
+ }
+ Optional<String> gramMode =
Env.getCurrentEnv().getIndexPolicyMgr().resolveGramTokenizerMode(analyzerName);
+ if (!gramMode.isPresent()) {
+ return;
+ }
+ if (properties.get(INVERTED_INDEX_PARSER_CHAR_FILTER_TYPE) != null
+ || properties.get(INVERTED_INDEX_PARSER_CHAR_FILTER_PATTERN)
!= null
+ ||
properties.get(INVERTED_INDEX_PARSER_CHAR_FILTER_REPLACEMENT) != null) {
+ throw new AnalysisException("char_filter cannot be used with gram
tokenizer (mode="
+ + gramMode.get() + ")");
+ }
+ if (properties.get(INVERTED_INDEX_SUPPORT_PHRASE_KEY) == null) {
+ properties.put(INVERTED_INDEX_SUPPORT_PHRASE_KEY, "false");
Review Comment:
Confirmed and fixed in 98ec7f53029.
Reproduced on a live cluster: a gram index (docs-only by default) declared
before an analyzer index with `support_phrase=true`. `MATCH_PHRASE` and
`MATCH_PHRASE_PREFIX ... USING ANALYZER <positional>` failed with `[E-6000]
phrase queries require setting support_phrase = true` with the index on and
returned `[1,4]` with it off; declaring the indexes the other way round made
both pass. An ordinary english index with `support_phrase=false` in front fails
the same way, and master's `match.cpp` carries the identical check, so the
defect predates this PR -- the PR makes it easy to hit, because a gram index is
docs-only by default.
The check now asks `select_best_reader` for the reader `read_from_index`
will run the query on -- same column type, query type and analyzer key -- and
judges phrase support on that reader, when it is a FULLTEXT one, so a phrase
query over a lone untokenized index keeps its previous behaviour. Selection is
a pure function of those inputs and the readers, so the reader checked is the
reader used.
Tests:
`FunctionMatchTest.phrase_support_is_checked_on_the_index_the_analyzer_selects`
(docs-only reader first, positional reader second: the positional one serves
MATCH_PHRASE and MATCH_PHRASE_PREFIX, and the docs-only one is still refused
when named), and the phrase half of `test_gram_index_order`, with a gram index
and with an english docs-only index in front. The unit test fails against the
previous check with that same E-6000; the suite passes on the fixed build.
##########
be/src/storage/index/snii/snii_index_writer.cpp:
##########
@@ -377,6 +638,7 @@ void SniiIndexColumnWriter::close_on_error() {
// Balance the observation-tracker mirror before dropping the reporter.
_report_null_docids_capacity(/*release_all=*/true);
_report_encoded_norms_capacity(/*release_all=*/true);
+ _report_gram_buffers_capacity(/*release_all=*/true);
_memory_reporter.reset();
Review Comment:
Confirmed by reading the code; not changed, because the effect is bounded
and self-healing.
`close_on_error()` settles the null-docid, norms and gram-buffer charges but
not the retained density sample, so `~MemoryReporter` drains up to
`gram_index_density_sample_bytes` (4 MiB by default) plus the last row from the
process-wide counter while those bytes stay alive until the column writer is
destroyed, and logs a warning. That drain is the designed end state for
transient accounting abandoned on an error path; a load cancelled before
`finish()` destroys the writer without `close_on_error()` at all and settles
every buffer's charge the same way. The window is the teardown of a load that
has already failed, and the share it can understate is floored at `max(10% of
the process limit, 4 x 512 MiB)`, so the skew stays under 0.2% of the smallest
share.
Reaching it needs `_add_value_tokens` to throw during replay; the gram lane
is not CLucene code, so in practice that means an allocation failure. I could
not make that happen on a running cluster without an injection point, so this
one is confirmed from the code, not reproduced.
##########
be/src/storage/index/snii/writer/logical_index_writer.cpp:
##########
@@ -505,6 +562,93 @@ struct LogicalIndexWriter::BlockState {
// Out-of-line so unique_ptr<BlockState> sees the complete type (see header).
LogicalIndexWriter::~LogicalIndexWriter() = default;
+// Offers one term to the high-df digest. The digest keeps the highest-df
terms seen, as a
+// min-heap on df so the cheapest entry to evict is always at the front, which
makes the
+// whole pass O(terms log K) time and O(K) space -- no second sort over the
vocabulary.
+//
+// Terms below the floor are dropped outright: the digest exists to let a
query GIVE UP
+// early, and a term rare enough to sit below the floor is one no cost gate
would give up on.
+void LogicalIndexWriter::note_high_df_term(uint64_t term_hash, uint32_t df) {
+ // A zero floor means no digest for this index (see high_df_floor_for),
and it has to be
+ // tested separately: `df < 0` is never true, so the comparison below
would let every
+ // term through on exactly the indexes that want none.
+ if (high_df_floor_ == 0 || df < high_df_floor_) {
+ return;
+ }
+ const auto by_df = [](const HighDfEntry& lhs, const HighDfEntry& rhs) {
+ return lhs.df > rhs.df; // greater-than gives a MIN-heap on df
+ };
+ if (high_df_terms_.size() < format::kMaxHighDfDigestTerms) {
+ high_df_terms_.push_back({df, term_hash});
Review Comment:
Confirmed by reading the code; not changed, because the amount does not move
any limit.
`HighDfEntry` is 16 bytes aligned and the heap is capped at 4,096 entries:
64 KiB retained per logical gram index, plus roughly 112 KiB of transient
copies while `finish_high_df_digest` runs. Those bytes are charged to the
task's allocator tracker and visible to the limiter's process-level signals
(soft limit, available memory); what they miss is the SNII build-share term
only. That share is `max(10% of the process limit, 4 x 512 MiB)`, so leaving
out even 1% of the smallest share takes about 320 gram indexes holding a full
digest at the same moment, each on a segment with at least 4,096 terms above
the high-df floor. Pre-charging would add reservation plumbing to the
term-emission path for an error below what the limiter can resolve.
--
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]
