eldenmoon commented on code in PR #69761:
URL: https://github.com/apache/doris/pull/69761#discussion_r4432203638
##########
be/src/exec/common/variant_util.cpp:
##########
@@ -1152,19 +1156,55 @@ void
VariantCompactionUtil::get_compaction_subcolumns_from_data_types(
append_empty_key_subcolumn_from_stats(paths_set_info, parent_column,
output_schema);
}
+bool VariantCompactionUtil::has_values_index(const TabletSchema& schema) {
+ return std::ranges::any_of(schema.inverted_indexes(), [](const
TabletIndex* index) {
+ return index->is_variant_values_index() &&
!index->col_unique_ids().empty();
+ });
+}
+
+Status VariantCompactionUtil::plan_values_index_compaction(
+ const std::vector<RowsetSharedPtr>& rowsets, bool expand_subcolumns,
+ const std::set<std::pair<int32_t, int64_t>>& native_indexes,
TabletSchemaSPtr& schema,
+ VariantCompactionPathsSPtr* paths) {
+ std::set<int32_t> rebuild_roots;
+ for (const auto* index : schema->inverted_indexes()) {
+ // Replayed indexes without column IDs are skipped by index compaction
as well.
+ if (index->col_unique_ids().empty()) {
+ continue;
+ }
+ if (index->is_variant_values_index() &&
+ !native_indexes.contains({index->col_unique_ids().front(),
index->index_id()})) {
Review Comment:
Not reachable. Subcolumn writers inherit ordinary path indexes through the
root UID (`variant_writer_helpers.cpp:387` →
`TabletSchema::inverted_indexs(uid, path)`), and `inherit_index` excludes only
the values index (`variant_util.cpp:1651`); this lookup is unchanged from
master. With `enable_match_without_inverted_index=false`, `v['s'] MATCH_ANY`
still runs on `path_idx` after full compaction in all four native/rebuild ×
vertical-expansion on/off combinations, as does the same table without a values
index. `test_variant_values_index_compaction` now asserts this.
##########
be/src/storage/segment/variant/v2/variant_shredder.cpp:
##########
@@ -613,6 +617,88 @@ struct VariantShredder::Impl {
return publish_materialized(selection, candidate_builders, result);
}
+ // Typed paths are indexed only after the same conversion used by storage.
Merge compact
+ // rowids instead of scanning every path for every row or reconstructing
complete objects.
+ //
NOLINTNEXTLINE(readability-function-size,readability-function-cognitive-complexity,readability-make-member-function-const):
keep encoded leaf ownership and synchronous writer consumption in one row
scope; the root index writers are mutated.
+ Status write_converted_root_indexes(const VariantShreddedColumns& result,
+ const
DorisVector<VariantPathBuilder*>& builders) {
+ using namespace variant_v2::variant_assembler_detail;
+ using Position = std::pair<size_t, size_t>;
+ std::priority_queue<Position, DorisVector<Position>, std::greater<>>
pending;
+ DorisVector<PreparedMaterializedColumn> values;
+ DorisVector<size_t> positions(builders.size(), 0);
+ values.reserve(builders.size());
+ for (size_t index = 0; index < builders.size() &&
+ options.physical_layout !=
VariantShredderPhysicalLayout::DOC;
+ ++index) {
+ const auto* builder = builders[index];
+ if (builder->rowids().empty()) {
+ values.emplace_back();
+ continue;
+ }
+ values.push_back(prepare_materialized_column(builder->type(),
builder->column().get(),
+
builder->rowids().size()));
+ pending.emplace(builder->rowids().front(), index);
+ }
+ const auto& roots = assert_cast<const
ColumnString&>(*result.root_jsonb);
+ DorisVector<size_t> row_paths;
+ row_paths.reserve(builders.size() + 1);
+ for (size_t row = 0; row < rows; ++row) {
+ row_paths.clear();
+ VariantBatchBuilder encoded;
+ if (!index_nulls[row] && !roots.get_data_at(row).empty()) {
+ auto output = encoded.begin_row();
+ jsonb_to_variant(roots.get_data_at(row), output, 0);
+ output.finish();
+ row_paths.push_back(builders.size());
+ }
+ if (!index_nulls[row] &&
+ options.physical_layout == VariantShredderPhysicalLayout::DOC)
{
+ // DOC root reads the finalized binary buckets, not converted
typed sidecars.
+ for (const auto& bucket : result.binary_buckets) {
+ const auto& map = assert_cast<const
ColumnMap&>(*bucket.column);
+ const auto& cells = assert_cast<const
ColumnString&>(map.get_values());
+ const auto& offsets = map.get_offsets();
+ const size_t begin = row == 0 ? 0 : offsets[row - 1];
+ for (size_t cell = begin; cell < offsets[row]; ++cell) {
+ auto output = encoded.begin_row();
+
RETURN_IF_ERROR(variant_v2::append_v1_storage_cell(cells.get_data_at(cell),
+
output, 0));
+ output.finish();
+ row_paths.push_back(builders.size());
+ }
+ }
+ }
+ while (!pending.empty() && pending.top().first == row) {
+ const size_t index = pending.top().second;
+ pending.pop();
+ const size_t position = positions[index]++;
+ DCHECK(!index_nulls[row]);
+ auto output = encoded.begin_row();
+ RETURN_IF_ERROR(append_materialized_value(values[index],
position, output, 0));
+ output.finish();
+ row_paths.push_back(index);
+ if (positions[index] < builders[index]->rowids().size()) {
+
pending.emplace(builders[index]->rowids()[positions[index]], index);
+ }
+ }
+ // Root token values borrow bytes until end_document; retain all
encoded leaves
+ // for this row through synchronous SNII consumption.
+ auto batch = encoded.finish_batch();
+ for (auto* writer : options.root_index_writers) {
+ RETURN_IF_ERROR(writer->begin_document(index_nulls[row] != 0));
+ }
+ for (size_t leaf = 0; leaf < row_paths.size(); ++leaf) {
+
RETURN_IF_ERROR(append_variant_root_index_leaves(options.root_index_writers,
Review Comment:
Already fixed: the values writer indexes `VariantAssembler` output, not the
shredded leaves (`variant_column_writer.cpp:340`). For
`{"a":"hidden","a.b":"visible"}`, indexed and forced-scan filtering agree in
materialized, sparse and DOC layouts, including `NOT`;
`test_variant_values_index_query` now asserts the WHERE results for both.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzer.java:
##########
@@ -1019,9 +1020,14 @@ public Expression visitMatch(Match match,
ExpressionRewriteContext context) {
right.toSql(), match.toSql(), right.getDataType()));
}
- if (left.getDataType().isVariantType()) {
- left = new Cast(left, right.getDataType());
+ // Only direct roots use leaf-token semantics. Ordinary object
subpaths keep their
+ // existing JSON-to-STRING scalar semantics, including when pruning is
disabled.
+ if (left.getDataType().isVariantType()
+ && (!(left instanceof SlotReference) || ((SlotReference)
left).hasSubColPath()
Review Comment:
Not reachable at this head. An alias of `v['obj']` does not inherit the
root's column metadata, and an alias of a pruned subpath slot keeps its
subpath, so `isVariantRootSlot` rejects both and the MATCH keeps master's
STRING cast (`ExpressionAnalyzer.java:1051`). The bot's query returns the same
rows as direct `v['obj'] MATCH_ANY` with nested-column pruning and index
queries on and off; FE UT and the query suite cover both alias forms.
--
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]
