airborne12 commented on code in PR #67918:
URL: https://github.com/apache/doris/pull/67918#discussion_r4068728241
##########
be/src/storage/index/inverted/tokenizer/tokenizer.h:
##########
@@ -39,12 +45,148 @@ class DorisTokenizer : public Tokenizer, public
DorisTokenStream {
using Tokenizer::reset;
// Only use the parameterless reset method
- void reset() override { _in = _in_pending; };
+ void reset() override {
+ _in = _in_pending;
+ _source_byte_offsets.clear();
+ _source_byte_end_offsets.clear();
+ release_oversized_scratch(_source_byte_offsets);
+ release_oversized_scratch(_source_byte_end_offsets);
+ };
+
+ std::span<const int32_t> get_source_byte_offsets() const override {
+ return _source_byte_offsets_enabled ? std::span<const int32_t>
{_source_byte_offsets}
+ : std::span<const int32_t> {};
+ }
+
+ std::span<const int32_t> get_source_byte_end_offsets() const override {
+ return _source_byte_offsets_enabled ? std::span<const int32_t>
{_source_byte_end_offsets}
+ : std::span<const int32_t> {};
+ }
+
+ void set_source_byte_offsets_enabled(bool enabled) override {
+ _source_byte_offsets_enabled = enabled;
+ }
+
+ size_t source_byte_offsets_capacity_for_test() const {
+ return _source_byte_offsets.capacity() +
_source_byte_end_offsets.capacity();
+ }
protected:
+ int32_t correct_source_offset(int32_t offset) const {
+ const auto* char_filter = dynamic_cast<const
DorisCharFilter*>(_in.get());
+ return char_filter == nullptr ? offset :
char_filter->correct_offset(offset);
+ }
+
+ void set_source_byte_offsets(std::string_view term, int32_t source_start) {
+ set_source_byte_offsets(term, term, source_start);
+ }
+
+ void set_source_byte_offsets(std::string_view term, std::string_view
source,
+ int32_t source_start) {
+ _source_byte_offsets.clear();
+ _source_byte_end_offsets.clear();
+ if (!_source_byte_offsets_enabled) {
+ return;
+ }
+
+ const auto* char_filter = dynamic_cast<const
DorisCharFilter*>(_in.get());
+ const int32_t corrected_start =
+ char_filter == nullptr ? source_start :
char_filter->correct_offset(source_start);
+ std::vector<int32_t> source_offsets {0};
Review Comment:
Fixed in db18af01772. Reproduced first: with offsets enabled, a 26-rune
token followed by two-rune tokens left the tokenizer's published capacity at 32
and then 4, i.e. every token re-grew a fresh local vector and dropped the
previous buffer (same for IK, which moved its per-token vector into the publish
path).
`DorisTokenizer` now builds boundaries into a reusable
`_source_offsets_scratch` and `publish_source_byte_offsets()` swaps it with the
published vector in the strictly-increasing case (the widened case fills the
published vectors in place), so after two tokens every following token lands in
one of the two warmed buffers without allocating. IK's
`regularize_with_source_byte_offsets()` writes into its own reusable vector and
publishes through the same swap. The oversized-reset high-water release now
also covers the scratch vectors.
`TestOffsetTrackingReusesTokenizerScratchAcrossTokens` asserts the alternating
steady state for the standard and IK tokenizers, and the existing ASAN
allocation bound for a 4 MiB token still holds.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/invertedindex/AnalyzerIdentityBuilder.java:
##########
@@ -226,26 +671,115 @@ private static String resolveTokenFilterIdentity(String
filterList) {
* IMPORTANT: Order is preserved because filter order is semantically
significant.
*/
private static String resolveCharFilterIdentity(String filterList) {
+ return resolveCharFilterIdentity(filterList, false);
+ }
+
+ private static String resolveCharFilterIdentity(String filterList, boolean
lowercaseDownstream) {
if (Strings.isNullOrEmpty(filterList)) {
return "";
}
- StringBuilder sb = new StringBuilder();
+ ArrayDeque<String> identities = new ArrayDeque<>();
String[] filters = filterList.split(",\\s*");
// DO NOT sort - filter order is semantically significant
- for (int i = 0; i < filters.length; i++) {
- String filter = filters[i].trim();
- if (i > 0) {
- sb.append(",");
+ for (int i = filters.length - 1; i >= 0; --i) {
+ String filterName = filters[i].trim();
+ String filter = resolveComponentIdentity(
+ filterName, IndexPolicyTypeEnum.CHAR_FILTER,
lowercaseDownstream);
+ if (Strings.isNullOrEmpty(filter)) {
+ continue;
}
+ identities.addFirst(filter);
+ lowercaseDownstream = isCaseFoldingCharFilter(filterName);
Review Comment:
Fixed in f5d7670ec2f. Reproduced first: `lower_a,x_to_y,fold` still carried
`{pattern=A, replacement=a, ...}` in its identity while `x_to_y,fold` did not,
so both CREATE (`canHaveMultipleInvertedIndexes` returned true) and ALTER
admitted the pair.
The reverse walk now carries a fold context instead of a boolean: after a
case-folding filter the context is empty, a `char_replace` filter in between
adds the bytes it rewrites (its pattern), and any other filter ends the
context. `canonicalizeCharReplacePattern` drops `X -> x` only when neither `X`
nor `x` is rewritten between that filter and the fold, which is exactly when
the fold makes the mapping unobservable.
`testCaseFoldCarriesThroughNonInteractingCharReplace` covers the intervening
`x_to_y` case, keeps `lower_a,a_to_b,fold` distinct from `a_to_b,fold`, and
adds an `A -> z` interacting negative; the CREATE and ALTER duplicate cases are
in `InvertedIndexPropertiesTest` and `SchemaChangeHandlerTest`.
--
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]
