From 15eb86276b5d41f972288b5aa033215263f24696 Mon Sep 17 00:00:00 2001 From: Hu Shenggang Date: Tue, 30 Jun 2026 18:45:10 +0800 Subject: [PATCH] [fix](be) Fix short-circuit row store nested access path read ### What problem does this PR solve? Issue Number: None Problem Summary: Short-circuit point queries on row-store Unique Key MOW tables could deserialize full row-store complex column payloads into nested access-path pruned slots. The serialized layouts do not match and could crash the backend while reading nested struct/map/string fields. This change keeps access-path slots on the column-store rowid path and propagates their access paths to the segment iterator, while other row-store columns continue to be read from row store. ### Release note None ### Check List (For Author) - Test: - Regression test: doris-local-regression.sh --network 10.26.20.3/24 all -d point_query_p0 -s test_short_circuit_rowstore_nested_complex -forceGenOut - Regression test: doris-local-regression.sh --network 10.26.20.3/24 all -d point_query_p0 -s test_short_circuit_rowstore_nested_complex - Format check: build-support/check-format.sh - Diff check: git diff --check - Static analysis: build-support/run-clang-tidy.sh --build-dir be/build_Release (not passed in local environment due existing clang-tidy/header setup issues) - Behavior changed: No - Does this need documentation: No --- be/src/service/point_query_executor.cpp | 155 ++++++++++++------ ..._short_circuit_rowstore_nested_complex.out | 9 + ...ort_circuit_rowstore_nested_complex.groovy | 131 +++++++++++++++ 3 files changed, 245 insertions(+), 50 deletions(-) create mode 100644 regression-test/data/point_query_p0/test_short_circuit_rowstore_nested_complex.out create mode 100644 regression-test/suites/point_query_p0/test_short_circuit_rowstore_nested_complex.groovy diff --git a/be/src/service/point_query_executor.cpp b/be/src/service/point_query_executor.cpp index 8e3e83cfd46654..ef0c60f52e39b8 100644 --- a/be/src/service/point_query_executor.cpp +++ b/be/src/service/point_query_executor.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include "cloud/cloud_tablet.h" @@ -87,25 +88,70 @@ static void get_missing_and_include_cids(const TabletSchema& schema, std::unordered_set& include_cids) { missing_cids.clear(); include_cids.clear(); + std::unordered_set access_path_cids; for (auto* slot : slots) { - missing_cids.insert(slot->col_unique_id()); + const int cid = slot->col_unique_id(); + missing_cids.insert(cid); + if (!slot->all_access_paths().empty() || !slot->predicate_access_paths().empty()) { + access_path_cids.insert(cid); + } } // insert delete sign column id - missing_cids.insert(schema.columns()[schema.delete_sign_idx()]->unique_id()); + const int delete_sign_cid = schema.columns()[schema.delete_sign_idx()]->unique_id(); + missing_cids.insert(delete_sign_cid); if (target_rs_column_id == -1) { // no row store columns return; } const TabletColumn& target_rs_column = schema.column_by_uid(target_rs_column_id); DCHECK(target_rs_column.is_row_store_column()); - // The full column group is considered a full match, thus no missing cids + // The full column group is considered a full match, thus no missing cids. if (schema.row_columns_uids().empty()) { missing_cids.clear(); + } else { + for (int cid : schema.row_columns_uids()) { + missing_cids.erase(cid); + include_cids.insert(cid); + } + } + + if (!access_path_cids.empty()) { + // Row store keeps complex columns in their full serialized layout, while nested + // access-path slots can use a pruned layout. Read those slots from column store. + if (schema.row_columns_uids().empty()) { + // Empty include_cids means "read all columns" for jsonb_to_columns(). Switch to + // an explicit include set before excluding access-path columns. + for (auto* slot : slots) { + include_cids.insert(slot->col_unique_id()); + } + include_cids.insert(delete_sign_cid); + } + for (int cid : access_path_cids) { + missing_cids.insert(cid); + include_cids.erase(cid); + } + } +} + +static void set_slot_access_paths(const SlotDescriptor& slot, const TabletSchema& schema, + StorageReadOptions& storage_read_options) { + int32_t unique_id = slot.col_unique_id(); + const int field_index = + unique_id >= 0 ? schema.field_index(unique_id) : schema.field_index(slot.col_name()); + if (field_index >= 0) { + const auto& column = schema.column(field_index); + unique_id = column.unique_id() >= 0 ? column.unique_id() : column.parent_unique_id(); + } + if (unique_id < 0) { return; } - for (int cid : schema.row_columns_uids()) { - missing_cids.erase(cid); - include_cids.insert(cid); + + if (!slot.all_access_paths().empty()) { + storage_read_options.all_access_paths[unique_id] = slot.all_access_paths(); + } + + if (!slot.predicate_access_paths().empty()) { + storage_read_options.predicate_access_paths[unique_id] = slot.predicate_access_paths(); } } @@ -465,7 +511,7 @@ Status PointQueryExecutor::_lookup_row_key() { std::vector> segment_caches(specified_rowsets.size()); for (size_t i = 0; i < _row_read_ctxs.size(); ++i) { RowLocation location; - if (!config::disable_storage_row_cache) { + if (!config::disable_storage_row_cache && _reusable->missing_col_uids().empty()) { RowCache::CacheHandle cache_handle; auto hit_cache = RowCache::instance()->lookup( {_tablet->tablet_id(), _row_read_ctxs[i]._primary_key}, &cache_handle); @@ -496,6 +542,53 @@ Status PointQueryExecutor::_lookup_row_key() { return Status::OK(); } +static Status lookup_missing_columns_by_rowid(const BaseTabletSPtr& tablet, Reusable& reusable, + Metrics& profile_metrics, + OlapReaderStatistics* read_stats, + const RowLocation& row_loc, + MutableColumns& result_columns) { + if (!reusable.runtime_state()->enable_short_circuit_query_access_column_store()) { + std::string missing_columns; + for (int cid : reusable.missing_col_uids()) { + missing_columns += tablet->tablet_schema()->column_by_uid(cid).name() + ","; + } + return Status::InternalError( + "Not support column store, set store_row_column=true or " + "row_store_columns in table properties, missing columns: " + + missing_columns + " should be added to row store"); + } + + // fill missing columns by column store + BetaRowsetSharedPtr rowset = + std::static_pointer_cast(tablet->get_rowset(row_loc.rowset_id)); + SegmentCacheHandle segment_cache; + { + SCOPED_TIMER(&profile_metrics.load_segment_data_stage_ns); + RETURN_IF_ERROR(SegmentLoader::instance()->load_segments(rowset, &segment_cache, true)); + } + // find segment + auto it = + std::find_if(segment_cache.get_segments().cbegin(), segment_cache.get_segments().cend(), + [&](const segment_v2::SegmentSharedPtr& seg) { + return seg->id() == row_loc.segment_id; + }); + const auto& segment = *it; + for (int cid : reusable.missing_col_uids()) { + int pos = reusable.get_col_uid_to_idx().at(cid); + std::vector row_ids {static_cast(row_loc.row_id)}; + auto& column = result_columns[pos]; + std::unique_ptr iter; + SlotDescriptor* slot = reusable.tuple_desc()->slots()[pos]; + StorageReadOptions storage_read_options; + storage_read_options.stats = read_stats; + storage_read_options.io_ctx.reader_type = ReaderType::READER_QUERY; + set_slot_access_paths(*slot, *tablet->tablet_schema(), storage_read_options); + RETURN_IF_ERROR(segment->seek_and_read_by_rowid(*tablet->tablet_schema(), slot, row_ids, + column, storage_read_options, iter)); + } + return Status::OK(); +} + Status PointQueryExecutor::_lookup_row_data() { // 3. get values SCOPED_TIMER(&_profile_metrics.lookup_data_ns); @@ -518,7 +611,8 @@ Status PointQueryExecutor::_lookup_row_data() { std::string value; // fill block by row store if (_reusable->rs_column_uid() != -1) { - bool use_row_cache = !config::disable_storage_row_cache; + bool use_row_cache = + !config::disable_storage_row_cache && _reusable->missing_col_uids().empty(); RETURN_IF_ERROR(_tablet->lookup_row_data( _row_read_ctxs[i]._primary_key, _row_read_ctxs[i]._row_location.value(), *(_row_read_ctxs[i]._rowset_ptr), _profile_metrics.read_stats, value, @@ -530,48 +624,9 @@ Status PointQueryExecutor::_lookup_row_data() { _reusable->get_col_default_values(), _reusable->include_col_uids())); } if (!_reusable->missing_col_uids().empty()) { - if (!_reusable->runtime_state()->enable_short_circuit_query_access_column_store()) { - std::string missing_columns; - for (int cid : _reusable->missing_col_uids()) { - missing_columns += - _tablet->tablet_schema()->column_by_uid(cid).name() + ","; - } - return Status::InternalError( - "Not support column store, set store_row_column=true or " - "row_store_columns in table properties, missing columns: " + - missing_columns + " should be added to row store"); - } - // fill missing columns by column store - RowLocation row_loc = _row_read_ctxs[i]._row_location.value(); - BetaRowsetSharedPtr rowset = std::static_pointer_cast( - _tablet->get_rowset(row_loc.rowset_id)); - SegmentCacheHandle segment_cache; - { - SCOPED_TIMER(&_profile_metrics.load_segment_data_stage_ns); - RETURN_IF_ERROR( - SegmentLoader::instance()->load_segments(rowset, &segment_cache, true)); - } - // find segment - auto it = std::find_if(segment_cache.get_segments().cbegin(), - segment_cache.get_segments().cend(), - [&](const segment_v2::SegmentSharedPtr& seg) { - return seg->id() == row_loc.segment_id; - }); - const auto& segment = *it; - for (int cid : _reusable->missing_col_uids()) { - int pos = _reusable->get_col_uid_to_idx().at(cid); - std::vector row_ids { - static_cast(row_loc.row_id)}; - auto& column = result_columns[pos]; - std::unique_ptr iter; - SlotDescriptor* slot = _reusable->tuple_desc()->slots()[pos]; - StorageReadOptions storage_read_options; - storage_read_options.stats = &_read_stats; - storage_read_options.io_ctx.reader_type = ReaderType::READER_QUERY; - RETURN_IF_ERROR(segment->seek_and_read_by_rowid(*_tablet->tablet_schema(), slot, - row_ids, column, - storage_read_options, iter)); - } + RETURN_IF_ERROR(lookup_missing_columns_by_rowid( + _tablet, *_reusable, _profile_metrics, &_read_stats, + _row_read_ctxs[i]._row_location.value(), result_columns)); } } if (result_columns.size() > _reusable->include_col_uids().size()) { diff --git a/regression-test/data/point_query_p0/test_short_circuit_rowstore_nested_complex.out b/regression-test/data/point_query_p0/test_short_circuit_rowstore_nested_complex.out new file mode 100644 index 00000000000000..edc32152a8f887 --- /dev/null +++ b/regression-test/data/point_query_p0/test_short_circuit_rowstore_nested_complex.out @@ -0,0 +1,9 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !normal_path -- +5 8 8 \N \N 3 false + +-- !short_circuit_path -- +5 8 8 \N \N 3 false + +-- !short_circuit_path_repeat -- +5 8 8 \N \N 3 false diff --git a/regression-test/suites/point_query_p0/test_short_circuit_rowstore_nested_complex.groovy b/regression-test/suites/point_query_p0/test_short_circuit_rowstore_nested_complex.groovy new file mode 100644 index 00000000000000..d9425a6a3b0b4e --- /dev/null +++ b/regression-test/suites/point_query_p0/test_short_circuit_rowstore_nested_complex.groovy @@ -0,0 +1,131 @@ +// 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. + +suite("test_short_circuit_rowstore_nested_complex", "p0,nonConcurrent") { + def backendId_to_backendIP = [:] + def backendId_to_backendHttpPort = [:] + getBackendIpHttpPort(backendId_to_backendIP, backendId_to_backendHttpPort) + def set_be_config = { key, value -> + for (String backend_id : backendId_to_backendIP.keySet()) { + def (code, out, err) = update_be_config(backendId_to_backendIP.get(backend_id), + backendId_to_backendHttpPort.get(backend_id), key, value) + logger.info("update config: code=" + code + ", out=" + out + ", err=" + err) + } + } + + try { + set_be_config.call("disable_storage_row_cache", "false") + + sql "SET enable_nereids_planner=true" + sql "SET enable_sql_cache=false" + sql "SET enable_snapshot_point_query=true" + sql "SET enable_short_circuit_query_access_column_store=true" + + sql "DROP TABLE IF EXISTS short_circuit_rowstore_nested_complex" + sql """ + CREATE TABLE short_circuit_rowstore_nested_complex ( + pk INT, + deep STRUCT< + nested_str: VARCHAR(64), + inner_s: STRUCT, + deep_arr: ARRAY>, + deep_map: MAP> + > NULL, + s STRUCT NULL + ) ENGINE = OLAP + UNIQUE KEY(pk) + DISTRIBUTED BY HASH(pk) BUCKETS 1 + PROPERTIES ( + "replication_allocation" = "tag.location.default: 1", + "enable_unique_key_merge_on_write" = "true", + "light_schema_change" = "true", + "store_row_column" = "true", + "row_store_page_size" = "16384" + ) + """ + + sql """ + INSERT INTO short_circuit_rowstore_nested_complex VALUES + (5, + named_struct( + 'nested_str', 'b-deep-5', + 'inner_s', named_struct('deep_str', 'b-inner-5', 'flag', true, 'deep_char', 'bd5'), + 'deep_arr', array(named_struct('verified', false, 'txt', 'b-deep-arr-5', 'char_tag', 'bt5')), + 'deep_map', map('b_key', named_struct('leaf', 'b-leaf-5', 'n', -5, 'char_leaf', 'bl5'))), + named_struct('str', 'b-s-5', 'char_leaf', 'bc5', 'num', -35, 'sibling', 'b-sib-5')) + """ + sql "SYNC" + + sql "SET enable_short_circuit_query=false" + order_qt_normal_path """ + SELECT /*+ SET_VAR(enable_nereids_planner=true) */ + pk, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'b_key'), 'leaf')) AS hit_char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'b_key'), 'leaf'))) AS hit_lower_len, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf')) AS miss_char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf'))) AS miss_lower_len, + LENGTH(struct_element(struct_element(deep, 'inner_s'), 'deep_char')) AS char_storage_len, + ((struct_element(s, 'num') + 1) IS NULL) AS expr_is_null + FROM short_circuit_rowstore_nested_complex + WHERE pk = 5 + """ + + sql "SET enable_short_circuit_query=true" + explain { + sql """ + SELECT /*+ SET_VAR(enable_nereids_planner=true) */ + pk, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'b_key'), 'leaf')) AS hit_char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'b_key'), 'leaf'))) AS hit_lower_len, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf')) AS miss_char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf'))) AS miss_lower_len, + LENGTH(struct_element(struct_element(deep, 'inner_s'), 'deep_char')) AS char_storage_len, + ((struct_element(s, 'num') + 1) IS NULL) AS expr_is_null + FROM short_circuit_rowstore_nested_complex + WHERE pk = 5 + """ + contains "SHORT-CIRCUIT" + } + order_qt_short_circuit_path """ + SELECT /*+ SET_VAR(enable_nereids_planner=true) */ + pk, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'b_key'), 'leaf')) AS hit_char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'b_key'), 'leaf'))) AS hit_lower_len, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf')) AS miss_char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf'))) AS miss_lower_len, + LENGTH(struct_element(struct_element(deep, 'inner_s'), 'deep_char')) AS char_storage_len, + ((struct_element(s, 'num') + 1) IS NULL) AS expr_is_null + FROM short_circuit_rowstore_nested_complex + WHERE pk = 5 + """ + + order_qt_short_circuit_path_repeat """ + SELECT /*+ SET_VAR(enable_nereids_planner=true) */ + pk, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'b_key'), 'leaf')) AS hit_char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'b_key'), 'leaf'))) AS hit_lower_len, + CHAR_LENGTH(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf')) AS miss_char_len, + LENGTH(LOWER(struct_element(element_at(struct_element(deep, 'deep_map'), 'dense'), 'leaf'))) AS miss_lower_len, + LENGTH(struct_element(struct_element(deep, 'inner_s'), 'deep_char')) AS char_storage_len, + ((struct_element(s, 'num') + 1) IS NULL) AS expr_is_null + FROM short_circuit_rowstore_nested_complex + WHERE pk = 5 + """ + } finally { + set_be_config.call("disable_storage_row_cache", "true") + } +}