github-actions[bot] commented on code in PR #69761:
URL: https://github.com/apache/doris/pull/69761#discussion_r4422686036
##########
be/src/storage/tablet/tablet_schema.cpp:
##########
@@ -1645,6 +1646,13 @@ std::vector<TabletIndexPtr>
TabletSchema::inverted_index_by_field_pattern(
}
std::vector<const TabletIndex*> TabletSchema::inverted_indexs(const
TabletColumn& col) const {
Review Comment:
[P2] Keep ordinary VARIANT indexes discoverable during DROP
`IndexBuilder::update_inverted_index_info()` uses
`TabletSchema::inverted_indexs(column)` to find the index being dropped and
remove it from the output rowset schema. This PR makes that overload return
only values-root indexes for every direct VARIANT column, so an ordinary parent
index such as `idx_var2(v)` is never found even though DROP INDEX is allowed.
The output rowset retains the dropped index metadata and its V1 files are not
accounted/deleted. Restrict root-only filtering to query/root-writer selection
or make the DROP/index-builder path use the unfiltered UID/suffix lookup.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/BindScoreToOlapScan.java:
##########
@@ -216,6 +216,15 @@ private static void check(List<Plan> path, boolean
outermost) {
if (where.stream().noneMatch(conjunct ->
conjunct.containsType(Match.class, Search.class))) {
throw new AnalysisException(UNSUPPORTED_SELECT_ERROR);
}
+ for (Expression conjunct : where) {
+ for (Expression expression : conjunct.<Expression>collect(e -> e
instanceof Match)) {
+ Match match = (Match) expression;
+ if (match.left().getDataType().isVariantType()) {
Review Comment:
[P2] Limit the score restriction to root values indexes
This new check rejects every MATCH whose left expression is VARIANT,
including `v['obj']` bound to an ordinary parser-backed path index. The BE only
rejects scoring when `variant_root_index::is_root_index(...)` is true; path
indexes still use the existing collection-similarity scoring flow. Restrict the
FE guard to a direct root slot/root values index (for example, no subcolumn
path) so `score()` remains available for supported VARIANT path MATCH queries.
##########
be/src/storage/segment/variant/v2/variant_root_index_writer.cpp:
##########
@@ -0,0 +1,192 @@
+// 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/segment/variant/v2/variant_root_index_writer.h"
+
+#include "common/cast_set.h"
+#include "common/exception.h"
+#include "core/value/variant/variant_leaf_visitor.h"
+#include "storage/index/inverted/analysis/analyzer/analyzer.h"
+#include "storage/index/inverted/analysis/inverted_index_parser.h"
+#include "storage/index/inverted/variant_root_index.h"
+#include "storage/index/inverted/variant_term_codec.h"
+#include "storage/tablet/tablet_schema.h"
+
+namespace doris::segment_v2 {
+
+VariantRootIndexWriter::VariantRootIndexWriter(IndexFileWriter*
index_file_writer,
+ const TabletIndex* index_meta,
bool is_direct_load)
+ : _index_file_writer(index_file_writer),
+ _index_meta(index_meta),
+ _is_direct_load(is_direct_load) {}
+
+VariantRootIndexWriter::~VariantRootIndexWriter() {
+ close_on_error();
+}
+
+Status VariantRootIndexWriter::init() {
+ DORIS_CHECK(_index_file_writer != nullptr);
+ DORIS_CHECK(_index_meta != nullptr);
+ DORIS_CHECK(variant_root_index::is_root_index(*_index_meta));
+ if
(get_parser_phrase_support_string_from_properties(_index_meta->properties()) ==
+ INVERTED_INDEX_PARSER_PHRASE_SUPPORT_YES) {
+ return Status::Error<ErrorCode::INVERTED_INDEX_NOT_SUPPORTED>(
+ "VARIANT root index does not support phrase positions");
+ }
+ if
(!inverted_index::InvertedIndexAnalyzer::should_analyzer(_index_meta->properties()))
{
+ return Status::Error<ErrorCode::INVERTED_INDEX_NOT_SUPPORTED>(
+ "VARIANT values index requires a text analyzer");
+ }
+ _writer = std::make_unique<SniiIndexColumnWriter>(_index_file_writer,
_index_meta,
+
FieldType::OLAP_FIELD_TYPE_VARCHAR);
+ RETURN_IF_ERROR(_writer->init());
+ _writer->set_direct_load(_is_direct_load);
+ return Status::OK();
+}
+
+Status VariantRootIndexWriter::begin_document(bool sql_null) {
+ DORIS_CHECK(_writer != nullptr);
+ DORIS_CHECK(!_document_open);
+ _document_open = true;
+ _sql_null = sql_null;
+ _analyzed_values.clear();
+ _owned_texts.clear();
+ return Status::OK();
+}
+
+Status VariantRootIndexWriter::add_leaf(const VariantLeaf& leaf) {
+ DORIS_CHECK(_document_open);
+ DORIS_CHECK(!_sql_null);
+ if (leaf.kind == VariantLeafKind::OTHER) {
+ return Status::OK();
+ }
+ if (leaf.kind == VariantLeafKind::STRING) {
+ _analyzed_values.emplace_back(leaf.string_value.data,
leaf.string_value.size);
+ return Status::OK();
+ }
+ // Numbers and booleans are searchable by their canonical text, the same
text the scalar
+ // MATCH fallback tokenizes. Slices into _owned_texts are taken in
end_document(), after the
+ // vector stopped growing.
+ std::string text;
+ const bool has_text = canonical_leaf_text(leaf, &text);
+ DORIS_CHECK(has_text);
+ _owned_texts.push_back(std::move(text));
+ return Status::OK();
+}
+
+Status VariantRootIndexWriter::end_document() {
+ DORIS_CHECK(_document_open);
+ Status status;
+ if (_sql_null) {
+ status = _writer->add_nulls(1);
+ } else {
+ for (const std::string& text : _owned_texts) {
+ _analyzed_values.emplace_back(text);
+ }
+ status =
_writer->add_document(variant_term_codec::token_term_prefix(),
_analyzed_values);
+ }
+ _document_open = false;
+ return status;
+}
+
+// NOLINTNEXTLINE(readability-make-member-function-const): finishing the owned
SNII writer is a mutation.
+Status VariantRootIndexWriter::finish() {
+ DORIS_CHECK(_writer != nullptr);
+ DORIS_CHECK(!_document_open);
+ return _writer->finish();
+}
+
+void VariantRootIndexWriter::close_on_error() {
+ if (_writer != nullptr) {
+ _writer->close_on_error();
+ }
+}
+
+size_t VariantRootIndexWriter::size() const {
+ size_t result = _analyzed_values.capacity() * sizeof(Slice);
+ for (const std::string& text : _owned_texts) {
+ result += text.capacity();
+ }
+ return result;
+}
+
+Status append_variant_root_index_leaves(std::span<VariantRootIndexWriter*>
writers,
+ const VariantRef& value) {
+ for (VariantRootIndexWriter* writer : writers) {
+ DORIS_CHECK(writer != nullptr);
+ }
+ return visit_variant_leaves(value, [&](const VariantLeaf& leaf) {
+ for (VariantRootIndexWriter* writer : writers) {
+ RETURN_IF_ERROR(writer->add_leaf(leaf));
+ }
+ return Status::OK();
+ });
+}
+
+Status append_variant_root_indexes(std::span<VariantRootIndexWriter*> writers,
+ const ColumnVariantV2::ReadView& view,
size_t begin,
+ size_t length, std::span<const uint8_t>
outer_nulls) {
+ DORIS_CHECK(!writers.empty());
+ if (view.is_typed()) {
+ return Status::InvalidArgument(
+ "VARIANT root index requires encoded E-state input; caller
must ensure_encoded");
+ }
+ if (begin > view.size() || length > view.size() - begin) {
+ return Status::InvalidArgument("VARIANT root index range [{}, {})
exceeds input size {}",
+ begin, begin + length, view.size());
+ }
+ if (!outer_nulls.empty() && outer_nulls.size() != length) {
+ return Status::InvalidArgument(
+ "VARIANT root index outer-null span has {} rows, expected {}",
outer_nulls.size(),
+ length);
+ }
+ try {
+ for (size_t offset = 0; offset < length; ++offset) {
+ const bool outer_null = !outer_nulls.empty() &&
outer_nulls[offset] != 0;
+ for (VariantRootIndexWriter* writer : writers) {
+ DORIS_CHECK(writer != nullptr);
+ RETURN_IF_ERROR(writer->begin_document(outer_null));
+ }
+ if (!outer_null) {
+ RETURN_IF_ERROR(
+ append_variant_root_index_leaves(writers,
view.value_at(begin + offset)));
Review Comment:
[P1] Apply final-row visibility before root-only indexing
The `_root_only` writer calls `append_variant_root_indexes()` on the raw
Variant document, and that helper visits every leaf. A row such as
`{"a":"hidden","a.b":"visible"}` therefore publishes both terms even though the
final Variant assembler suppresses `a` when `a.b` exists. With the root index
enabled, `MATCH_ANY 'hidden'` can be accepted by the index and return a row
that scalar evaluation of the final logical value excludes. Reuse the
assembler’s visibility rules (including empty-array handling) for this direct
path, and cover the root-only writer separately from converted-layout
compaction.
##########
be/src/storage/segment/variant/v2/variant_column_writer.cpp:
##########
@@ -108,6 +115,9 @@ Status VariantV2ColumnWriter::init() {
Review Comment:
[P1] Strip root indexes before the root-only scalar writer
When a VARIANT column already has extracted children, `_root_only` is true
and this branch constructs `ScalarColumnWriter` with `_opts`. The PR now makes
`_opts.inverted_indexes` include the VARIANT values index, so
`ScalarColumnWriter::init()` calls `IndexColumnWriter::create()` for
`TYPE_VARIANT`; the SNII factory rejects that type and the rowset write fails
before `VariantRootIndexWriter` can build the values index. This occurs after
extracted paths have been materialized (for example during later compaction).
Pass root-only storage options with `inverted_indexes` cleared and
`need_inverted_index=false`, as `_write_root()` already does for the
non-root-only path.
--
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]
