airborne12 commented on code in PR #68758:
URL: https://github.com/apache/doris/pull/68758#discussion_r4265609116


##########
be/src/storage/index/inverted/query/logical/match_lowering.cpp:
##########
@@ -0,0 +1,163 @@
+// 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.
+
+#include "storage/index/inverted/query/logical/match_lowering.h"
+
+#include <algorithm>
+#include <cctype>
+#include <charconv>
+#include <string_view>
+#include <utility>
+
+#include "storage/index/inverted/query/logical/lowering_common.h"
+
+namespace doris::index_query::logical {
+
+using segment_v2::InvertedIndexQueryType;
+using detail::flatten;
+using detail::lower_phrase;
+using detail::lower_phrase_prefix;
+using detail::lower_phrase_edge;
+
+namespace {
+
+// Removes a trailing " ~N" or " ~N+" and returns its slop and ordering rule.
+std::pair<int32_t, bool> take_slop(std::string_view* value) {
+    const size_t space = value->find_last_of(' ');
+    if (space == std::string_view::npos || value->substr(space + 1, 1) != "~") 
{
+        return {};
+    }
+    std::string_view digits = value->substr(space + 2);
+    const bool ordered = digits.size() > 1 && digits.back() == '+';
+    if (ordered) {
+        digits.remove_suffix(1);
+    }
+    int32_t slop = 0;
+    if (digits.empty() ||
+        !std::ranges::all_of(digits, [](unsigned char c) { return 
std::isdigit(c) != 0; }) ||
+        std::from_chars(digits.data(), digits.data() + digits.size(), slop).ec 
!= std::errc()) {
+        return {};
+    }
+    *value = value->substr(0, space);
+    return {slop, ordered};
+}
+
+} // namespace
+
+Status lower_match(InvertedIndexQueryType query_type, std::string_view value,
+                   const AnalyzeValue& analyze, Node* out) {
+    DCHECK(out != nullptr);
+    switch (query_type) {
+    case InvertedIndexQueryType::MATCH_REGEXP_QUERY:
+    case InvertedIndexQueryType::WILDCARD_QUERY:
+        out->value = Expand {.field = {},
+                             .kind = query_type == 
InvertedIndexQueryType::MATCH_REGEXP_QUERY
+                                             ? ExpandKind::kRegexp
+                                             : ExpandKind::kWildcard,
+                             .pattern = std::string(value)};
+        return Status::OK();
+    case InvertedIndexQueryType::EQUAL_QUERY:
+    case InvertedIndexQueryType::MATCH_ANY_QUERY:
+    case InvertedIndexQueryType::MATCH_ALL_QUERY:
+    case InvertedIndexQueryType::MATCH_PHRASE_QUERY:
+    case InvertedIndexQueryType::MATCH_PHRASE_PREFIX_QUERY:
+    case InvertedIndexQueryType::MATCH_PHRASE_EDGE_QUERY:
+        break;
+    default:
+        return Status::Error<ErrorCode::INVERTED_INDEX_NOT_SUPPORTED>(
+                "no index query lowers query type {}", 
query_type_to_string(query_type));
+    }
+    const auto [slop, ordered] = query_type == 
InvertedIndexQueryType::MATCH_PHRASE_QUERY
+                                         ? take_slop(&value)

Review Comment:
   Fixed in 522ca4c55c9, keeping the keyword slop syntax.
   
   Keyword indexes read a trailing ` ~N` or ` ~N+` as the phrase slop and look 
up the rest as one term. SNII already did this on master (`parse_phrase_slop` 
ran before its keyword branch), and this PR applies the same rule to V2 keyword 
indexes through the shared lowering. Row evaluation was the only path that kept 
the whole value.
   
   `FunctionMatchPhrase::execute_match` now removes the suffix with the parser 
the lowering uses (`logical::take_slop`, now declared in `match_lowering.h`) 
when the column is not analyzed. That is the same condition 
`TextIndexReader::_lower` uses to choose the keyword branch: 
`analyzer_ctx->requires_analysis()` is false. Analyzed columns are unchanged.
   
   `KeywordMatchPhraseTakesTheSlop` now runs each query through the V2 keyword 
index and through `FunctionMatchPhrase::execute_match` on the same values 
(`apple`, `apple ~1`, `banana`, `apple ~x`) and expects the same rows:
   
   | Query | Rows |
   |---|---|
   | `apple ~1` | 0 |
   | `apple ~1+` | 0 |
   | `apple ~x` (not a slop) | 3 |
   
   Before the change the function path returned row 1 for `apple ~1` and no 
rows for `apple ~1+`.
   



-- 
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]

Reply via email to