From 3f78359da6644a0ee23bd0e3019f7f1742db1699 Mon Sep 17 00:00:00 2001 From: Hu Shenggang Date: Fri, 17 Jul 2026 17:22:29 +0800 Subject: [PATCH 1/9] [fix](be) Support versioned metadata access paths ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Nested-column pruning uses terminal NULL and OFFSET components to read only null maps or offsets. The legacy wire format encodes every access path as DATA, which makes those metadata selectors ambiguous with real nested fields that have the same names. Introduce an optional access-path version, preserve it through Thrift and protobuf descriptors, and use the path type to select the authoritative DATA or META payload in the typed version. New BEs continue to decode the legacy all-DATA format from old FEs, while the supported rolling-upgrade order upgrades all BEs before the FE emits typed paths. Refactor nested iterator planning to separate current data, current metadata, and descendant routing, and keep sibling META paths explicit so unrelated DATA projections do not disable metadata-only reads. ### Release note Fix metadata-only nested-column pruning and distinguish metadata selectors from same-named data fields. ### Check List (For Author) - Test: Unit Test and Regression Test - `./run-fe-ut.sh --run 'org.apache.doris.nereids.rules.rewrite.PruneNestedColumnTest,org.apache.doris.analysis.DescriptorToThriftConverterTest'` (74 passed on the final rebased FE tree) - `./run-be-ut.sh --run --filter='ColumnReaderTest.*:SlotDescriptorTest.AccessPathsPreservedThroughProtobuf'` (50 passed before the final rebase; the final rebuild was blocked before GTest by the missing master-added `liblance_c.a`) - `nereids_rules_p0/column_pruning/string_length_column_pruning`, `nereids_rules_p0/column_pruning/null_column_pruning`, and `nereids_rules_p0/column_pruning/nested_container_offset_pruning` (passed before the final rebase; the final rerun was blocked by the same missing dependency) - Behavior changed: Yes. Version-1 paths use explicit DATA/META payload semantics, while absent/version-0 paths retain the legacy encoding. - Does this need documentation: No Co-authored-by: minghong --- be/src/runtime/descriptors.cpp | 6 + be/src/storage/segment/column_reader.cpp | 744 +++++++++------- be/src/storage/segment/column_reader.h | 58 +- be/test/runtime/descriptor_test.cpp | 59 +- .../storage/segment/column_reader_test.cpp | 828 +++++++++++++++--- .../analysis/DescriptorToThriftConverter.java | 2 + .../AccessPathExpressionCollector.java | 31 +- .../rewrite/AccessPathPlanCollector.java | 17 +- .../rules/rewrite/NestedColumnPruning.java | 117 ++- .../DescriptorToThriftConverterTest.java | 33 +- .../rules/rewrite/PruneNestedColumnTest.java | 294 +++++-- gensrc/proto/descriptors.proto | 3 + gensrc/thrift/Descriptors.thrift | 7 + .../column_pruning/null_column_pruning.groovy | 22 +- 14 files changed, 1567 insertions(+), 654 deletions(-) diff --git a/be/src/runtime/descriptors.cpp b/be/src/runtime/descriptors.cpp index 2c072cf1a22aa1..79a1d67679b362 100644 --- a/be/src/runtime/descriptors.cpp +++ b/be/src/runtime/descriptors.cpp @@ -104,6 +104,9 @@ SlotDescriptor::SlotDescriptor(const PSlotDescriptor& pdesc) auto convert_to_thrift_column_access_path = [](const PColumnAccessPath& pb_path) { TColumnAccessPath thrift_path; thrift_path.type = (TAccessPathType::type)pb_path.type(); + if (pb_path.has_version()) { + thrift_path.__set_version(pb_path.version()); + } if (pb_path.has_data_access_path()) { thrift_path.__isset.data_access_path = true; for (int i = 0; i < pb_path.data_access_path().path_size(); ++i) { @@ -160,6 +163,9 @@ void SlotDescriptor::to_protobuf(PSlotDescriptor* pslot) const { doris::PColumnAccessPath* pb_path) { pb_path->Clear(); pb_path->set_type((PAccessPathType)thrift_path.type); // 使用 reinterpret_cast 进行类型转换 + if (thrift_path.__isset.version) { + pb_path->set_version(thrift_path.version); + } if (thrift_path.__isset.data_access_path) { auto* pb_data = pb_path->mutable_data_access_path(); pb_data->Clear(); diff --git a/be/src/storage/segment/column_reader.cpp b/be/src/storage/segment/column_reader.cpp index db8f2d6819b04d..eb8e8a4d448285 100644 --- a/be/src/storage/segment/column_reader.cpp +++ b/be/src/storage/segment/column_reader.cpp @@ -18,6 +18,7 @@ #include "storage/segment/column_reader.h" #include +#include #include #include #include @@ -90,25 +91,229 @@ inline bool read_as_string(PrimitiveType type) { type == PrimitiveType::TYPE_BITMAP || type == PrimitiveType::TYPE_FIXED_LENGTH_OBJECT; } -bool is_current_level_meta_access_path(const TColumnAccessPath& path) { - if (path.data_access_path.path.size() != 1) { - return false; - } - const auto& component = path.data_access_path.path[0]; +bool is_meta_access_path_component(const std::string& component) { return StringCaseEqual()(component, ColumnIterator::ACCESS_OFFSET) || StringCaseEqual()(component, ColumnIterator::ACCESS_NULL); } -bool is_current_level_data_access_path(const TColumnAccessPath& path, - const std::string& column_name) { - return path.data_access_path.path.size() == 1 && - StringCaseEqual()(path.data_access_path.path[0], column_name); +bool uses_legacy_access_path_encoding(const TColumnAccessPath& path) { + return !path.__isset.version || + path.version == g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY; } -void remove_current_level_meta_access_paths(TColumnAccessPaths& paths) { - auto removed = std::ranges::remove_if(paths, is_current_level_meta_access_path); - paths.erase(removed.begin(), removed.end()); -} +namespace { + +// Routes only descendant paths produced by ColumnIterator::_split_access_paths(). Their versions +// have been validated, legacy paths have DATA type, and every active payload is non-empty. +// Routing rewrites only the payload selected by type. +class DescendantAccessPathRouter final { +public: + DescendantAccessPathRouter() = delete; + + struct ChildRoutes { + TColumnAccessPaths all; + TColumnAccessPaths predicate; + + bool empty() const { return all.empty() && predicate.empty(); } + }; + + struct MapRoutes { + ChildRoutes key; + ChildRoutes value; + }; + + static Result route_map_descendants(TColumnAccessPaths all_paths, + TColumnAccessPaths predicate_paths, + const std::string& key_name, + const std::string& value_name) { + MapRoutes routes; + auto status = route_map_path_set(std::move(all_paths), key_name, value_name, routes.key.all, + routes.value.all); + if (!status.ok()) { + return ResultError(std::move(status)); + } + status = route_map_path_set(std::move(predicate_paths), key_name, value_name, + routes.key.predicate, routes.value.predicate); + if (!status.ok()) { + return ResultError(std::move(status)); + } + return routes; + } + + static Result route_array_descendants(TColumnAccessPaths all_paths, + TColumnAccessPaths predicate_paths, + const std::string& item_name) { + ChildRoutes routes {.all = std::move(all_paths), .predicate = std::move(predicate_paths)}; + auto rewrite_item_roots = [&](TColumnAccessPaths& paths) -> Status { + for (auto& path : paths) { + const bool matches_access_all = DORIS_TRY( + root_matches(path, ColumnIterator::ACCESS_ALL, RootMatchMode::EXACT)); + if (matches_access_all) { + RETURN_IF_ERROR(rewrite_active_payload_root(path, item_name)); + } + } + return Status::OK(); + }; + + auto status = rewrite_item_roots(routes.all); + if (!status.ok()) { + return ResultError(std::move(status)); + } + status = rewrite_item_roots(routes.predicate); + if (!status.ok()) { + return ResultError(std::move(status)); + } + return routes; + } + + static Result route_struct_child(const TColumnAccessPaths& all_paths, + const TColumnAccessPaths& predicate_paths, + const std::string& child_name, + bool route_projection_paths) { + ChildRoutes routes; + auto route_path_set = [&](const TColumnAccessPaths& source_paths, + bool is_predicate) -> Status { + auto& child_paths = is_predicate ? routes.predicate : routes.all; + for (const auto& path : source_paths) { + const bool routes_to_child = + DORIS_TRY(root_matches(path, child_name, RootMatchMode::CASE_INSENSITIVE)); + if (routes_to_child) { + child_paths.emplace_back(path); + } + } + return Status::OK(); + }; + + if (route_projection_paths) { + auto status = route_path_set(all_paths, false); + if (!status.ok()) { + return ResultError(std::move(status)); + } + } + auto status = route_path_set(predicate_paths, true); + if (!status.ok()) { + return ResultError(std::move(status)); + } + return routes; + } + +private: + enum class RootMatchMode { EXACT, CASE_INSENSITIVE }; + enum class MapTarget { NONE, WILDCARD, KEY, VALUE }; + + template + static Status visit_normalized_payload(AccessPath& access_path, Visitor&& visitor) { + switch (access_path.type) { + case TAccessPathType::DATA: + if (!access_path.__isset.data_access_path) { + return Status::InternalError( + "Invalid DATA access path: data_access_path payload is not set"); + } + return std::forward(visitor)(access_path.data_access_path); + case TAccessPathType::META: + if (!access_path.__isset.meta_access_path) { + return Status::InternalError( + "Invalid META access path: meta_access_path payload is not set"); + } + return std::forward(visitor)(access_path.meta_access_path); + default: + return Status::InternalError("Invalid access path type: {}", + static_cast(access_path.type)); + } + } + + static Result root_matches(const TColumnAccessPath& access_path, + const std::string& expected_root, RootMatchMode match_mode) { + bool matches = false; + auto status = visit_normalized_payload(access_path, [&](const auto& payload) { + DORIS_CHECK(!payload.path.empty()); + matches = match_mode == RootMatchMode::EXACT + ? payload.path.front() == expected_root + : StringCaseEqual()(payload.path.front(), expected_root); + return Status::OK(); + }); + if (!status.ok()) { + return ResultError(std::move(status)); + } + return matches; + } + + static Status rewrite_active_payload_root(TColumnAccessPath& access_path, + const std::string& child_name) { + return visit_normalized_payload(access_path, [&](auto& payload) { + DORIS_CHECK(!payload.path.empty()); + payload.path.front() = child_name; + return Status::OK(); + }); + } + + // Map `*` applies trailing qualifiers only to values, while locating entries still requires + // the complete key column as DATA. Construct a fresh key path because the source may be META, + // and preserve its version so child routing keeps the same legacy/typed encoding. + static TColumnAccessPath create_version_preserving_whole_data_child_path( + const TColumnAccessPath& source_path, const std::string& child_name) { + TColumnAccessPath child_path; + child_path.__set_type(TAccessPathType::DATA); + TDataAccessPath data_path; + data_path.__set_path({child_name}); + child_path.__set_data_access_path(data_path); + if (source_path.__isset.version) { + child_path.__set_version(source_path.version); + } + return child_path; + } + + static Result map_target(const TColumnAccessPath& access_path) { + MapTarget target = MapTarget::NONE; + auto status = visit_normalized_payload(access_path, [&](const auto& payload) { + DORIS_CHECK(!payload.path.empty()); + const auto& root = payload.path.front(); + if (root == ColumnIterator::ACCESS_ALL) { + target = MapTarget::WILDCARD; + } else if (root == ColumnIterator::ACCESS_MAP_KEYS) { + target = MapTarget::KEY; + } else if (root == ColumnIterator::ACCESS_MAP_VALUES) { + target = MapTarget::VALUE; + } + return Status::OK(); + }); + if (!status.ok()) { + return ResultError(std::move(status)); + } + return target; + } + + static Status route_map_path_set(TColumnAccessPaths source_paths, const std::string& key_name, + const std::string& value_name, TColumnAccessPaths& key_paths, + TColumnAccessPaths& value_paths) { + for (auto& path : source_paths) { + const auto target = DORIS_TRY(map_target(path)); + switch (target) { + case MapTarget::WILDCARD: + // A wildcard needs complete keys for runtime lookup, while any remaining + // qualifiers apply only to the value. The value keeps its type and version. + key_paths.emplace_back( + create_version_preserving_whole_data_child_path(path, key_name)); + RETURN_IF_ERROR(rewrite_active_payload_root(path, value_name)); + value_paths.emplace_back(std::move(path)); + break; + case MapTarget::KEY: + RETURN_IF_ERROR(rewrite_active_payload_root(path, key_name)); + key_paths.emplace_back(std::move(path)); + break; + case MapTarget::VALUE: + RETURN_IF_ERROR(rewrite_active_payload_root(path, value_name)); + value_paths.emplace_back(std::move(path)); + break; + case MapTarget::NONE: + break; + } + } + return Status::OK(); + } +}; + +} // namespace Status ColumnReader::create_array(const ColumnReaderOptions& opts, const ColumnMetaPB& meta, const io::FileReaderSPtr& file_reader, @@ -951,44 +1156,122 @@ void ColumnIterator::_recovery_from_place_holder_column(MutableColumnPtr& dst) { } } -Result ColumnIterator::_get_sub_access_paths( - TColumnAccessPaths sub_access_paths, bool is_predicate) { - // Access paths passed to a complex iterator always start with the current - // column name. Strip that component and return the remaining child-relative - // paths to the caller. For example, when this iterator is for column `s`, - // path `s.a.b` is converted to `a.b` and then dispatched to child `a`. - // - // If stripping the current column consumes the whole path, the current - // iterator itself is requested rather than one of its children. Mark the - // current iterator according to the path source: predicate paths must be read - // in the predicate phase, while all/output paths become lazy output targets. - // Empty or mismatched paths indicate an FE/BE access-path contract violation. - for (auto it = sub_access_paths.begin(); it != sub_access_paths.end();) { - TColumnAccessPath& name_path = *it; - if (name_path.data_access_path.path.empty()) { +Result ColumnIterator::_split_access_paths( + TColumnAccessPaths access_paths) const { + AccessPathSplit split; + for (auto& path : access_paths) { + const bool uses_legacy_encoding = uses_legacy_access_path_encoding(path); + if (!uses_legacy_encoding && + path.version != g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED) { + return ResultError( + Status::InternalError("Unsupported access path version: {}", path.version)); + } + + std::vector* components = nullptr; + if (uses_legacy_encoding) { + if (path.type != TAccessPathType::DATA) { + return ResultError(Status::InternalError("Invalid legacy access path type: {}", + static_cast(path.type))); + } + if (!path.__isset.data_access_path) { + return ResultError(Status::InternalError( + "Invalid legacy access path: data_access_path payload is not set")); + } + components = &path.data_access_path.path; + } else { + switch (path.type) { + case TAccessPathType::DATA: + if (!path.__isset.data_access_path) { + return ResultError(Status::InternalError( + "Invalid DATA access path: data_access_path payload is not set")); + } + components = &path.data_access_path.path; + break; + case TAccessPathType::META: + if (!path.__isset.meta_access_path) { + return ResultError(Status::InternalError( + "Invalid META access path: meta_access_path payload is not set")); + } + components = &path.meta_access_path.path; + break; + default: + return ResultError(Status::InternalError("Invalid access path type: {}", + static_cast(path.type))); + } + } + + if (components->empty()) { return ResultError(Status::InternalError( "Invalid access path for column '{}': path is empty", _column_name)); } - if (!StringCaseEqual()(name_path.data_access_path.path[0], _column_name)) { + if (!StringCaseEqual()((*components)[0], _column_name)) { return ResultError(Status::InternalError( R"(Invalid access path for column: expected name "{}", got "{}")", _column_name, - name_path.data_access_path.path[0])); + (*components)[0])); } - name_path.data_access_path.path.erase(name_path.data_access_path.path.begin()); - if (!name_path.data_access_path.path.empty()) { - ++it; - } else { - if (is_predicate) { - set_read_requirement(ReadRequirement::PREDICATE); - } else { - set_lazy_output_requirement(); + components->erase(components->begin()); + if (components->empty()) { + split.reads_current_data = true; + continue; + } + + const bool is_current_level_meta = + components->size() == 1 && is_meta_access_path_component((*components)[0]) && + (path.type == TAccessPathType::META || uses_legacy_encoding); + if (is_current_level_meta) { + if (StringCaseEqual()((*components)[0], ACCESS_OFFSET)) { + split.current_meta_mode = MetaReadMode::OFFSET_ONLY; + } else if (split.current_meta_mode == MetaReadMode::DEFAULT) { + split.current_meta_mode = MetaReadMode::NULL_MAP_ONLY; } - it = sub_access_paths.erase(it); + continue; + } + + split.descendant_paths.emplace_back(std::move(path)); + } + return split; +} + +Result ColumnIterator::_prepare_nested_access_paths( + const TColumnAccessPaths& all_access_paths, + const TColumnAccessPaths& predicate_access_paths, NestedMetaSupport meta_support, + const std::function& set_all_data_descendants_read_requirement) { + const auto requirement_before = _read_requirement; + if (!predicate_access_paths.empty()) { + set_read_requirement_self(ReadRequirement::PREDICATE); + } + + NestedAccessPathPlan plan; + auto all_split = _split_access_paths(all_access_paths); + if (!all_split.has_value()) { + return ResultError(std::move(all_split).error()); + } + plan.all = std::move(all_split).value(); + + auto predicate_split = _split_access_paths(predicate_access_paths); + if (!predicate_split.has_value()) { + return ResultError(std::move(predicate_split).error()); + } + plan.predicate = std::move(predicate_split).value(); + if (plan.all.reads_current_data) { + set_lazy_output_requirement(); + } + if (plan.predicate.reads_current_data) { + set_read_requirement(ReadRequirement::PREDICATE); + } + + if (!plan.predicate.has_descendant_paths()) { + RETURN_IF_ERROR_RESULT(_check_and_set_meta_read_mode(requirement_before, plan.all)); + plan.skip_data_descendants = + read_null_map_only() || + (meta_support == NestedMetaSupport::NULL_MAP_AND_OFFSET && read_offset_only()); + if (plan.skip_data_descendants) { + set_all_data_descendants_read_requirement(ReadRequirement::SKIP); } } - return sub_access_paths; + return plan; } ///====================== MapFileColumnIterator ============================//// @@ -1454,120 +1737,27 @@ Status MapFileColumnIterator::set_access_paths(const TColumnAccessPaths& all_acc return Status::OK(); } - const auto requirement_before_access_path = _read_requirement; - if (!predicate_access_paths.empty()) { - set_read_requirement_self(ReadRequirement::PREDICATE); - DLOG(INFO) << "Map column iterator set sub-column " << _column_name << " to PREDICATE"; - } - - const bool has_current_level_data_path = - std::ranges::any_of(all_access_paths, [this](const TColumnAccessPath& path) { - return is_current_level_data_access_path(path, _column_name); - }); - auto sub_all_access_paths = DORIS_TRY(_get_sub_access_paths(all_access_paths)); - auto sub_predicate_access_paths = - DORIS_TRY(_get_sub_access_paths(predicate_access_paths, true)); - if (has_current_level_data_path) { - remove_current_level_meta_access_paths(sub_all_access_paths); - } - const bool has_current_level_predicate_meta_path = - std::ranges::any_of(sub_predicate_access_paths, is_current_level_meta_access_path); - // Current-level predicate metadata paths are consumed by this map iterator and must not be - // forwarded to key/value children. The FE keeps all_access_paths as a superset of predicate - // paths, so meta-only mode is still decided from sub_all_access_paths below. - remove_current_level_meta_access_paths(sub_predicate_access_paths); - - if (sub_predicate_access_paths.empty() && _read_requirement == ReadRequirement::PREDICATE && - !has_current_level_predicate_meta_path) { - // if no sub-column in predicate_access_paths, but current column is PREDICATE, - // then we should set key/value iterator to PREDICATE too. - _key_iterator->set_read_requirement(ReadRequirement::PREDICATE); - _val_iterator->set_read_requirement(ReadRequirement::PREDICATE); - } - - if (sub_predicate_access_paths.empty()) { - // Check for meta-only modes (OFFSET_ONLY or NULL_MAP_ONLY). Only skip key/value - // iterators when no predicate sub-path needs them in the predicate phase. - _check_and_set_meta_read_mode(requirement_before_access_path, sub_all_access_paths); - if (read_offset_only()) { - _key_iterator->set_read_requirement(ReadRequirement::SKIP); - _val_iterator->set_read_requirement(ReadRequirement::SKIP); - DLOG(INFO) << "Map column iterator set column " << _column_name - << " to OFFSET_ONLY meta read mode, key/value columns set to SKIP"; - return Status::OK(); - } - if (read_null_map_only()) { - _key_iterator->set_read_requirement(ReadRequirement::SKIP); - _val_iterator->set_read_requirement(ReadRequirement::SKIP); - DLOG(INFO) << "Map column iterator set column " << _column_name - << " to NULL_MAP_ONLY meta read mode, key/value columns set to SKIP"; - return Status::OK(); - } + auto plan = DORIS_TRY(_prepare_nested_access_paths( + all_access_paths, predicate_access_paths, NestedMetaSupport::NULL_MAP_AND_OFFSET, + [this](ReadRequirement requirement) { + _key_iterator->set_read_requirement(requirement); + _val_iterator->set_read_requirement(requirement); + })); + if (plan.skip_data_descendants) { + return Status::OK(); } - // A current-level data path is consumed by _get_sub_access_paths() and leaves - // sub_all_access_paths empty after marking key/value as lazy-read targets. Predicate - // sub-paths still have to be routed to child iterators for the predicate phase. - if (sub_all_access_paths.empty() && sub_predicate_access_paths.empty()) { + if (!plan.all.has_descendant_paths() && !plan.predicate.has_descendant_paths()) { return Status::OK(); } - TColumnAccessPaths key_all_access_paths; - TColumnAccessPaths val_all_access_paths; - TColumnAccessPaths key_predicate_access_paths; - TColumnAccessPaths val_predicate_access_paths; - - for (auto paths : sub_all_access_paths) { - if (paths.data_access_path.path[0] == ACCESS_ALL) { - // ACCESS_ALL means element_at(map, key) style access: the key column must be - // fully read so that the runtime can match the requested key, while any sub-path - // qualifiers (e.g. OFFSET) apply only to the value column. - // For key: create a path with just the column name (= full data access). - TColumnAccessPath key_path; - key_path.__set_type(paths.type); - TDataAccessPath key_data_path; - key_data_path.__set_path({_key_iterator->column_name()}); - key_path.__set_data_access_path(key_data_path); - key_all_access_paths.emplace_back(std::move(key_path)); - // For value: pass the full sub-path so qualifiers like OFFSET propagate. - paths.data_access_path.path[0] = _val_iterator->column_name(); - val_all_access_paths.emplace_back(paths); - } else if (paths.data_access_path.path[0] == ACCESS_MAP_KEYS) { - paths.data_access_path.path[0] = _key_iterator->column_name(); - key_all_access_paths.emplace_back(paths); - } else if (paths.data_access_path.path[0] == ACCESS_MAP_VALUES) { - paths.data_access_path.path[0] = _val_iterator->column_name(); - val_all_access_paths.emplace_back(paths); - } - } - for (auto paths : sub_predicate_access_paths) { - if (paths.data_access_path.path[0] == ACCESS_ALL) { - // Same logic as above: key needs full data, value gets the sub-path. - TColumnAccessPath key_path; - key_path.__set_type(paths.type); - TDataAccessPath key_data_path; - key_data_path.__set_path({_key_iterator->column_name()}); - key_path.__set_data_access_path(key_data_path); - key_predicate_access_paths.emplace_back(std::move(key_path)); - paths.data_access_path.path[0] = _val_iterator->column_name(); - val_predicate_access_paths.emplace_back(paths); - } else if (paths.data_access_path.path[0] == ACCESS_MAP_KEYS) { - paths.data_access_path.path[0] = _key_iterator->column_name(); - key_predicate_access_paths.emplace_back(paths); - } else if (paths.data_access_path.path[0] == ACCESS_MAP_VALUES) { - paths.data_access_path.path[0] = _val_iterator->column_name(); - val_predicate_access_paths.emplace_back(paths); - } - } - - const auto need_read_keys = - !key_all_access_paths.empty() || !key_predicate_access_paths.empty(); - const auto need_read_values = - !val_all_access_paths.empty() || !val_predicate_access_paths.empty(); - - if (need_read_keys) { + auto child_paths = DORIS_TRY(DescendantAccessPathRouter::route_map_descendants( + std::move(plan.all.descendant_paths), std::move(plan.predicate.descendant_paths), + _key_iterator->column_name(), _val_iterator->column_name())); + + if (!child_paths.key.empty()) { RETURN_IF_ERROR( - _key_iterator->set_access_paths(key_all_access_paths, key_predicate_access_paths)); + _key_iterator->set_access_paths(child_paths.key.all, child_paths.key.predicate)); // Apply LAZY_OUTPUT after child predicate paths have been handled. Read requirements are // monotonic, so a predicate-only child already promoted to PREDICATE will not // be downgraded, while a non-predicate child becomes a lazy materialization target. @@ -1577,9 +1767,9 @@ Status MapFileColumnIterator::set_access_paths(const TColumnAccessPaths& all_acc DLOG(INFO) << "Map column iterator set key column to SKIP"; } - if (need_read_values) { - RETURN_IF_ERROR( - _val_iterator->set_access_paths(val_all_access_paths, val_predicate_access_paths)); + if (!child_paths.value.empty()) { + RETURN_IF_ERROR(_val_iterator->set_access_paths(child_paths.value.all, + child_paths.value.predicate)); // Same as keys: predicate-only value paths stay PREDICATE because this // post-processing update cannot lower a stronger child requirement. _val_iterator->set_read_requirement_self(ReadRequirement::LAZY_OUTPUT); @@ -1834,70 +2024,31 @@ Status StructFileColumnIterator::set_access_paths( return Status::OK(); } - const auto requirement_before_access_path = _read_requirement; - if (!predicate_access_paths.empty()) { - set_read_requirement_self(ReadRequirement::PREDICATE); - DLOG(INFO) << "Struct column iterator set sub-column " << _column_name << " to PREDICATE"; - } - - const bool has_current_level_data_path = - std::ranges::any_of(all_access_paths, [this](const TColumnAccessPath& path) { - return is_current_level_data_access_path(path, _column_name); - }); - auto sub_all_access_paths = DORIS_TRY(_get_sub_access_paths(all_access_paths)); - auto sub_predicate_access_paths = - DORIS_TRY(_get_sub_access_paths(predicate_access_paths, true)); - if (has_current_level_data_path) { - remove_current_level_meta_access_paths(sub_all_access_paths); - } - const bool has_current_level_predicate_meta_path = - std::ranges::any_of(sub_predicate_access_paths, is_current_level_meta_access_path); - // Current-level predicate metadata paths are consumed by this struct iterator and must not be - // forwarded to child fields. The FE keeps all_access_paths as a superset of predicate paths, so - // NULL_MAP_ONLY is still decided from sub_all_access_paths below. - remove_current_level_meta_access_paths(sub_predicate_access_paths); - - if (sub_predicate_access_paths.empty()) { - // Check for NULL_MAP_ONLY mode: only read null map, skip all sub-columns. - // Do not take this early return when predicate child paths must still be read. - _check_and_set_meta_read_mode(requirement_before_access_path, sub_all_access_paths); - if (read_null_map_only()) { - for (auto& sub_iterator : _sub_column_iterators) { - sub_iterator->set_read_requirement(ReadRequirement::SKIP); - } - DLOG(INFO) << "Struct column iterator set column " << _column_name - << " to NULL_MAP_ONLY meta read mode, all sub-columns set to SKIP"; - return Status::OK(); - } - } + auto plan = DORIS_TRY(_prepare_nested_access_paths( + all_access_paths, predicate_access_paths, NestedMetaSupport::NULL_MAP, + [this](ReadRequirement requirement) { + for (auto& sub_iterator : _sub_column_iterators) { + sub_iterator->set_read_requirement(requirement); + } + })); - const auto no_sub_column_to_skip = sub_all_access_paths.empty(); - const auto no_predicate_sub_column = sub_predicate_access_paths.empty(); + if (plan.skip_data_descendants) { + DLOG(INFO) << "Struct column iterator set column " << _column_name + << " to NULL_MAP_ONLY meta read mode, all sub-columns set to SKIP"; + return Status::OK(); + } + const bool reads_all_sub_columns = plan.all.reads_current_data; + const bool route_projection_paths = !reads_all_sub_columns; for (auto& sub_iterator : _sub_column_iterators) { const auto name = sub_iterator->column_name(); - TColumnAccessPaths sub_all_access_paths_of_this; - if (!no_sub_column_to_skip) { - for (const auto& paths : sub_all_access_paths) { - if (paths.data_access_path.path[0] == name) { - sub_all_access_paths_of_this.emplace_back(paths); - } - } - } - - TColumnAccessPaths sub_predicate_access_paths_of_this; - if (!no_predicate_sub_column) { - for (const auto& paths : sub_predicate_access_paths) { - if (StringCaseEqual()(paths.data_access_path.path[0], name)) { - sub_predicate_access_paths_of_this.emplace_back(paths); - } - } - } + auto paths = DORIS_TRY(DescendantAccessPathRouter::route_struct_child( + plan.all.descendant_paths, plan.predicate.descendant_paths, name, + route_projection_paths)); // Predicate-only child paths still need to be routed to the child iterator // even when the child is not requested by ordinary projection access paths. - const bool need_to_read = no_sub_column_to_skip || !sub_all_access_paths_of_this.empty() || - !sub_predicate_access_paths_of_this.empty(); + const bool need_to_read = reads_all_sub_columns || !paths.empty(); if (!need_to_read) { set_read_requirement_self(ReadRequirement::SKIP); sub_iterator->set_read_requirement(ReadRequirement::SKIP); @@ -1905,15 +2056,7 @@ Status StructFileColumnIterator::set_access_paths( continue; } - if (no_predicate_sub_column && _read_requirement == ReadRequirement::PREDICATE && - !has_current_level_predicate_meta_path) { - // if no sub-column in predicate_access_paths, but current column is PREDICATE, - // then we should set sub iterator to PREDICATE too. - sub_iterator->set_read_requirement(ReadRequirement::PREDICATE); - } - - RETURN_IF_ERROR(sub_iterator->set_access_paths(sub_all_access_paths_of_this, - sub_predicate_access_paths_of_this)); + RETURN_IF_ERROR(sub_iterator->set_access_paths(paths.all, paths.predicate)); // Set LAZY_OUTPUT after routing child predicate paths. If the child was needed only for // predicate evaluation, set_access_paths() has already promoted it to // PREDICATE and this monotonic update will not downgrade it. Otherwise, this @@ -2285,87 +2428,24 @@ Status ArrayFileColumnIterator::set_access_paths(const TColumnAccessPaths& all_a return Status::OK(); } - const auto requirement_before_access_path = _read_requirement; - if (!predicate_access_paths.empty()) { - set_read_requirement_self(ReadRequirement::PREDICATE); - DLOG(INFO) << "Array column iterator set sub-column " << _column_name << " to PREDICATE"; - } - - const bool has_current_level_data_path = - std::ranges::any_of(all_access_paths, [this](const TColumnAccessPath& path) { - return is_current_level_data_access_path(path, _column_name); - }); - auto sub_all_access_paths = DORIS_TRY(_get_sub_access_paths(all_access_paths)); - auto sub_predicate_access_paths = - DORIS_TRY(_get_sub_access_paths(predicate_access_paths, true)); - if (has_current_level_data_path) { - // A current-level data path already reads the array offsets while materializing the array. - // Do not let a redundant current-level OFFSET/NULL path switch this iterator into a - // meta-only mode that would skip item data. - remove_current_level_meta_access_paths(sub_all_access_paths); - } - const bool has_current_level_predicate_meta_path = - std::ranges::any_of(sub_predicate_access_paths, is_current_level_meta_access_path); - // Current-level predicate metadata paths are consumed by this array iterator and must not be - // forwarded to the item iterator. The FE keeps all_access_paths as a superset of predicate - // paths, so meta-only mode is still decided from sub_all_access_paths below. - auto removed = - std::ranges::remove_if(sub_predicate_access_paths, is_current_level_meta_access_path); - sub_predicate_access_paths.erase(removed.begin(), removed.end()); - - if (sub_predicate_access_paths.empty()) { - // Check for meta-only modes (OFFSET_ONLY or NULL_MAP_ONLY). Only skip the item - // iterator when no predicate sub-path needs it in the predicate phase. - _check_and_set_meta_read_mode(requirement_before_access_path, sub_all_access_paths); - if (read_offset_only()) { - _item_iterator->set_read_requirement(ReadRequirement::SKIP); - DLOG(INFO) << "Array column iterator set column " << _column_name - << " to OFFSET_ONLY meta read mode, item column set to SKIP"; - return Status::OK(); - } - if (read_null_map_only()) { - _item_iterator->set_read_requirement(ReadRequirement::SKIP); - DLOG(INFO) << "Array column iterator set column " << _column_name - << " to NULL_MAP_ONLY meta read mode, item column set to SKIP"; - return Status::OK(); - } - } - // OFFSET/NULL at the current array level is consumed by this iterator. After deciding that - // the array is not in a meta-only mode, do not forward those paths to the item iterator. - remove_current_level_meta_access_paths(sub_all_access_paths); - - const auto no_sub_column_to_skip = sub_all_access_paths.empty(); - const auto no_predicate_sub_column = sub_predicate_access_paths.empty(); + auto plan = DORIS_TRY(_prepare_nested_access_paths( + all_access_paths, predicate_access_paths, NestedMetaSupport::NULL_MAP_AND_OFFSET, + [this](ReadRequirement requirement) { + _item_iterator->set_read_requirement(requirement); + })); - if (!no_sub_column_to_skip) { - for (auto& path : sub_all_access_paths) { - if (path.data_access_path.path[0] == ACCESS_ALL) { - path.data_access_path.path[0] = _item_iterator->column_name(); - } - } + if (plan.skip_data_descendants) { + return Status::OK(); } - if (no_predicate_sub_column) { - // Current-level predicate meta paths (OFFSET/NULL) are consumed by the array itself and - // removed before forwarding paths to the item iterator. If they are the only predicate - // paths, the item iterator may still be needed later for lazy materialization, but it must - // not be promoted to PREDICATE. Only propagate the predicate requirement when the - // parent predicate really applies to the item/whole value instead of array metadata only. - if (_read_requirement == ReadRequirement::PREDICATE && - !has_current_level_predicate_meta_path) { - _item_iterator->set_read_requirement(ReadRequirement::PREDICATE); - } - } else { - for (auto& path : sub_predicate_access_paths) { - if (path.data_access_path.path[0] == ACCESS_ALL) { - path.data_access_path.path[0] = _item_iterator->column_name(); - } - } - } + const bool has_all_descendant = plan.all.has_descendant_paths(); + const bool has_predicate_descendant = plan.predicate.has_descendant_paths(); + auto child_paths = DORIS_TRY(DescendantAccessPathRouter::route_array_descendants( + std::move(plan.all.descendant_paths), std::move(plan.predicate.descendant_paths), + _item_iterator->column_name())); - if (!no_sub_column_to_skip || !no_predicate_sub_column) { - RETURN_IF_ERROR( - _item_iterator->set_access_paths(sub_all_access_paths, sub_predicate_access_paths)); + if (has_all_descendant || has_predicate_descendant) { + RETURN_IF_ERROR(_item_iterator->set_access_paths(child_paths.all, child_paths.predicate)); // Predicate-only item paths stay PREDICATE because this update runs after // child set_access_paths() and read requirements are monotonic. Non-predicate item paths are // marked as lazy materialization targets. @@ -2394,26 +2474,7 @@ Status StringFileColumnIterator::init(const ColumnIteratorOptions& opts) { Status StringFileColumnIterator::set_access_paths( const TColumnAccessPaths& all_access_paths, const TColumnAccessPaths& predicate_access_paths) { - if (all_access_paths.empty() && predicate_access_paths.empty()) { - return Status::OK(); - } - - const auto requirement_before_access_path = _read_requirement; - if (!predicate_access_paths.empty()) { - set_read_requirement(ReadRequirement::PREDICATE); - } - - const bool has_current_level_data_path = - std::ranges::any_of(all_access_paths, [this](const TColumnAccessPath& path) { - return is_current_level_data_access_path(path, _column_name); - }); - // Strip the column name from path[0] before checking for meta-only modes. - // Raw paths look like ["col_name", "OFFSET"] or ["col_name", "NULL"]. - auto sub_all_access_paths = DORIS_TRY(_get_sub_access_paths(all_access_paths)); - if (has_current_level_data_path) { - remove_current_level_meta_access_paths(sub_all_access_paths); - } - _check_and_set_meta_read_mode(requirement_before_access_path, sub_all_access_paths); + RETURN_IF_ERROR(FileColumnIterator::set_access_paths(all_access_paths, predicate_access_paths)); // OFFSET_ONLY mode is fundamentally incompatible with CHAR columns: // CHAR is stored padded to its declared length (see // OlapColumnDataConvertorChar::clone_and_padding), so the per-row length @@ -2449,10 +2510,32 @@ Status StringFileColumnIterator::set_access_paths( //////////////////////////////////////////////////////////////////////////////// +Status FileColumnIterator::set_access_paths(const TColumnAccessPaths& all_access_paths, + const TColumnAccessPaths& predicate_access_paths) { + if (all_access_paths.empty() && predicate_access_paths.empty()) { + return Status::OK(); + } + + const auto requirement_before_access_path = _read_requirement; + if (!predicate_access_paths.empty()) { + set_read_requirement(ReadRequirement::PREDICATE); + } + + auto all_split = DORIS_TRY(_split_access_paths(all_access_paths)); + auto predicate_split = DORIS_TRY(_split_access_paths(predicate_access_paths)); + if (all_split.reads_current_data) { + set_lazy_output_requirement(); + } + if (predicate_split.reads_current_data) { + set_read_requirement(ReadRequirement::PREDICATE); + } + return _check_and_set_meta_read_mode(requirement_before_access_path, all_split); +} + FileColumnIterator::FileColumnIterator(std::shared_ptr reader) : _reader(reader) {} -void ColumnIterator::_check_and_set_meta_read_mode(ReadRequirement requirement_before_access_path, - const TColumnAccessPaths& sub_all_access_paths) { +Status ColumnIterator::_check_and_set_meta_read_mode(ReadRequirement requirement_before_access_path, + const AccessPathSplit& all_access_paths) { _meta_read_mode = MetaReadMode::DEFAULT; if (requirement_before_access_path != ReadRequirement::NORMAL && requirement_before_access_path != ReadRequirement::SKIP) { @@ -2460,32 +2543,15 @@ void ColumnIterator::_check_and_set_meta_read_mode(ReadRequirement requirement_b // to materialize data. In that case a later predicate NULL/OFFSET path is only // an additional predicate requirement and must not downgrade the read to // meta-only. - return; + return Status::OK(); } - bool has_offset_path = false; - bool has_null_path = false; - for (const auto& path : sub_all_access_paths) { - if (!is_current_level_meta_access_path(path)) { - _meta_read_mode = MetaReadMode::DEFAULT; - return; - } - const auto& component = path.data_access_path.path[0]; - if (StringCaseEqual()(component, ACCESS_OFFSET)) { - has_offset_path = true; - } else { - has_null_path = true; - } - } - if (has_offset_path) { - // OFFSET_ONLY skips actual child/string data, but nullable complex iterators still - // materialize the current-level null map. So OFFSET covers OFFSET+NULL metadata. - _meta_read_mode = MetaReadMode::OFFSET_ONLY; - } else if (has_null_path) { - _meta_read_mode = MetaReadMode::NULL_MAP_ONLY; - } else { - _meta_read_mode = MetaReadMode::DEFAULT; + if (all_access_paths.reads_current_data || all_access_paths.has_descendant_paths()) { + return Status::OK(); } + + _meta_read_mode = all_access_paths.current_meta_mode; + return Status::OK(); } Status FileColumnIterator::init(const ColumnIteratorOptions& opts) { diff --git a/be/src/storage/segment/column_reader.h b/be/src/storage/segment/column_reader.h index 327c44c4fee181..a234427b650b42 100644 --- a/be/src/storage/segment/column_reader.h +++ b/be/src/storage/segment/column_reader.h @@ -24,6 +24,7 @@ #include // for size_t #include // for uint32_t +#include #include #include // for unique_ptr #include @@ -500,18 +501,48 @@ class ColumnIterator { } protected: + struct AccessPathSplit { + TColumnAccessPaths descendant_paths; + bool reads_current_data = false; + MetaReadMode current_meta_mode = MetaReadMode::DEFAULT; + + bool has_descendant_paths() const { return !descendant_paths.empty(); } + }; + + // Nested columns share the same current-level access-path planning, while their data-child + // topology and descendant routing remain container-specific. + struct NestedAccessPathPlan { + AccessPathSplit all; + AccessPathSplit predicate; + bool skip_data_descendants = false; + }; + + // At their current level, Struct supports null-map metadata. Map and Array additionally + // support offsets. + enum class NestedMetaSupport { NULL_MAP, NULL_MAP_AND_OFFSET }; + void _convert_to_place_holder_column(MutableColumnPtr& dst, size_t count); void _recovery_from_place_holder_column(MutableColumnPtr& dst); - // Derive current-level meta-only read mode from access paths. Meta-only is valid only when - // this iterator had no data-read requirement before applying the current paths, and every - // visible path at this level is NULL/OFFSET metadata. - void _check_and_set_meta_read_mode(ReadRequirement requirement_before_access_path, - const TColumnAccessPaths& sub_all_access_paths); - - Result _get_sub_access_paths(TColumnAccessPaths access_paths, - bool is_predicate = false); + // Derive current-level meta-only read mode from an explicit access-path split. Meta-only is + // valid only when this iterator had no data-read requirement before applying the current paths, + // no current DATA path exists, and no path must be routed to a descendant iterator. + Status _check_and_set_meta_read_mode(ReadRequirement requirement_before_access_path, + const AccessPathSplit& all_access_paths); + + // Apply the common current-level access-path state transitions and select a supported + // parent-owned meta-only mode. When that mode skips data descendants, synchronously invoke the + // callback once with SKIP before returning the routing plan. The callback is never retained. + Result _prepare_nested_access_paths( + const TColumnAccessPaths& all_access_paths, + const TColumnAccessPaths& predicate_access_paths, NestedMetaSupport meta_support, + const std::function& set_all_data_descendants_read_requirement); + + // Normalize the wire encoding, strip this iterator's column name, and explicitly partition + // paths consumed by this iterator from paths that must be routed to descendants. This helper is + // intentionally side-effect free; callers apply DATA/predicate read requirements explicitly. + Result _split_access_paths(TColumnAccessPaths access_paths) const; ColumnIteratorOptions _opts; ReadRequirement _read_requirement {ReadRequirement::NORMAL}; @@ -542,6 +573,9 @@ class FileColumnIterator : public ColumnIterator { Status read_by_rowids(const rowid_t* rowids, const size_t count, MutableColumnPtr& dst) override; + Status set_access_paths(const TColumnAccessPaths& all_access_paths, + const TColumnAccessPaths& predicate_access_paths) override; + ordinal_t get_current_ordinal() const override { return _current_ordinal; } // get row ranges by zone map @@ -618,10 +652,10 @@ class EmptyFileColumnIterator final : public ColumnIterator { ordinal_t get_current_ordinal() const override { return 0; } }; -// StringFileColumnIterator extends FileColumnIterator with meta-only reading -// support for string/binary column types. When the OFFSET path is detected in -// set_access_paths, it sets only_read_offsets on the ColumnIteratorOptions so -// that the BinaryPlainPageDecoder skips chars memcpy and only fills offsets. +// StringFileColumnIterator extends FileColumnIterator's NULL metadata support with OFFSET-only +// reading for string/binary column types. When the OFFSET path is detected in set_access_paths, it +// sets only_read_offsets on the ColumnIteratorOptions so that the BinaryPlainPageDecoder skips +// chars memcpy and only fills offsets. class StringFileColumnIterator final : public FileColumnIterator { public: explicit StringFileColumnIterator(std::shared_ptr reader); diff --git a/be/test/runtime/descriptor_test.cpp b/be/test/runtime/descriptor_test.cpp index 20c2b1000c8303..0a665cfc34c343 100644 --- a/be/test/runtime/descriptor_test.cpp +++ b/be/test/runtime/descriptor_test.cpp @@ -15,8 +15,10 @@ // specific language governing permissions and limitations // under the License. +#include #include #include +#include #include #include "common/exception.h" @@ -196,4 +198,59 @@ TEST_F(SlotDescriptorTest, DebugString) { EXPECT_TRUE(debug_str2.find("is_virtual=true") != std::string::npos); } -} // namespace doris \ No newline at end of file +TEST_F(SlotDescriptorTest, AccessPathsPreservedThroughProtobuf) { + TColumnAccessPath data_path; + data_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + data_path.__set_type(TAccessPathType::DATA); + TDataAccessPath data_payload; + data_payload.__set_path({"s", "field"}); + data_path.__set_data_access_path(data_payload); + + TColumnAccessPath meta_path; + meta_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + meta_path.__set_type(TAccessPathType::META); + TMetaAccessPath meta_payload; + meta_payload.__set_path({"s", "field", "NULL"}); + meta_path.__set_meta_access_path(meta_payload); + + TColumnAccessPath legacy_path; + legacy_path.__set_type(TAccessPathType::DATA); + data_payload.__set_path({"s", "legacy", "NULL"}); + legacy_path.__set_data_access_path(data_payload); + + TColumnAccessPath explicit_legacy_path; + explicit_legacy_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + explicit_legacy_path.__set_type(TAccessPathType::DATA); + data_payload.__set_path({"s", "legacy", "VALUES"}); + explicit_legacy_path.__set_data_access_path(data_payload); + + TSlotDescriptor thrift_descriptor = create_basic_slot_descriptor(); + thrift_descriptor.__set_all_access_paths( + {data_path, meta_path, legacy_path, explicit_legacy_path}); + thrift_descriptor.__set_predicate_access_paths({meta_path}); + + SlotDescriptor original(thrift_descriptor); + PSlotDescriptor protobuf_descriptor; + original.to_protobuf(&protobuf_descriptor); + ASSERT_EQ(protobuf_descriptor.all_access_paths_size(), 4); + EXPECT_TRUE(protobuf_descriptor.all_access_paths(0).has_version()); + EXPECT_EQ(protobuf_descriptor.all_access_paths(0).version(), + g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + EXPECT_TRUE(protobuf_descriptor.all_access_paths(1).has_version()); + EXPECT_FALSE(protobuf_descriptor.all_access_paths(2).has_version()); + EXPECT_TRUE(protobuf_descriptor.all_access_paths(3).has_version()); + EXPECT_EQ(protobuf_descriptor.all_access_paths(3).version(), + g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + SlotDescriptor round_trip(protobuf_descriptor); + + EXPECT_EQ(round_trip.all_access_paths(), thrift_descriptor.all_access_paths); + EXPECT_EQ(round_trip.predicate_access_paths(), thrift_descriptor.predicate_access_paths); + EXPECT_FALSE(round_trip.all_access_paths()[1].__isset.data_access_path); + EXPECT_FALSE(round_trip.predicate_access_paths()[0].__isset.data_access_path); + EXPECT_FALSE(round_trip.all_access_paths()[2].__isset.version); + EXPECT_TRUE(round_trip.all_access_paths()[3].__isset.version); + EXPECT_EQ(round_trip.all_access_paths()[3].version, + g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); +} + +} // namespace doris diff --git a/be/test/storage/segment/column_reader_test.cpp b/be/test/storage/segment/column_reader_test.cpp index 3dfe42ac7cd4d2..d46e04f629ce9b 100644 --- a/be/test/storage/segment/column_reader_test.cpp +++ b/be/test/storage/segment/column_reader_test.cpp @@ -16,6 +16,7 @@ // under the License. #include "storage/segment/column_reader.h" +#include #include #include #include @@ -73,14 +74,16 @@ class TestColumnIterator final : public ColumnIterator { _read_requirement = requirement; } - Result get_sub_access_paths(const TColumnAccessPaths& access_paths, - bool is_predicate = false) { - return _get_sub_access_paths(access_paths, is_predicate); + using ColumnIterator::AccessPathSplit; + + Result split_access_paths(const TColumnAccessPaths& access_paths) const { + return _split_access_paths(access_paths); } - void check_and_set_meta_read_mode(ReadRequirement requirement_before_access_path, - const TColumnAccessPaths& access_paths) { - _check_and_set_meta_read_mode(requirement_before_access_path, access_paths); + Status check_and_set_meta_read_mode(ReadRequirement requirement_before_access_path, + const TColumnAccessPaths& access_paths) { + auto split = DORIS_TRY(_split_access_paths(access_paths)); + return _check_and_set_meta_read_mode(requirement_before_access_path, split); } void convert_to_place_holder_column(MutableColumnPtr& dst, size_t count) { @@ -88,14 +91,35 @@ class TestColumnIterator final : public ColumnIterator { } }; -TColumnAccessPath create_access_path(std::vector path) { +TColumnAccessPath create_data_access_path(std::vector path) { + TColumnAccessPath access_path; + access_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + access_path.__set_type(TAccessPathType::DATA); + TDataAccessPath data_access_path; + data_access_path.__set_path(std::move(path)); + access_path.__set_data_access_path(std::move(data_access_path)); + return access_path; +} + +TColumnAccessPath create_legacy_data_access_path(std::vector path) { TColumnAccessPath access_path; + access_path.__set_type(TAccessPathType::DATA); TDataAccessPath data_access_path; data_access_path.__set_path(std::move(path)); access_path.__set_data_access_path(std::move(data_access_path)); return access_path; } +TColumnAccessPath create_meta_access_path(std::vector path) { + TColumnAccessPath access_path; + access_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + access_path.__set_type(TAccessPathType::META); + TMetaAccessPath meta_access_path; + meta_access_path.__set_path(std::move(path)); + access_path.__set_meta_access_path(std::move(meta_access_path)); + return access_path; +} + std::shared_ptr create_test_reader( bool is_nullable = false, uint64_t num_rows = 0, FieldType field_type = FieldType::OLAP_FIELD_TYPE_INT) { @@ -148,6 +172,13 @@ class TrackingColumnIterator final : public ColumnIterator { ordinal_t get_current_ordinal() const override { return _current_ordinal; } + Status set_access_paths(const TColumnAccessPaths& all_access_paths, + const TColumnAccessPaths& predicate_access_paths) override { + routed_all_access_paths = all_access_paths; + routed_predicate_access_paths = predicate_access_paths; + return ColumnIterator::set_access_paths(all_access_paths, predicate_access_paths); + } + void collect_prefetchers( std::map>& prefetchers, PrefetcherInitMethod init_method) override { @@ -164,12 +195,16 @@ class TrackingColumnIterator final : public ColumnIterator { next_batch_sizes.clear(); read_by_rowids_batches.clear(); collect_methods.clear(); + routed_all_access_paths.clear(); + routed_predicate_access_paths.clear(); } std::vector seek_ordinals; std::vector next_batch_sizes; std::vector> read_by_rowids_batches; std::vector collect_methods; + TColumnAccessPaths routed_all_access_paths; + TColumnAccessPaths routed_predicate_access_paths; private: void record_collect_method(PrefetcherInitMethod init_method) { @@ -452,9 +487,9 @@ TEST_F(ColumnReaderTest, StructAccessPaths) { // Only reading sub_col_1 // sub_col_2 should be set to SKIP - all_access_paths[0].data_access_path.path = {"self", "sub_col_1"}; + all_access_paths[0] = create_data_access_path({"self", "sub_col_1"}); - predicate_access_paths[0].data_access_path.path = {"self", "sub_col_1"}; + predicate_access_paths[0] = create_data_access_path({"self", "sub_col_1"}); st = iterator->set_access_paths(all_access_paths, predicate_access_paths); // invalid name leads to error @@ -472,7 +507,7 @@ TEST_F(ColumnReaderTest, StructAccessPaths) { ColumnIterator::ReadRequirement::SKIP); // Reading all sub columns - all_access_paths[0].data_access_path.path = {"self"}; + all_access_paths[0] = create_data_access_path({"self"}); iterator = create_struct_iterator(); iterator->set_column_name("self"); st = iterator->set_access_paths(all_access_paths, predicate_access_paths); @@ -548,35 +583,383 @@ TEST_F(ColumnReaderTest, MetaReadModePrefersOffsetOverNull) { auto assert_meta_read_mode = [](TColumnAccessPaths access_paths, bool offset_only, bool null_map_only) { TestColumnIterator iterator; - iterator.check_and_set_meta_read_mode(ColumnIterator::ReadRequirement::NORMAL, - access_paths); + iterator.set_column_name("self"); + auto st = iterator.check_and_set_meta_read_mode(ColumnIterator::ReadRequirement::NORMAL, + access_paths); + ASSERT_TRUE(st.ok()) << st.to_string(); EXPECT_EQ(iterator.read_offset_only(), offset_only); EXPECT_EQ(iterator.read_null_map_only(), null_map_only); }; assert_meta_read_mode(TColumnAccessPaths {}, false, false); - assert_meta_read_mode(TColumnAccessPaths {create_access_path({ColumnIterator::ACCESS_OFFSET})}, - true, false); - assert_meta_read_mode(TColumnAccessPaths {create_access_path({ColumnIterator::ACCESS_NULL})}, - false, true); - assert_meta_read_mode(TColumnAccessPaths {create_access_path({ColumnIterator::ACCESS_OFFSET}), - create_access_path({ColumnIterator::ACCESS_NULL})}, - true, false); - assert_meta_read_mode(TColumnAccessPaths {create_access_path({"child"})}, false, false); - assert_meta_read_mode(TColumnAccessPaths {create_access_path({ColumnIterator::ACCESS_OFFSET}), - create_access_path({"child"})}, - false, false); + assert_meta_read_mode( + TColumnAccessPaths {create_meta_access_path({"self", ColumnIterator::ACCESS_OFFSET})}, + true, false); + assert_meta_read_mode( + TColumnAccessPaths {create_meta_access_path({"self", ColumnIterator::ACCESS_NULL})}, + false, true); + assert_meta_read_mode( + TColumnAccessPaths {create_meta_access_path({"self", ColumnIterator::ACCESS_OFFSET}), + create_meta_access_path({"self", ColumnIterator::ACCESS_NULL})}, + true, false); + assert_meta_read_mode(TColumnAccessPaths {create_data_access_path({"self", "child"})}, false, + false); + assert_meta_read_mode( + TColumnAccessPaths {create_data_access_path({"self", ColumnIterator::ACCESS_OFFSET})}, + false, false); + assert_meta_read_mode( + TColumnAccessPaths {create_data_access_path({"self", ColumnIterator::ACCESS_NULL})}, + false, false); + assert_meta_read_mode( + TColumnAccessPaths {create_meta_access_path({"self", ColumnIterator::ACCESS_OFFSET}), + create_data_access_path({"self", "child"})}, + false, false); { TestColumnIterator iterator; - iterator.check_and_set_meta_read_mode( + iterator.set_column_name("self"); + auto st = iterator.check_and_set_meta_read_mode( ColumnIterator::ReadRequirement::LAZY_OUTPUT, - TColumnAccessPaths {create_access_path({ColumnIterator::ACCESS_NULL})}); + TColumnAccessPaths { + create_meta_access_path({"self", ColumnIterator::ACCESS_NULL})}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_FALSE(iterator.read_null_map_only()); + EXPECT_FALSE(iterator.read_offset_only()); + } +} + +TEST_F(ColumnReaderTest, TypedAccessPathRequiresMatchingPayload) { + FileColumnIterator iterator(create_test_reader(true)); + iterator.set_column_name("c"); + + TColumnAccessPath unknown_type_path; + unknown_type_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + TDataAccessPath data_access_path; + data_access_path.__set_path({"c"}); + unknown_type_path.__set_data_access_path(data_access_path); + auto st = iterator.set_access_paths({unknown_type_path}, {}); + EXPECT_FALSE(st.ok()); + EXPECT_THAT(st.to_string(), ::testing::HasSubstr("Invalid access path type")); + + TColumnAccessPath missing_meta_payload; + missing_meta_payload.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + missing_meta_payload.__set_type(TAccessPathType::META); + missing_meta_payload.__set_data_access_path(data_access_path); + st = iterator.set_access_paths({missing_meta_payload}, {}); + EXPECT_FALSE(st.ok()); + EXPECT_THAT(st.to_string(), ::testing::HasSubstr("meta_access_path payload is not set")); + + TColumnAccessPath missing_data_payload; + missing_data_payload.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + missing_data_payload.__set_type(TAccessPathType::DATA); + TMetaAccessPath meta_access_path; + meta_access_path.__set_path({"c", ColumnIterator::ACCESS_NULL}); + missing_data_payload.__set_meta_access_path(meta_access_path); + st = iterator.set_access_paths({missing_data_payload}, {}); + EXPECT_FALSE(st.ok()); + EXPECT_THAT(st.to_string(), ::testing::HasSubstr("data_access_path payload is not set")); + + TColumnAccessPath missing_legacy_payload; + missing_legacy_payload.__set_type(TAccessPathType::DATA); + st = iterator.set_access_paths({missing_legacy_payload}, {}); + EXPECT_FALSE(st.ok()); + EXPECT_THAT(st.to_string(), + ::testing::HasSubstr( + "Invalid legacy access path: data_access_path payload is not set")); + + for (const bool explicit_legacy_version : {false, true}) { + TColumnAccessPath invalid_legacy_type; + invalid_legacy_type.__set_type(TAccessPathType::META); + invalid_legacy_type.__set_data_access_path(data_access_path); + invalid_legacy_type.__set_meta_access_path(meta_access_path); + if (explicit_legacy_version) { + invalid_legacy_type.__set_version( + g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + } + st = iterator.set_access_paths({invalid_legacy_type}, {}); + EXPECT_FALSE(st.ok()); + EXPECT_THAT(st.to_string(), ::testing::HasSubstr("Invalid legacy access path type")); + } + + FileColumnIterator data_precedence_iterator(create_test_reader(true)); + data_precedence_iterator.set_column_name("c"); + auto compatible_data_path = create_data_access_path({"c"}); + compatible_data_path.__set_meta_access_path(meta_access_path); + st = data_precedence_iterator.set_access_paths({compatible_data_path}, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_FALSE(data_precedence_iterator.read_null_map_only()); + EXPECT_FALSE(data_precedence_iterator.read_offset_only()); + + auto compatible_meta_path = create_meta_access_path({"c", ColumnIterator::ACCESS_NULL}); + compatible_meta_path.__set_data_access_path(data_access_path); + st = iterator.set_access_paths({compatible_meta_path}, {compatible_meta_path}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_TRUE(iterator.read_null_map_only()); + + FileColumnIterator wrong_root_iterator(create_test_reader(true)); + wrong_root_iterator.set_column_name("c"); + auto wrong_root_path = create_meta_access_path({"other", ColumnIterator::ACCESS_NULL}); + st = wrong_root_iterator.set_access_paths({}, {wrong_root_path}); + EXPECT_FALSE(st.ok()); + EXPECT_THAT(st.to_string(), ::testing::HasSubstr("expected name \"c\", got \"other\"")); + + auto unsupported_version_path = create_data_access_path({"c"}); + unsupported_version_path.__set_version( + g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED + 1); + st = iterator.set_access_paths({unsupported_version_path}, {}); + EXPECT_FALSE(st.ok()); + EXPECT_THAT(st.to_string(), ::testing::HasSubstr("Unsupported access path version")); +} + +TEST_F(ColumnReaderTest, AccessPathVersionControlsLegacyMetaEncoding) { + for (const bool explicit_legacy_version : {false, true}) { + SCOPED_TRACE(explicit_legacy_version ? "explicit-version-0" : "missing-version"); + FileColumnIterator iterator(create_test_reader(true)); + iterator.set_column_name("c"); + auto legacy_null_path = create_legacy_data_access_path({"c", ColumnIterator::ACCESS_NULL}); + if (explicit_legacy_version) { + legacy_null_path.__set_version( + g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + } + auto st = iterator.set_access_paths({legacy_null_path}, {legacy_null_path}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_TRUE(iterator.read_null_map_only()); + } + + { + FileColumnIterator iterator(create_test_reader(true)); + iterator.set_column_name("c"); + auto typed_data_path = create_data_access_path({"c", ColumnIterator::ACCESS_NULL}); + auto st = iterator.set_access_paths({typed_data_path}, {typed_data_path}); + ASSERT_TRUE(st.ok()) << st.to_string(); EXPECT_FALSE(iterator.read_null_map_only()); EXPECT_FALSE(iterator.read_offset_only()); } } +TEST_F(ColumnReaderTest, LegacyDataSpecialComponentsRemainDataSelectors) { + auto make_map_iterator = []() { + auto offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + auto key_iterator = std::make_unique(create_test_reader()); + key_iterator->set_column_name("key"); + auto value_iterator = std::make_unique(create_test_reader()); + value_iterator->set_column_name("value"); + auto map_iterator = std::make_unique( + create_test_reader(), nullptr, std::move(offsets_iterator), std::move(key_iterator), + std::move(value_iterator)); + map_iterator->set_column_name("m"); + return map_iterator; + }; + + struct SelectorCase { + const char* component; + bool explicit_legacy_version; + ColumnIterator::ReadRequirement key_requirement; + ColumnIterator::ReadRequirement value_requirement; + }; + for (const auto& test_case : {SelectorCase {ColumnIterator::ACCESS_MAP_KEYS, false, + ColumnIterator::ReadRequirement::LAZY_OUTPUT, + ColumnIterator::ReadRequirement::SKIP}, + SelectorCase {ColumnIterator::ACCESS_MAP_VALUES, true, + ColumnIterator::ReadRequirement::SKIP, + ColumnIterator::ReadRequirement::LAZY_OUTPUT}}) { + SCOPED_TRACE(test_case.component); + auto map_iterator = make_map_iterator(); + auto path = create_legacy_data_access_path({"m", test_case.component}); + if (test_case.explicit_legacy_version) { + path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + } + + auto st = map_iterator->set_access_paths({path}, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_FALSE(map_iterator->read_null_map_only()); + EXPECT_FALSE(map_iterator->read_offset_only()); + EXPECT_EQ(map_iterator->_key_iterator->read_requirement(), test_case.key_requirement); + EXPECT_EQ(map_iterator->_val_iterator->read_requirement(), test_case.value_requirement); + } + + // `*` is also a legacy DATA selector. It keeps keys readable while OFFSET is promoted only + // after the remaining path reaches the value iterator. + auto map_iterator = make_map_iterator(); + auto offset_path = create_legacy_data_access_path( + {"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET}); + offset_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + auto st = map_iterator->set_access_paths({offset_path}, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_EQ(map_iterator->_key_iterator->read_requirement(), + ColumnIterator::ReadRequirement::LAZY_OUTPUT); + EXPECT_TRUE(map_iterator->_val_iterator->read_offset_only()); +} + +TEST_F(ColumnReaderTest, TypedMetaPathsRouteThroughScalarAndComplexIterators) { + { + FileColumnIterator scalar_iterator(create_test_reader(true)); + scalar_iterator.set_column_name("i"); + TColumnAccessPaths null_path {create_meta_access_path({"i", ColumnIterator::ACCESS_NULL})}; + auto st = scalar_iterator.set_access_paths(null_path, null_path); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_TRUE(scalar_iterator.read_null_map_only()); + } + + { + auto item_iterator = std::make_unique(create_test_reader()); + item_iterator->set_column_name("item"); + auto offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + ArrayFileColumnIterator array_iterator(create_test_reader(), std::move(offsets_iterator), + std::move(item_iterator), nullptr); + array_iterator.set_column_name("a"); + + TColumnAccessPaths offset_path { + create_meta_access_path({"a", ColumnIterator::ACCESS_OFFSET})}; + auto st = array_iterator.set_access_paths(offset_path, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_TRUE(array_iterator.read_offset_only()); + EXPECT_EQ(array_iterator._item_iterator->read_requirement(), + ColumnIterator::ReadRequirement::SKIP); + } + + { + auto offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + auto key_iterator = std::make_unique(create_test_reader()); + key_iterator->set_column_name("key"); + auto value_iterator = std::make_unique(create_test_reader()); + value_iterator->set_column_name("value"); + MapFileColumnIterator map_iterator(create_test_reader(), nullptr, + std::move(offsets_iterator), std::move(key_iterator), + std::move(value_iterator)); + map_iterator.set_column_name("m"); + + TColumnAccessPaths offset_path { + create_meta_access_path({"m", ColumnIterator::ACCESS_OFFSET})}; + auto st = map_iterator.set_access_paths(offset_path, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_TRUE(map_iterator.read_offset_only()); + EXPECT_EQ(map_iterator._key_iterator->read_requirement(), + ColumnIterator::ReadRequirement::SKIP); + EXPECT_EQ(map_iterator._val_iterator->read_requirement(), + ColumnIterator::ReadRequirement::SKIP); + } + + { + std::vector sub_iterators; + auto string_iterator = std::make_unique(create_test_reader()); + string_iterator->set_column_name("text"); + sub_iterators.emplace_back(std::move(string_iterator)); + StructFileColumnIterator struct_iterator(create_test_reader(), nullptr, + std::move(sub_iterators)); + struct_iterator.set_column_name("s"); + + TColumnAccessPaths offset_path { + create_meta_access_path({"s", "text", ColumnIterator::ACCESS_OFFSET})}; + auto st = struct_iterator.set_access_paths(offset_path, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + auto* text_iterator = static_cast( + struct_iterator._sub_column_iterators[0].get()); + EXPECT_TRUE(text_iterator->read_offset_only()); + } + + { + auto item_iterator = std::make_unique(create_test_reader()); + item_iterator->set_column_name("item"); + auto offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + ArrayFileColumnIterator array_iterator(create_test_reader(), std::move(offsets_iterator), + std::move(item_iterator), nullptr); + array_iterator.set_column_name("a"); + + TColumnAccessPaths offset_path {create_meta_access_path( + {"a", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET})}; + auto st = array_iterator.set_access_paths(offset_path, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + auto* typed_item_iterator = + static_cast(array_iterator._item_iterator.get()); + EXPECT_TRUE(typed_item_iterator->read_offset_only()); + } + + { + auto offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + auto key_iterator = std::make_unique(create_test_reader()); + key_iterator->set_column_name("key"); + auto value_iterator = std::make_unique(create_test_reader()); + value_iterator->set_column_name("value"); + MapFileColumnIterator map_iterator(create_test_reader(), nullptr, + std::move(offsets_iterator), std::move(key_iterator), + std::move(value_iterator)); + map_iterator.set_column_name("m"); + + TColumnAccessPaths offset_path {create_meta_access_path( + {"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET})}; + auto st = map_iterator.set_access_paths(offset_path, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + auto* typed_key_iterator = + static_cast(map_iterator._key_iterator.get()); + auto* typed_value_iterator = + static_cast(map_iterator._val_iterator.get()); + EXPECT_FALSE(typed_key_iterator->read_offset_only()); + EXPECT_EQ(typed_key_iterator->read_requirement(), + ColumnIterator::ReadRequirement::LAZY_OUTPUT); + EXPECT_TRUE(typed_value_iterator->read_offset_only()); + } +} + +TEST_F(ColumnReaderTest, TypedDataFieldsNamedMetaComponentsAreNotTreatedAsMetaPaths) { + auto make_struct_iterator = [](const std::string& field_name) { + std::vector sub_iterators; + auto field_iterator = std::make_unique(create_test_reader()); + field_iterator->set_column_name(field_name); + sub_iterators.emplace_back(std::move(field_iterator)); + auto struct_iterator = std::make_unique( + create_test_reader(), nullptr, std::move(sub_iterators)); + struct_iterator->set_column_name("s"); + return struct_iterator; + }; + + for (const std::string field_name : + {ColumnIterator::ACCESS_NULL, ColumnIterator::ACCESS_OFFSET}) { + SCOPED_TRACE(field_name); + auto struct_iterator = make_struct_iterator(field_name); + TColumnAccessPaths data_path {create_data_access_path({"s", field_name})}; + auto st = struct_iterator->set_access_paths(data_path, data_path); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_FALSE(struct_iterator->read_null_map_only()); + EXPECT_FALSE(struct_iterator->read_offset_only()); + ASSERT_EQ(struct_iterator->_sub_column_iterators.size(), 1); + EXPECT_EQ(struct_iterator->_sub_column_iterators[0]->read_requirement(), + ColumnIterator::ReadRequirement::PREDICATE); + } + + auto struct_iterator = make_struct_iterator(ColumnIterator::ACCESS_NULL); + TColumnAccessPaths meta_path {create_meta_access_path({"s", ColumnIterator::ACCESS_NULL})}; + auto st = struct_iterator->set_access_paths(meta_path, meta_path); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_TRUE(struct_iterator->read_null_map_only()); + EXPECT_EQ(struct_iterator->_sub_column_iterators[0]->read_requirement(), + ColumnIterator::ReadRequirement::SKIP); +} + +TEST_F(ColumnReaderTest, LegacyStructMetaComponentsRemainSentinels) { + auto null_iterator = std::make_unique(create_test_reader()); + std::vector sub_iterators; + auto field_iterator = std::make_unique(create_test_reader()); + field_iterator->set_column_name(ColumnIterator::ACCESS_NULL); + sub_iterators.emplace_back(std::move(field_iterator)); + StructFileColumnIterator struct_iterator(create_test_reader(), std::move(null_iterator), + std::move(sub_iterators)); + struct_iterator.set_column_name("s"); + + auto legacy_path = create_legacy_data_access_path({"s", ColumnIterator::ACCESS_NULL}); + legacy_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + auto st = struct_iterator.set_access_paths({legacy_path}, {legacy_path}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_TRUE(struct_iterator.read_null_map_only()); + EXPECT_EQ(struct_iterator._sub_column_iterators[0]->read_requirement(), + ColumnIterator::ReadRequirement::SKIP); +} + TEST_F(ColumnReaderTest, PlaceHolderLifecycleInLazyMode) { TestColumnIterator iterator; iterator.force_set_read_requirement(ColumnIterator::ReadRequirement::LAZY_OUTPUT); @@ -890,23 +1273,190 @@ TEST_F(ColumnReaderTest, FinalizeLazyModeOnNestedStruct) { EXPECT_EQ(0, nested_after->size()); } -TEST_F(ColumnReaderTest, GetSubAccessPathsSetsPredicateFlag) { +TEST_F(ColumnReaderTest, SplitAccessPathsClassifiesCurrentAndDescendantPaths) { TestColumnIterator iterator; iterator.set_column_name("self"); + iterator.force_set_read_requirement(ColumnIterator::ReadRequirement::NORMAL); - TColumnAccessPaths access_paths; - access_paths.emplace_back(); - access_paths[0].data_access_path.path = {"self"}; + { + auto split = TEST_TRY(iterator.split_access_paths( + TColumnAccessPaths {create_data_access_path({"self"})})); + EXPECT_TRUE(split.reads_current_data); + EXPECT_EQ(split.current_meta_mode, ColumnIterator::MetaReadMode::DEFAULT); + EXPECT_TRUE(split.descendant_paths.empty()); + } - iterator.force_set_read_requirement(ColumnIterator::ReadRequirement::NORMAL); - auto sub_paths = TEST_TRY(iterator.get_sub_access_paths(access_paths)); - EXPECT_TRUE(sub_paths.empty()); - EXPECT_EQ(iterator._read_requirement, ColumnIterator::ReadRequirement::LAZY_OUTPUT); + { + auto split = TEST_TRY(iterator.split_access_paths(TColumnAccessPaths { + create_meta_access_path({"self", ColumnIterator::ACCESS_NULL}), + create_meta_access_path({"self", ColumnIterator::ACCESS_OFFSET})})); + EXPECT_FALSE(split.reads_current_data); + EXPECT_EQ(split.current_meta_mode, ColumnIterator::MetaReadMode::OFFSET_ONLY); + EXPECT_TRUE(split.descendant_paths.empty()); + } - iterator.force_set_read_requirement(ColumnIterator::ReadRequirement::NORMAL); - sub_paths = TEST_TRY(iterator.get_sub_access_paths(access_paths, true)); - EXPECT_TRUE(sub_paths.empty()); - EXPECT_EQ(iterator._read_requirement, ColumnIterator::ReadRequirement::PREDICATE); + { + auto split = TEST_TRY(iterator.split_access_paths(TColumnAccessPaths { + create_data_access_path({"self", "child"}), + create_meta_access_path({"self", "child", ColumnIterator::ACCESS_NULL})})); + EXPECT_FALSE(split.reads_current_data); + EXPECT_EQ(split.current_meta_mode, ColumnIterator::MetaReadMode::DEFAULT); + ASSERT_EQ(split.descendant_paths.size(), 2); + EXPECT_EQ(split.descendant_paths[0].type, TAccessPathType::DATA); + EXPECT_EQ(split.descendant_paths[0].data_access_path.path, + (std::vector {"child"})); + EXPECT_EQ(split.descendant_paths[1].type, TAccessPathType::META); + EXPECT_EQ(split.descendant_paths[1].meta_access_path.path, + (std::vector {"child", ColumnIterator::ACCESS_NULL})); + } + + { + auto legacy_null = create_legacy_data_access_path({"self", ColumnIterator::ACCESS_NULL}); + auto legacy_offset = + create_legacy_data_access_path({"self", ColumnIterator::ACCESS_OFFSET}); + legacy_offset.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + auto split = TEST_TRY(iterator.split_access_paths( + TColumnAccessPaths {std::move(legacy_null), std::move(legacy_offset)})); + EXPECT_FALSE(split.reads_current_data); + EXPECT_EQ(split.current_meta_mode, ColumnIterator::MetaReadMode::OFFSET_ONLY); + EXPECT_TRUE(split.descendant_paths.empty()); + } + + { + auto legacy_keys = + create_legacy_data_access_path({"self", ColumnIterator::ACCESS_MAP_KEYS}); + auto split = TEST_TRY(iterator.split_access_paths( + TColumnAccessPaths {create_data_access_path({"self", ColumnIterator::ACCESS_NULL}), + std::move(legacy_keys)})); + EXPECT_FALSE(split.reads_current_data); + EXPECT_EQ(split.current_meta_mode, ColumnIterator::MetaReadMode::DEFAULT); + ASSERT_EQ(split.descendant_paths.size(), 2); + EXPECT_EQ(split.descendant_paths[0].data_access_path.path, + (std::vector {ColumnIterator::ACCESS_NULL})); + EXPECT_EQ(split.descendant_paths[1].data_access_path.path, + (std::vector {ColumnIterator::ACCESS_MAP_KEYS})); + } + + // Splitting is a pure classification step. Callers explicitly apply lazy/predicate state. + EXPECT_EQ(iterator._read_requirement, ColumnIterator::ReadRequirement::NORMAL); +} + +TEST_F(ColumnReaderTest, MapDescendantRoutingPreservesActivePayloadAndVersion) { + auto key_iterator = std::make_unique(); + key_iterator->set_column_name("key"); + auto* key = key_iterator.get(); + auto value_iterator = std::make_unique(); + value_iterator->set_column_name("value"); + auto* value = value_iterator.get(); + auto offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + MapFileColumnIterator map_iterator(create_test_reader(), nullptr, std::move(offsets_iterator), + std::move(key_iterator), std::move(value_iterator)); + map_iterator.set_column_name("m"); + + auto path = create_meta_access_path( + {"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET}); + TDataAccessPath inactive_data_payload; + inactive_data_payload.__set_path(path.meta_access_path.path); + path.__set_data_access_path(std::move(inactive_data_payload)); + auto legacy_path = create_data_access_path( + {"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET}); + legacy_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + + auto st = map_iterator.set_access_paths({path}, {legacy_path}); + ASSERT_TRUE(st.ok()) << st.to_string(); + + ASSERT_EQ(key->routed_all_access_paths.size(), 1); + const auto& key_path = key->routed_all_access_paths[0]; + EXPECT_EQ(key_path.type, TAccessPathType::DATA); + ASSERT_TRUE(key_path.__isset.version); + EXPECT_EQ(key_path.version, g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + EXPECT_EQ(key_path.data_access_path.path, (std::vector {"key"})); + ASSERT_EQ(key->routed_predicate_access_paths.size(), 1); + const auto& legacy_key_path = key->routed_predicate_access_paths[0]; + EXPECT_EQ(legacy_key_path.type, TAccessPathType::DATA); + ASSERT_TRUE(legacy_key_path.__isset.version); + EXPECT_EQ(legacy_key_path.version, g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + EXPECT_EQ(legacy_key_path.data_access_path.path, (std::vector {"key"})); + + ASSERT_EQ(value->routed_all_access_paths.size(), 1); + const auto& value_path = value->routed_all_access_paths[0]; + EXPECT_EQ(value_path.type, TAccessPathType::META); + ASSERT_TRUE(value_path.__isset.version); + EXPECT_EQ(value_path.version, g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + EXPECT_EQ(value_path.meta_access_path.path, + (std::vector {"value", ColumnIterator::ACCESS_OFFSET})); + EXPECT_EQ(value_path.data_access_path.path, + (std::vector {"m", ColumnIterator::ACCESS_ALL, + ColumnIterator::ACCESS_OFFSET})); + ASSERT_EQ(value->routed_predicate_access_paths.size(), 1); + const auto& legacy_value_path = value->routed_predicate_access_paths[0]; + EXPECT_EQ(legacy_value_path.type, TAccessPathType::DATA); + ASSERT_TRUE(legacy_value_path.__isset.version); + EXPECT_EQ(legacy_value_path.version, + g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + EXPECT_EQ(legacy_value_path.data_access_path.path, + (std::vector {"value", ColumnIterator::ACCESS_OFFSET})); +} + +TEST_F(ColumnReaderTest, ArrayDescendantRoutingRewritesOnlyActivePayload) { + auto item_iterator = std::make_unique(); + item_iterator->set_column_name("item"); + auto* item = item_iterator.get(); + auto offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + ArrayFileColumnIterator array_iterator(create_test_reader(), std::move(offsets_iterator), + std::move(item_iterator), nullptr); + array_iterator.set_column_name("a"); + + auto path = create_meta_access_path( + {"a", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET}); + TDataAccessPath inactive_data_payload; + inactive_data_payload.__set_path(path.meta_access_path.path); + path.__set_data_access_path(std::move(inactive_data_payload)); + + auto st = array_iterator.set_access_paths({path}, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + + ASSERT_EQ(item->routed_all_access_paths.size(), 1); + const auto& item_path = item->routed_all_access_paths[0]; + EXPECT_EQ(item_path.type, TAccessPathType::META); + ASSERT_TRUE(item_path.__isset.version); + EXPECT_EQ(item_path.version, g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); + EXPECT_EQ(item_path.meta_access_path.path, + (std::vector {"item", ColumnIterator::ACCESS_OFFSET})); + EXPECT_EQ(item_path.data_access_path.path, + (std::vector {"a", ColumnIterator::ACCESS_ALL, + ColumnIterator::ACCESS_OFFSET})); +} + +TEST_F(ColumnReaderTest, StructCurrentMetaDoesNotRouteToDataFieldWithSameName) { + std::vector sub_iterators; + auto null_field_iterator = std::make_unique(); + null_field_iterator->set_column_name(ColumnIterator::ACCESS_NULL); + auto* null_field = null_field_iterator.get(); + sub_iterators.emplace_back(std::move(null_field_iterator)); + auto data_field_iterator = std::make_unique(); + data_field_iterator->set_column_name("data"); + auto* data_field = data_field_iterator.get(); + sub_iterators.emplace_back(std::move(data_field_iterator)); + + StructFileColumnIterator struct_iterator(create_test_reader(), nullptr, + std::move(sub_iterators)); + struct_iterator.set_column_name("s"); + TColumnAccessPaths all_access_paths { + create_meta_access_path({"s", ColumnIterator::ACCESS_NULL}), + create_data_access_path({"s", "data"})}; + TColumnAccessPaths predicate_access_paths {create_data_access_path({"s", "data"})}; + + auto st = struct_iterator.set_access_paths(all_access_paths, predicate_access_paths); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_TRUE(null_field->routed_all_access_paths.empty()); + EXPECT_TRUE(null_field->routed_predicate_access_paths.empty()); + EXPECT_EQ(null_field->read_requirement(), ColumnIterator::ReadRequirement::SKIP); + ASSERT_EQ(data_field->routed_all_access_paths.size(), 1); + ASSERT_EQ(data_field->routed_predicate_access_paths.size(), 1); + EXPECT_EQ(data_field->read_requirement(), ColumnIterator::ReadRequirement::PREDICATE); } TEST_F(ColumnReaderTest, NestedIteratorsPropagateReadPhase) { @@ -966,7 +1516,7 @@ TEST_F(ColumnReaderTest, AccessPathsPropagatePredicateToChildren) { TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"s"}; + all_access_paths[0] = create_data_access_path({"s"}); TColumnAccessPaths predicate_access_paths = all_access_paths; auto st = struct_iterator->set_access_paths(all_access_paths, predicate_access_paths); @@ -988,7 +1538,7 @@ TEST_F(ColumnReaderTest, AccessPathsPropagatePredicateToChildren) { array_iterator.set_column_name("a"); TColumnAccessPaths array_access_paths; array_access_paths.emplace_back(); - array_access_paths[0].data_access_path.path = {"a"}; + array_access_paths[0] = create_data_access_path({"a"}); TColumnAccessPaths array_predicate_paths = array_access_paths; st = array_iterator.set_access_paths(array_access_paths, array_predicate_paths); ASSERT_TRUE(st.ok()) << "failed to set array access paths: " << st.to_string(); @@ -1007,7 +1557,7 @@ TEST_F(ColumnReaderTest, AccessPathsPropagatePredicateToChildren) { map_iterator.set_column_name("m"); TColumnAccessPaths map_access_paths; map_access_paths.emplace_back(); - map_access_paths[0].data_access_path.path = {"m"}; + map_access_paths[0] = create_data_access_path({"m"}); TColumnAccessPaths map_predicate_paths = map_access_paths; st = map_iterator.set_access_paths(map_access_paths, map_predicate_paths); ASSERT_TRUE(st.ok()) << "failed to set map access paths: " << st.to_string(); @@ -1032,8 +1582,8 @@ TEST_F(ColumnReaderTest, StructPredicateOnlyChildPathStillRoutesToChild) { std::move(sub_iters)); struct_iterator.set_column_name("s"); - TColumnAccessPaths all_access_paths {create_access_path({"s", "a"})}; - TColumnAccessPaths predicate_access_paths {create_access_path({"s", "b"})}; + TColumnAccessPaths all_access_paths {create_data_access_path({"s", "a"})}; + TColumnAccessPaths predicate_access_paths {create_data_access_path({"s", "b"})}; auto st = struct_iterator.set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set struct access paths: " << st.to_string(); @@ -1050,7 +1600,7 @@ TEST_F(ColumnReaderTest, StructPredicateOnlyChildPathStillRoutesToChild) { EXPECT_EQ(struct_iterator._sub_column_iterators[1]->column_name(), "b"); } -TEST_F(ColumnReaderTest, CurrentLevelPredicateNullPathUsesMetaOnlyMode) { +TEST_F(ColumnReaderTest, LegacyCurrentLevelPredicateNullPathUsesMetaOnlyMode) { auto make_struct_iterator = []() { auto null_iter = std::make_unique(std::make_shared()); std::vector sub_iters; @@ -1070,9 +1620,9 @@ TEST_F(ColumnReaderTest, CurrentLevelPredicateNullPathUsesMetaOnlyMode) { { auto struct_iterator = make_struct_iterator(); TColumnAccessPaths all_access_paths { - create_access_path({"s", ColumnIterator::ACCESS_NULL})}; + create_legacy_data_access_path({"s", ColumnIterator::ACCESS_NULL})}; TColumnAccessPaths predicate_access_paths { - create_access_path({"s", ColumnIterator::ACCESS_NULL})}; + create_legacy_data_access_path({"s", ColumnIterator::ACCESS_NULL})}; auto st = struct_iterator->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set struct access paths: " << st.to_string(); @@ -1090,9 +1640,10 @@ TEST_F(ColumnReaderTest, CurrentLevelPredicateNullPathUsesMetaOnlyMode) { { auto struct_iterator = make_struct_iterator(); TColumnAccessPaths all_access_paths { - create_access_path({"s"}), create_access_path({"s", ColumnIterator::ACCESS_NULL})}; + create_legacy_data_access_path({"s"}), + create_legacy_data_access_path({"s", ColumnIterator::ACCESS_NULL})}; TColumnAccessPaths predicate_access_paths { - create_access_path({"s", ColumnIterator::ACCESS_NULL})}; + create_legacy_data_access_path({"s", ColumnIterator::ACCESS_NULL})}; auto st = struct_iterator->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set struct access paths: " << st.to_string(); @@ -1117,9 +1668,9 @@ TEST_F(ColumnReaderTest, CurrentLevelPredicateNullPathUsesMetaOnlyMode) { array_iterator.set_column_name("a"); TColumnAccessPaths all_access_paths { - create_access_path({"a", ColumnIterator::ACCESS_NULL})}; + create_legacy_data_access_path({"a", ColumnIterator::ACCESS_NULL})}; TColumnAccessPaths predicate_access_paths { - create_access_path({"a", ColumnIterator::ACCESS_NULL})}; + create_legacy_data_access_path({"a", ColumnIterator::ACCESS_NULL})}; auto st = array_iterator.set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set array access paths: " << st.to_string(); @@ -1140,9 +1691,9 @@ TEST_F(ColumnReaderTest, CurrentLevelPredicateNullPathUsesMetaOnlyMode) { map_iterator.set_column_name("m"); TColumnAccessPaths all_access_paths { - create_access_path({"m", ColumnIterator::ACCESS_NULL})}; + create_legacy_data_access_path({"m", ColumnIterator::ACCESS_NULL})}; TColumnAccessPaths predicate_access_paths { - create_access_path({"m", ColumnIterator::ACCESS_NULL})}; + create_legacy_data_access_path({"m", ColumnIterator::ACCESS_NULL})}; auto st = map_iterator.set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set map access paths: " << st.to_string(); @@ -1173,10 +1724,10 @@ TEST_F(ColumnReaderTest, StructPredicateMetaPathDoesNotOverrideExistingDataNeed) auto struct_iterator = make_struct_iterator(); TColumnAccessPaths all_access_paths { - create_access_path({"s"}), - create_access_path({"s", "city", ColumnIterator::ACCESS_NULL})}; + create_data_access_path({"s"}), + create_meta_access_path({"s", "city", ColumnIterator::ACCESS_NULL})}; TColumnAccessPaths predicate_access_paths { - create_access_path({"s", "city", ColumnIterator::ACCESS_NULL})}; + create_meta_access_path({"s", "city", ColumnIterator::ACCESS_NULL})}; auto st = struct_iterator->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set struct access paths: " << st.to_string(); @@ -1189,7 +1740,7 @@ TEST_F(ColumnReaderTest, StructPredicateMetaPathDoesNotOverrideExistingDataNeed) ColumnIterator::ReadRequirement::LAZY_OUTPUT); struct_iterator = make_struct_iterator(); - all_access_paths = {create_access_path({"s", "city", ColumnIterator::ACCESS_NULL})}; + all_access_paths = {create_meta_access_path({"s", "city", ColumnIterator::ACCESS_NULL})}; predicate_access_paths = all_access_paths; st = struct_iterator->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set predicate-only struct access paths: " << st.to_string(); @@ -1200,6 +1751,39 @@ TEST_F(ColumnReaderTest, StructPredicateMetaPathDoesNotOverrideExistingDataNeed) ColumnIterator::ReadRequirement::SKIP); } +TEST_F(ColumnReaderTest, StructSiblingDataPathDoesNotDisablePredicateMetaOnlyRead) { + auto city_iterator = std::make_unique( + create_test_reader(true, 0, FieldType::OLAP_FIELD_TYPE_STRING)); + city_iterator->set_column_name("city"); + auto* city_iterator_ptr = city_iterator.get(); + + auto data_iterator = std::make_unique(create_test_reader()); + data_iterator->set_column_name("data"); + auto* data_iterator_ptr = data_iterator.get(); + + std::vector sub_iterators; + sub_iterators.emplace_back(std::move(city_iterator)); + sub_iterators.emplace_back(std::move(data_iterator)); + StructFileColumnIterator struct_iterator( + create_test_reader(false, 0, FieldType::OLAP_FIELD_TYPE_STRUCT), nullptr, + std::move(sub_iterators)); + struct_iterator.set_column_name("s"); + + TColumnAccessPaths all_access_paths { + create_data_access_path({"s", "data"}), + create_meta_access_path({"s", "city", ColumnIterator::ACCESS_NULL})}; + TColumnAccessPaths predicate_access_paths { + create_meta_access_path({"s", "city", ColumnIterator::ACCESS_NULL})}; + + auto st = struct_iterator.set_access_paths(all_access_paths, predicate_access_paths); + ASSERT_TRUE(st.ok()) << st.to_string(); + + EXPECT_EQ(struct_iterator.read_requirement(), ColumnIterator::ReadRequirement::PREDICATE); + EXPECT_EQ(data_iterator_ptr->read_requirement(), ColumnIterator::ReadRequirement::LAZY_OUTPUT); + EXPECT_TRUE(city_iterator_ptr->read_null_map_only()); + EXPECT_EQ(city_iterator_ptr->read_requirement(), ColumnIterator::ReadRequirement::PREDICATE); +} + TEST_F(ColumnReaderTest, ArrayPredicateMetaPathDoesNotOverrideExistingDataNeed) { auto make_array_iterator = []() { auto item_iter = @@ -1216,11 +1800,11 @@ TEST_F(ColumnReaderTest, ArrayPredicateMetaPathDoesNotOverrideExistingDataNeed) }; auto array_iterator = make_array_iterator(); - TColumnAccessPaths all_access_paths { - create_access_path({"a"}), - create_access_path({"a", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_NULL})}; - TColumnAccessPaths predicate_access_paths { - create_access_path({"a", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_NULL})}; + TColumnAccessPaths all_access_paths {create_data_access_path({"a"}), + create_meta_access_path({"a", ColumnIterator::ACCESS_ALL, + ColumnIterator::ACCESS_NULL})}; + TColumnAccessPaths predicate_access_paths {create_meta_access_path( + {"a", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_NULL})}; auto st = array_iterator->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set array access paths: " << st.to_string(); @@ -1229,8 +1813,8 @@ TEST_F(ColumnReaderTest, ArrayPredicateMetaPathDoesNotOverrideExistingDataNeed) EXPECT_EQ(item_iter->read_requirement(), ColumnIterator::ReadRequirement::PREDICATE); array_iterator = make_array_iterator(); - all_access_paths = { - create_access_path({"a", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_NULL})}; + all_access_paths = {create_meta_access_path( + {"a", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_NULL})}; predicate_access_paths = all_access_paths; st = array_iterator->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set predicate-only array access paths: " << st.to_string(); @@ -1256,10 +1840,11 @@ TEST_F(ColumnReaderTest, MapPredicateMetaPathDoesNotOverrideExistingDataNeed) { }; auto map_iterator = make_map_iterator(); - TColumnAccessPaths all_access_paths {create_access_path({"m"}), - create_access_path({"m", ColumnIterator::ACCESS_MAP_VALUES, - ColumnIterator::ACCESS_NULL})}; - TColumnAccessPaths predicate_access_paths {create_access_path( + TColumnAccessPaths all_access_paths { + create_data_access_path({"m"}), + create_meta_access_path( + {"m", ColumnIterator::ACCESS_MAP_VALUES, ColumnIterator::ACCESS_NULL})}; + TColumnAccessPaths predicate_access_paths {create_meta_access_path( {"m", ColumnIterator::ACCESS_MAP_VALUES, ColumnIterator::ACCESS_NULL})}; auto st = map_iterator->set_access_paths(all_access_paths, predicate_access_paths); @@ -1271,7 +1856,7 @@ TEST_F(ColumnReaderTest, MapPredicateMetaPathDoesNotOverrideExistingDataNeed) { EXPECT_EQ(value_iter->read_requirement(), ColumnIterator::ReadRequirement::PREDICATE); map_iterator = make_map_iterator(); - all_access_paths = {create_access_path( + all_access_paths = {create_meta_access_path( {"m", ColumnIterator::ACCESS_MAP_VALUES, ColumnIterator::ACCESS_NULL})}; predicate_access_paths = all_access_paths; st = map_iterator->set_access_paths(all_access_paths, predicate_access_paths); @@ -1311,13 +1896,13 @@ TEST_F(ColumnReaderTest, MapFullProjectionStillRoutesPredicateSubPaths) { TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"m"}; + all_access_paths[0] = create_data_access_path({"m"}); TColumnAccessPaths predicate_access_paths; predicate_access_paths.emplace_back(); - predicate_access_paths[0].data_access_path.path = {"m", "KEYS"}; + predicate_access_paths[0] = create_data_access_path({"m", "KEYS"}); predicate_access_paths.emplace_back(); - predicate_access_paths[1].data_access_path.path = {"m", "VALUES", "a"}; + predicate_access_paths[1] = create_data_access_path({"m", "VALUES", "a"}); auto st = map_iterator->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set map access paths: " << st.to_string(); @@ -1336,6 +1921,33 @@ TEST_F(ColumnReaderTest, MapFullProjectionStillRoutesPredicateSubPaths) { } TEST_F(ColumnReaderTest, MetaOnlyAllPathsStillRoutePredicateSubPaths) { + { + auto struct_null_iter = + std::make_unique(std::make_shared()); + std::vector sub_iters; + auto selected_iter = std::make_unique(std::make_shared()); + selected_iter->set_column_name("selected"); + auto skipped_iter = std::make_unique(std::make_shared()); + skipped_iter->set_column_name("skipped"); + sub_iters.emplace_back(std::move(selected_iter)); + sub_iters.emplace_back(std::move(skipped_iter)); + StructFileColumnIterator struct_iterator(std::make_shared(), + std::move(struct_null_iter), std::move(sub_iters)); + struct_iterator.set_column_name("s"); + + TColumnAccessPaths all_access_paths { + create_meta_access_path({"s", ColumnIterator::ACCESS_NULL})}; + TColumnAccessPaths predicate_access_paths {create_data_access_path({"s", "selected"})}; + + auto st = struct_iterator.set_access_paths(all_access_paths, predicate_access_paths); + ASSERT_TRUE(st.ok()) << "failed to set struct access paths: " << st.to_string(); + EXPECT_FALSE(struct_iterator.read_null_map_only()); + EXPECT_EQ(struct_iterator._sub_column_iterators[0]->_read_requirement, + ColumnIterator::ReadRequirement::PREDICATE); + EXPECT_EQ(struct_iterator._sub_column_iterators[1]->_read_requirement, + ColumnIterator::ReadRequirement::SKIP); + } + { auto array_item_iterator = std::make_unique(std::make_shared()); @@ -1349,10 +1961,10 @@ TEST_F(ColumnReaderTest, MetaOnlyAllPathsStillRoutePredicateSubPaths) { array_iterator.set_column_name("a"); TColumnAccessPaths all_access_paths { - create_access_path({"a", ColumnIterator::ACCESS_OFFSET}), - create_access_path({"a", ColumnIterator::ACCESS_NULL})}; + create_meta_access_path({"a", ColumnIterator::ACCESS_OFFSET}), + create_meta_access_path({"a", ColumnIterator::ACCESS_NULL})}; TColumnAccessPaths predicate_access_paths { - create_access_path({"a", ColumnIterator::ACCESS_ALL})}; + create_data_access_path({"a", ColumnIterator::ACCESS_ALL})}; auto st = array_iterator.set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set array access paths: " << st.to_string(); @@ -1373,10 +1985,10 @@ TEST_F(ColumnReaderTest, MetaOnlyAllPathsStillRoutePredicateSubPaths) { map_iterator.set_column_name("m"); TColumnAccessPaths all_access_paths { - create_access_path({"m", ColumnIterator::ACCESS_OFFSET}), - create_access_path({"m", ColumnIterator::ACCESS_NULL})}; + create_meta_access_path({"m", ColumnIterator::ACCESS_OFFSET}), + create_meta_access_path({"m", ColumnIterator::ACCESS_NULL})}; TColumnAccessPaths predicate_access_paths { - create_access_path({"m", ColumnIterator::ACCESS_MAP_KEYS})}; + create_data_access_path({"m", ColumnIterator::ACCESS_MAP_KEYS})}; auto st = map_iterator.set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set map access paths: " << st.to_string(); @@ -1437,7 +2049,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPaths) { TColumnAccessPaths access_paths; access_paths.emplace_back(); - access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES", "a"}; + access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES", "a"}); TColumnAccessPaths predicate_access_paths = access_paths; auto st = top_struct->set_access_paths(access_paths, predicate_access_paths); @@ -1519,7 +2131,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col1"}; + all_access_paths[0] = create_data_access_path({"root", "col1"}); TColumnAccessPaths predicate_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1536,10 +2148,10 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*", "KEYS"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*", "KEYS"}); TColumnAccessPaths predicate_access_paths; predicate_access_paths.emplace_back(); - predicate_access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES", "b"}; + predicate_access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES", "b"}); auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); EXPECT_TRUE(st.ok()) << "failed to set access paths: " << st.to_string(); @@ -1549,7 +2161,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2"}; + all_access_paths[0] = create_data_access_path({"root", "col2"}); TColumnAccessPaths predicate_access_paths = all_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1579,7 +2191,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { TColumnAccessPaths all_access_paths; TColumnAccessPaths predicate_access_paths; predicate_access_paths.emplace_back(); - predicate_access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES", "a"}; + predicate_access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES", "a"}); auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); EXPECT_TRUE(st.ok()) << "failed to set access paths: " << st.to_string(); @@ -1588,7 +2200,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*", "KEYS"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*", "KEYS"}); TColumnAccessPaths predicate_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1607,7 +2219,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES"}); TColumnAccessPaths predicate_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1632,7 +2244,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*"}); TColumnAccessPaths predicate_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1657,7 +2269,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES", "a"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES", "a"}); TColumnAccessPaths predicate_access_paths = all_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1686,10 +2298,10 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*"}); TColumnAccessPaths predicate_access_paths; predicate_access_paths.emplace_back(); - predicate_access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES"}; + predicate_access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES"}); auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set access paths: " << st.to_string(); @@ -1707,7 +2319,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {}; + all_access_paths[0] = create_data_access_path({}); TColumnAccessPaths predicate_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1718,7 +2330,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"wrong_root", "col2"}; + all_access_paths[0] = create_data_access_path({"wrong_root", "col2"}); TColumnAccessPaths predicate_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1729,7 +2341,7 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "wrong_item"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "wrong_item"}); TColumnAccessPaths predicate_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -1800,10 +2412,10 @@ TEST_F(ColumnReaderTest, DeepNestedAccessPathsFiveLevels) { TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "m", "VALUES", "arr", "*"}; + all_access_paths[0] = create_data_access_path({"root", "m", "VALUES", "arr", "*"}); TColumnAccessPaths predicate_access_paths; predicate_access_paths.emplace_back(); - predicate_access_paths[0].data_access_path.path = {"root", "m", "VALUES", "arr", "*", "q"}; + predicate_access_paths[0] = create_data_access_path({"root", "m", "VALUES", "arr", "*", "q"}); auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set deep access paths: " << st.to_string(); @@ -2009,7 +2621,7 @@ TEST_F(ColumnReaderTest, NestedReadPhaseLazyOutputMatrix) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES", "a"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES", "a"}); TColumnAccessPaths predicate_access_paths = all_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -2021,7 +2633,7 @@ TEST_F(ColumnReaderTest, NestedReadPhaseLazyOutputMatrix) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*", "KEYS"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*", "KEYS"}); TColumnAccessPaths predicate_access_paths; auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); @@ -2033,10 +2645,10 @@ TEST_F(ColumnReaderTest, NestedReadPhaseLazyOutputMatrix) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*"}); TColumnAccessPaths predicate_access_paths; predicate_access_paths.emplace_back(); - predicate_access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES"}; + predicate_access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES"}); auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "failed to set access paths: " << st.to_string(); @@ -2047,10 +2659,10 @@ TEST_F(ColumnReaderTest, NestedReadPhaseLazyOutputMatrix) { auto top_struct = build_nested_iterator(); TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"root", "col2", "*", "VALUES"}; + all_access_paths[0] = create_data_access_path({"root", "col2", "*", "VALUES"}); TColumnAccessPaths predicate_access_paths; predicate_access_paths.emplace_back(); - predicate_access_paths[0].data_access_path.path = {"root", "col2", "*", "KEYS"}; + predicate_access_paths[0] = create_data_access_path({"root", "col2", "*", "KEYS"}); auto st = top_struct->set_access_paths(all_access_paths, predicate_access_paths); EXPECT_TRUE(st.ok()); @@ -2134,7 +2746,7 @@ TEST_F(ColumnReaderTest, MultiAccessPaths) { // all access paths: // self.sub_col_2.*.KEYS // predicates paths empty - all_access_paths[0].data_access_path.path = {"self", "sub_col_2", "*", "KEYS"}; + all_access_paths[0] = create_data_access_path({"self", "sub_col_2", "*", "KEYS"}); TColumnAccessPaths predicate_access_paths; @@ -2210,7 +2822,7 @@ TEST_F(ColumnReaderTest, StructNullMapOnlyNextBatchSkipsSubColumns) { std::move(sub_column_iterators)); struct_iterator.set_column_name("s"); - TColumnAccessPaths null_path {create_access_path({"s", ColumnIterator::ACCESS_NULL})}; + TColumnAccessPaths null_path {create_meta_access_path({"s", ColumnIterator::ACCESS_NULL})}; auto st = struct_iterator.set_access_paths(null_path, null_path); ASSERT_TRUE(st.ok()) << "set_access_paths failed: " << st.to_string(); EXPECT_TRUE(struct_iterator.read_null_map_only()); @@ -2245,7 +2857,7 @@ TEST_F(ColumnReaderTest, ArrayNullMapOnlyNextBatchAndReadByRowidsSkipItems) { std::move(item_iterator), std::move(null_iterator)); array_iterator.set_column_name("a"); - TColumnAccessPaths null_path {create_access_path({"a", ColumnIterator::ACCESS_NULL})}; + TColumnAccessPaths null_path {create_meta_access_path({"a", ColumnIterator::ACCESS_NULL})}; auto st = array_iterator.set_access_paths(null_path, null_path); ASSERT_TRUE(st.ok()) << "set_access_paths failed: " << st.to_string(); EXPECT_TRUE(array_iterator.read_null_map_only()); @@ -2289,7 +2901,7 @@ TEST_F(ColumnReaderTest, MapNullMapOnlyNextBatchAndReadByRowidsSkipKeysAndValues std::move(value_iterator)); map_iterator.set_column_name("m"); - TColumnAccessPaths null_path {create_access_path({"m", ColumnIterator::ACCESS_NULL})}; + TColumnAccessPaths null_path {create_meta_access_path({"m", ColumnIterator::ACCESS_NULL})}; auto st = map_iterator.set_access_paths(null_path, null_path); ASSERT_TRUE(st.ok()) << "set_access_paths failed: " << st.to_string(); EXPECT_TRUE(map_iterator.read_null_map_only()); @@ -2445,7 +3057,7 @@ TEST_F(ColumnReaderTest, MapPredicateAccessAllWithOffsetKeepsKeysReadable) { std::move(key_iter), std::move(val_iter)); map_iter.set_column_name("map_col"); - TColumnAccessPaths access_paths {create_access_path( + TColumnAccessPaths access_paths {create_meta_access_path( {"map_col", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET})}; auto st = map_iter.set_access_paths(access_paths, access_paths); ASSERT_TRUE(st.ok()) << "set_access_paths failed: " << st.to_string(); @@ -2556,7 +3168,7 @@ TEST_F(ColumnReaderTest, MapLazyReadByRowidsFillsSkippedKeysForValuesOnlyPath) { map_iter.set_column_name("map_col"); TColumnAccessPaths all_access_paths { - create_access_path({"map_col", ColumnIterator::ACCESS_MAP_VALUES})}; + create_data_access_path({"map_col", ColumnIterator::ACCESS_MAP_VALUES})}; auto st = map_iter.set_access_paths(all_access_paths, {}); ASSERT_TRUE(st.ok()) << "set_access_paths failed: " << st.to_string(); ASSERT_EQ(key_ptr->read_requirement(), ColumnIterator::ReadRequirement::SKIP); @@ -2597,9 +3209,9 @@ TEST_F(ColumnReaderTest, MapLazyReadByRowidsPreservesPredicateKeysForValuesOnlyP map_iter.set_column_name("map_col"); TColumnAccessPaths all_access_paths { - create_access_path({"map_col", ColumnIterator::ACCESS_MAP_VALUES})}; + create_data_access_path({"map_col", ColumnIterator::ACCESS_MAP_VALUES})}; TColumnAccessPaths predicate_access_paths { - create_access_path({"map_col", ColumnIterator::ACCESS_MAP_KEYS})}; + create_data_access_path({"map_col", ColumnIterator::ACCESS_MAP_KEYS})}; auto st = map_iter.set_access_paths(all_access_paths, predicate_access_paths); ASSERT_TRUE(st.ok()) << "set_access_paths failed: " << st.to_string(); ASSERT_EQ(key_ptr->read_requirement(), ColumnIterator::ReadRequirement::PREDICATE); @@ -2650,7 +3262,7 @@ TEST_F(ColumnReaderTest, MapAccessAllWithOffsetDoesNotPropagateOffsetToKey) { // path: [map_col, *, OFFSET] — simulates length(map_col['c_phone']) TColumnAccessPaths all_access_paths; all_access_paths.emplace_back(); - all_access_paths[0].data_access_path.path = {"map_col", "*", "OFFSET"}; + all_access_paths[0] = create_meta_access_path({"map_col", "*", "OFFSET"}); TColumnAccessPaths predicate_access_paths; auto st = map_iter.set_access_paths(all_access_paths, predicate_access_paths); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorToThriftConverter.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorToThriftConverter.java index 042baf31d6c7c7..33c9a6e378ae96 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorToThriftConverter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/DescriptorToThriftConverter.java @@ -19,6 +19,7 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.TableIf; +import org.apache.doris.thrift.DescriptorsConstants; import org.apache.doris.thrift.TAccessPathType; import org.apache.doris.thrift.TColumnAccessPath; import org.apache.doris.thrift.TDataAccessPath; @@ -88,6 +89,7 @@ public static TSlotDescriptor toThrift(SlotDescriptor slotDesc) { public static TColumnAccessPath toThrift(ColumnAccessPath accessPath) { TColumnAccessPath result = new TColumnAccessPath( accessPath.getType() == ColumnAccessPathType.DATA ? TAccessPathType.DATA : TAccessPathType.META); + result.setVersion(DescriptorsConstants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); if (accessPath.getType() == ColumnAccessPathType.DATA) { result.setDataAccessPath(new TDataAccessPath(accessPath.getPath())); } else { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java index 0633637b3377aa..8abe242c3e1d69 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java @@ -124,17 +124,17 @@ public Void visitSlotReference(SlotReference slotReference, CollectorContext con if (slotReference.hasSubColPath()) { path.addAll(slotReference.getSubPath()); } - // Strip NULL suffix for variant sub-column access — null-flag-only optimization - // does not apply to variant sub-column data layout. List builderPath = context.accessPathBuilder.getPathList(); - if (builderPath.size() > 1 - && AccessPathInfo.ACCESS_NULL.equals(builderPath.get(builderPath.size() - 1))) { + ColumnAccessPathType pathType = context.type; + if (pathType == ColumnAccessPathType.META) { + // Variant readers do not support metadata-only access paths. Drop the synthetic + // NULL/OFFSET component and read the referenced variant path as ordinary data. builderPath = new ArrayList<>(builderPath.subList(0, builderPath.size() - 1)); + pathType = ColumnAccessPathType.DATA; } path.addAll(builderPath); int slotId = slotReference.getExprId().asInt(); - slotToAccessPaths.put(slotId, new CollectAccessPathResult( - path, context.bottomFilter, ColumnAccessPathType.DATA)); + slotToAccessPaths.put(slotId, new CollectAccessPathResult(path, context.bottomFilter, pathType)); return null; } if (dataType instanceof NestedColumnPrunable) { @@ -151,7 +151,7 @@ public Void visitSlotReference(SlotReference slotReference, CollectorContext con context.accessPathBuilder.addPrefix(slotReference.getName()); ImmutableList path = ImmutableList.copyOf(context.accessPathBuilder.accessPath); slotToAccessPaths.put(slotId, - new CollectAccessPathResult(path, context.bottomFilter, ColumnAccessPathType.DATA)); + new CollectAccessPathResult(path, context.bottomFilter, context.type)); } else { // Direct access to the string column → record a DATA path so that any // concurrent offset-only path for the same slot is suppressed. @@ -170,7 +170,7 @@ public Void visitSlotReference(SlotReference slotReference, CollectorContext con ImmutableList path = ImmutableList.copyOf(context.accessPathBuilder.accessPath); int slotId = slotReference.getExprId().asInt(); slotToAccessPaths.put(slotId, - new CollectAccessPathResult(path, context.bottomFilter, ColumnAccessPathType.DATA)); + new CollectAccessPathResult(path, context.bottomFilter, context.type)); } // For any other nullable column type accessed directly (not via IS NULL / length / etc.): // record a [col_name] full-access path so that when the column is also used via IS NULL, @@ -214,6 +214,7 @@ public Void visitLength(Length length, CollectorContext context) { CollectorContext offsetContext = new CollectorContext(context.statementContext, context.bottomFilter); offsetContext.accessPathBuilder.addSuffix(AccessPathInfo.ACCESS_OFFSET); + offsetContext.setType(ColumnAccessPathType.META); return arg.accept(this, offsetContext); } // fall through to default (recurse into children with fresh contexts) @@ -232,6 +233,7 @@ public Void visitMapSize(MapSize mapSize, CollectorContext context) { CollectorContext offsetContext = new CollectorContext(context.statementContext, context.bottomFilter); offsetContext.accessPathBuilder.addSuffix(AccessPathInfo.ACCESS_OFFSET); + offsetContext.setType(ColumnAccessPathType.META); return arg.accept(this, offsetContext); } return visit(mapSize, context); @@ -251,6 +253,7 @@ public Void visitCardinality(Cardinality cardinality, CollectorContext context) CollectorContext offsetContext = new CollectorContext(context.statementContext, context.bottomFilter); offsetContext.accessPathBuilder.addSuffix(AccessPathInfo.ACCESS_OFFSET); + offsetContext.setType(ColumnAccessPathType.META); // cardinality(map_keys(m)) == cardinality(m) == cardinality(map_values(m)): // all three count map entries, so emit the same [map_col, OFFSET] path. Expression effectiveArg = (arg instanceof MapKeys || arg instanceof MapValues) @@ -292,9 +295,10 @@ public Void visitCast(Cast cast, CollectorContext context) { cast.child().getDataType(), ColumnAccessPathType.DATA); List replacePath = new ArrayList<>(context.accessPathBuilder.getPathList()); - if (originTree.replacePathByAnotherTree(castTree, replacePath, 0)) { + if (originTree.replacePathByAnotherTree(castTree, replacePath, 0, context.type)) { CollectorContext castContext = new CollectorContext(context.statementContext, context.bottomFilter); castContext.accessPathBuilder.accessPath.addAll(replacePath); + castContext.setType(context.type); return continueCollectAccessPath(cast.child(), castContext); } } @@ -365,6 +369,7 @@ public Void visitMapKeys(MapKeys mapKeys, CollectorContext context) { = new CollectorContext(context.statementContext, context.bottomFilter); removeStarContext.accessPathBuilder.accessPath.addAll(suffixPath.subList(1, suffixPath.size())); removeStarContext.accessPathBuilder.addPrefix(AccessPathInfo.ACCESS_MAP_KEYS); + removeStarContext.setType(context.type); return continueCollectAccessPath(mapKeys.getArgument(0), removeStarContext); } context.accessPathBuilder.addPrefix(AccessPathInfo.ACCESS_MAP_KEYS); @@ -384,6 +389,7 @@ public Void visitMapValues(MapValues mapValues, CollectorContext context) { = new CollectorContext(context.statementContext, context.bottomFilter); removeStarContext.accessPathBuilder.accessPath.addAll(suffixPath.subList(1, suffixPath.size())); removeStarContext.accessPathBuilder.addPrefix(AccessPathInfo.ACCESS_MAP_VALUES); + removeStarContext.setType(context.type); return continueCollectAccessPath(mapValues.getArgument(0), removeStarContext); } context.accessPathBuilder.addPrefix(AccessPathInfo.ACCESS_MAP_VALUES); @@ -642,6 +648,7 @@ public Void visitIsNull(IsNull isNull, CollectorContext context) { CollectorContext nullContext = new CollectorContext(context.statementContext, context.bottomFilter); nullContext.accessPathBuilder.addSuffix(AccessPathInfo.ACCESS_NULL); + nullContext.setType(ColumnAccessPathType.META); return continueCollectAccessPath(arg, nullContext); } return visit(isNull, context); @@ -839,12 +846,14 @@ public boolean equals(Object o) { return false; } CollectAccessPathResult that = (CollectAccessPathResult) o; - return isPredicate == that.isPredicate && Objects.equals(path, that.path); + return isPredicate == that.isPredicate + && type == that.type + && Objects.equals(path, that.path); } @Override public int hashCode() { - return path.hashCode(); + return Objects.hash(path, isPredicate, type); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathPlanCollector.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathPlanCollector.java index 7e7674d20586c7..fc5388d71aff68 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathPlanCollector.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathPlanCollector.java @@ -354,7 +354,8 @@ public Void visitLogicalFileScan(LogicalFileScan fileScan, StatementContext cont } Collection accessPaths = allSlotToAccessPaths.get(slot.getExprId().asInt()); if (!accessPaths.isEmpty()) { - scanSlotToAccessPaths.put(slot, normalizeDataSkippingOnlyAccessPaths(accessPaths)); + scanSlotToAccessPaths.put( + slot, normalizeDataSkippingOnlyAccessPaths(accessPaths)); } } return null; @@ -368,7 +369,8 @@ public Void visitLogicalTVFRelation(LogicalTVFRelation tvfRelation, StatementCon } Collection accessPaths = allSlotToAccessPaths.get(slot.getExprId().asInt()); if (!accessPaths.isEmpty()) { - scanSlotToAccessPaths.put(slot, normalizeDataSkippingOnlyAccessPaths(accessPaths)); + scanSlotToAccessPaths.put( + slot, normalizeDataSkippingOnlyAccessPaths(accessPaths)); } } return null; @@ -401,7 +403,7 @@ static List normalizeDataSkippingOnlyAccessPaths( List normalizedAccessPaths = new ArrayList<>(); for (CollectAccessPathResult accessPath : accessPaths) { List path = accessPath.getPath(); - if (isDataSkippingOnlyAccessPath(path) && path.size() > 1) { + if (path.size() > 1 && accessPath.getType() == ColumnAccessPathType.META) { // NULL/OFFSET suffixes are OLAP segment-reader-only optimizations. External // table and TVF readers use access paths as real nested field paths, so read // the referenced column/sub-column normally instead of sending a pseudo field. @@ -415,13 +417,4 @@ static List normalizeDataSkippingOnlyAccessPaths( } return normalizedAccessPaths; } - - private static boolean isDataSkippingOnlyAccessPath(List path) { - if (path.isEmpty()) { - return false; - } - String lastComponent = path.get(path.size() - 1); - return AccessPathInfo.ACCESS_NULL.equals(lastComponent) - || AccessPathInfo.ACCESS_OFFSET.equals(lastComponent); - } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java index 6ae6ef2b1abf9d..72f85405774316 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java @@ -47,7 +47,6 @@ import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import com.google.common.collect.TreeMultimap; -import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -218,8 +217,9 @@ private static Map pruneDataType( Map slotIdToPredicateAccessTree = new LinkedHashMap<>(); Map variantSlots = new LinkedHashMap<>(); - Comparator>> pathComparator = Comparator.comparing( - l -> StringUtils.join(l.second, ".")); + Comparator>> pathComparator = Comparator + .comparing((Pair> path) -> path.first) + .thenComparing(path -> path.second, NestedColumnPruning::comparePathComponents); Multimap>> allAccessPaths = TreeMultimap.create( Comparator.naturalOrder(), pathComparator); @@ -230,16 +230,12 @@ private static Map pruneDataType( for (Entry> kv : slotToAccessPaths.entrySet()) { Slot slot = kv.getKey(); List collectAccessPathResults = kv.getValue(); - boolean hasRegularAccessPath = collectAccessPathResults.stream() - .anyMatch(resultItem -> !isDataSkippingOnlyAccessPath(resultItem.getPath())); if (slot.getDataType() instanceof VariantType) { variantSlots.put(slot, slot.getDataType()); for (CollectAccessPathResult collectAccessPathResult : collectAccessPathResults) { List path = collectAccessPathResult.getPath(); ColumnAccessPathType pathType = collectAccessPathResult.getType(); - Pair> allPath = - normalizePredicateMetaPathForAllAccessPath( - slot, collectAccessPathResult, hasRegularAccessPath); + Pair> allPath = Pair.of(pathType, path); allAccessPaths.put(slot.getExprId().asInt(), allPath); if (collectAccessPathResult.isPredicate()) { predicateAccessPaths.put( @@ -252,9 +248,7 @@ private static Map pruneDataType( for (CollectAccessPathResult collectAccessPathResult : collectAccessPathResults) { List path = collectAccessPathResult.getPath(); ColumnAccessPathType pathType = collectAccessPathResult.getType(); - Pair> allPath = - normalizePredicateMetaPathForAllAccessPath( - slot, collectAccessPathResult, hasRegularAccessPath); + Pair> allPath = Pair.of(pathType, path); DataTypeAccessTree allAccessTree = slotIdToAllAccessTree.computeIfAbsent( slot, i -> DataTypeAccessTree.ofRoot(slot, allPath.first) ); @@ -274,18 +268,15 @@ private static Map pruneDataType( } // phase 1.5: for slots with meta paths, expand map-star paths before building final - // access path lists. Predicate NULL/OFFSET paths are kept in predicateAccessPaths. - // When regular data paths also exist, allAccessPaths uses the stripped data path for - // mixed-version safety with older BEs that only understand allAccessPaths, except - // map-star predicate metadata paths. Those paths must stay unstripped until this phase - // so map.*.OFFSET can become map.KEYS + map.VALUES.OFFSET instead of broad map.*. + // access path lists. This turns map.*.OFFSET into precise map.KEYS + + // map.VALUES.OFFSET paths instead of broad map.* access. for (Entry kv : slotIdToAllAccessTree.entrySet()) { Slot slot = kv.getKey(); DataTypeAccessTree accessTree = kv.getValue(); if (!accessTree.hasOffsetPath() && !accessTree.hasNullPath()) { continue; } - // Expand both sets before stripping so covering is complete. + // Expand both sets so all-path and predicate-path routing stay consistent. expandMapStarPaths(slot, allAccessPaths); expandMapStarPaths(slot, predicateAccessPaths); } @@ -310,7 +301,7 @@ private static Map pruneDataType( new AccessPathInfo(slot.getDataType(), allPaths, new ArrayList<>())); } - // third: build predicate access path (strip already done in phase 1.5) + // third: build predicate access paths for (Entry kv : slotIdToPredicateAccessTree.entrySet()) { Slot slot = kv.getKey(); List predicatePaths = @@ -338,26 +329,15 @@ private static Map pruneDataType( return result; } - private static Pair> normalizePredicateMetaPathForAllAccessPath( - Slot slot, CollectAccessPathResult accessPathResult, boolean hasRegularAccessPath) { - List path = accessPathResult.getPath(); - ColumnAccessPathType pathType = accessPathResult.getType(); - if (accessPathResult.isPredicate() && hasRegularAccessPath - && isDataSkippingOnlyAccessPath(path)) { - if (hasMapStarAccessPath(slot, path)) { - // Keep map-star metadata until expandMapStarPaths() turns it into precise - // KEYS/VALUES paths. Stripping here would broaden map value reads to map.*. - return Pair.of(pathType, path); + private static int comparePathComponents(List left, List right) { + int commonSize = Math.min(left.size(), right.size()); + for (int i = 0; i < commonSize; i++) { + int result = left.get(i).compareTo(right.get(i)); + if (result != 0) { + return result; } - return Pair.of(ColumnAccessPathType.DATA, stripDataSkippingSuffix(path)); } - return Pair.of(pathType, path); - } - - private static boolean hasMapStarAccessPath(Slot slot, List path) { - List positions = new ArrayList<>(); - findMapStarPositions(path, slot.getDataType(), positions); - return !positions.isEmpty(); + return Integer.compare(left.size(), right.size()); } /** @@ -367,10 +347,8 @@ private static boolean hasMapStarAccessPath(Slot slot, List path) { * decide which data sub-iterators can be pruned; any predicate data path missing from * allAccessPaths can therefore make predicate reads disagree with pruning. * - * Keep predicate NULL/OFFSET paths out of allAccessPaths for mixed-version safety. Older BEs - * decide current-level meta-only mode from allAccessPaths only, so sending both metadata and - * data paths there could make them skip required child data. Newer BEs still receive those - * metadata predicates through predicateAccessPaths. + * Metadata predicate paths are already retained in allAccessPaths while the access trees are + * built, so only missing data paths need to be added here. */ private static void addPredicatePathsToFinalAllAccessPaths( List predicatePaths, List allPaths) { @@ -404,20 +382,7 @@ private static boolean isPrefixPath(List prefix, List path) { } private static boolean isMetaPath(ColumnAccessPath path) { - return isDataSkippingOnlyAccessPath(path.getPath()); - } - - private static boolean isDataSkippingOnlyAccessPath(List components) { - if (components.isEmpty()) { - return false; - } - String lastComponent = components.get(components.size() - 1); - return AccessPathInfo.ACCESS_NULL.equals(lastComponent) - || AccessPathInfo.ACCESS_OFFSET.equals(lastComponent); - } - - private static List stripDataSkippingSuffix(List components) { - return new ArrayList<>(components.subList(0, components.size() - 1)); + return path.getType() == ColumnAccessPathType.META; } private static List buildColumnAccessPaths( @@ -612,10 +577,23 @@ public DataType pruneCastType(DataTypeAccessTree origin, DataTypeAccessTree cast } /** replacePathByAnotherTree */ - public boolean replacePathByAnotherTree(DataTypeAccessTree cast, List path, int index) { + public boolean replacePathByAnotherTree( + DataTypeAccessTree cast, List path, int index, ColumnAccessPathType pathType) { if (index >= path.size()) { return true; } + if (pathType == ColumnAccessPathType.META && index == path.size() - 1) { + String metaPath = path.get(index); + if (!metaPath.equals(AccessPathInfo.ACCESS_NULL) + && !metaPath.equals(AccessPathInfo.ACCESS_OFFSET)) { + throw new AnalysisException("unsupported metadata access path: " + metaPath); + } + // A type-changing cast can change both the physical OFFSET representation and + // nullness (for example, a failed string-to-number cast). Only translate the + // metadata suffix when the source and target node types are identical; otherwise + // visitCast falls back to reading the original value data before evaluating cast. + return type.equals(cast.type); + } if (cast.type instanceof StructType) { List fields = ((StructType) cast.type).getFields(); for (int i = 0; i < fields.size(); i++) { @@ -624,17 +602,17 @@ public boolean replacePathByAnotherTree(DataTypeAccessTree cast, List pa String originFieldName = ((StructType) type).getFields().get(i).getName(); path.set(index, originFieldName); return children.get(originFieldName).replacePathByAnotherTree( - cast.children.get(castFieldName), path, index + 1 + cast.children.get(castFieldName), path, index + 1, pathType ); } } } else if (cast.type instanceof ArrayType) { return children.values().iterator().next().replacePathByAnotherTree( - cast.children.values().iterator().next(), path, index + 1); + cast.children.values().iterator().next(), path, index + 1, pathType); } else if (cast.type instanceof MapType) { String fieldName = path.get(index); return children.get(AccessPathInfo.ACCESS_MAP_VALUES).replacePathByAnotherTree( - cast.children.get(fieldName), path, index + 1 + cast.children.get(fieldName), path, index + 1, pathType ); } return false; @@ -651,10 +629,11 @@ public void setAccessByPath(List path, int accessIndex, ColumnAccessPath this.pathType = ColumnAccessPathType.DATA; } - // NULL path component: the column is accessed only via IS NULL / IS NOT NULL. + // Terminal NULL component: the column is accessed only via IS NULL / IS NOT NULL. // Mark null-check-only and return without setting accessAll or accessPartialChild, // so that parent nodes can distinguish "null-only leaf" from "has real sub-access". - if (path.get(accessIndex).equals(AccessPathInfo.ACCESS_NULL)) { + if (pathType == ColumnAccessPathType.META && accessIndex == path.size() - 1 + && path.get(accessIndex).equals(AccessPathInfo.ACCESS_NULL)) { hasNullPath = true; return; } @@ -672,8 +651,9 @@ public void setAccessByPath(List path, int accessIndex, ColumnAccessPath } return; } else if (this.type.isArrayType()) { - if (path.get(accessIndex).equals(AccessPathInfo.ACCESS_OFFSET)) { - // length(array_col) — only the offset array is needed, not element data. + if (pathType == ColumnAccessPathType.META && accessIndex == path.size() - 1 + && path.get(accessIndex).equals(AccessPathInfo.ACCESS_OFFSET)) { + // cardinality(array_col) — only the offset array is needed, not element data. hasOffsetPath = true; return; } @@ -685,8 +665,9 @@ public void setAccessByPath(List path, int accessIndex, ColumnAccessPath return; } else if (this.type.isMapType()) { String fieldName = path.get(accessIndex); - if (fieldName.equals(AccessPathInfo.ACCESS_OFFSET)) { - // length(map_col) — only the offset array is needed, not key/value data. + if (pathType == ColumnAccessPathType.META && accessIndex == path.size() - 1 + && fieldName.equals(AccessPathInfo.ACCESS_OFFSET)) { + // cardinality(map_col) — only the offset array is needed, not key/value data. hasOffsetPath = true; return; } @@ -717,7 +698,8 @@ public void setAccessByPath(List path, int accessIndex, ColumnAccessPath } else if (type.isStringLikeType()) { // String leaf accessed via the offset array (e.g. path ends in "offset"). // Mark offset-only so pruneDataType() can return BigIntType instead of full data. - if (path.get(accessIndex).equals(AccessPathInfo.ACCESS_OFFSET)) { + if (pathType == ColumnAccessPathType.META && accessIndex == path.size() - 1 + && path.get(accessIndex).equals(AccessPathInfo.ACCESS_OFFSET)) { hasOffsetPath = true; return; // do NOT set accessAll — offset-only is distinguishable from full access } @@ -910,7 +892,8 @@ private static List>> expandOnePath( } result.add(Pair.of(type, allValues)); - // KEYS-terminating path for each position + // KEYS-terminating path for each position. Map lookup still reads key data even when + // the values path only reads metadata. for (int i = 0; i < n; i++) { int keysPos = positions.get(i); List keysPath = new ArrayList<>(); @@ -922,7 +905,7 @@ private static List>> expandOnePath( keysPath.add(component); } keysPath.add(AccessPathInfo.ACCESS_MAP_KEYS); - result.add(Pair.of(type, keysPath)); + result.add(Pair.of(ColumnAccessPathType.DATA, keysPath)); } return result; diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/DescriptorToThriftConverterTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/DescriptorToThriftConverterTest.java index a0330f631ba2ee..c8df2004651b2e 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/DescriptorToThriftConverterTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/DescriptorToThriftConverterTest.java @@ -20,6 +20,7 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.TableIf; import org.apache.doris.catalog.Type; +import org.apache.doris.thrift.DescriptorsConstants; import org.apache.doris.thrift.TAccessPathType; import org.apache.doris.thrift.TColumnAccessPath; import org.apache.doris.thrift.TDescriptorTable; @@ -36,6 +37,12 @@ public class DescriptorToThriftConverterTest { + private static void assertTypedAccessPathVersion(TColumnAccessPath accessPath) { + Assertions.assertTrue(accessPath.isSetVersion()); + Assertions.assertEquals( + DescriptorsConstants.TCOLUMN_ACCESS_PATH_VERSION_TYPED, accessPath.getVersion()); + } + // ==================== SlotDescriptor tests ==================== @Test @@ -121,7 +128,7 @@ public void testSlotDescriptorWithAccessPaths() { slotDesc.setType(Type.INT); List allPaths = Arrays.asList( ColumnAccessPath.data(Arrays.asList("a", "b")), - ColumnAccessPath.meta(Arrays.asList("c"))); + ColumnAccessPath.meta(Arrays.asList("c", "NULL"))); List predPaths = Arrays.asList( ColumnAccessPath.data(Arrays.asList("x"))); slotDesc.setAllAccessPaths(allPaths); @@ -131,8 +138,10 @@ public void testSlotDescriptorWithAccessPaths() { Assertions.assertTrue(result.isSetAllAccessPaths()); Assertions.assertEquals(2, result.getAllAccessPaths().size()); + result.getAllAccessPaths().forEach(DescriptorToThriftConverterTest::assertTypedAccessPathVersion); Assertions.assertTrue(result.isSetPredicateAccessPaths()); Assertions.assertEquals(1, result.getPredicateAccessPaths().size()); + result.getPredicateAccessPaths().forEach(DescriptorToThriftConverterTest::assertTypedAccessPathVersion); } @Test @@ -158,7 +167,7 @@ public void testSlotDescriptorWithAllFields() { List allPaths = Arrays.asList( ColumnAccessPath.data(Arrays.asList("a"))); List predPaths = Arrays.asList( - ColumnAccessPath.meta(Arrays.asList("b"))); + ColumnAccessPath.meta(Arrays.asList("b", "NULL"))); slotDesc.setAllAccessPaths(allPaths); slotDesc.setPredicateAccessPaths(predPaths); @@ -289,6 +298,7 @@ public void testColumnAccessPathDataToThrift() { TColumnAccessPath result = DescriptorToThriftConverter.toThrift(accessPath); + assertTypedAccessPathVersion(result); Assertions.assertEquals(TAccessPathType.DATA, result.getType()); Assertions.assertTrue(result.isSetDataAccessPath()); Assertions.assertFalse(result.isSetMetaAccessPath()); @@ -297,30 +307,33 @@ public void testColumnAccessPathDataToThrift() { @Test public void testColumnAccessPathMetaToThrift() { - ColumnAccessPath accessPath = ColumnAccessPath.meta(Arrays.asList("col2", "field2")); + ColumnAccessPath accessPath = ColumnAccessPath.meta(Arrays.asList("col2", "field2", "NULL")); TColumnAccessPath result = DescriptorToThriftConverter.toThrift(accessPath); + assertTypedAccessPathVersion(result); Assertions.assertEquals(TAccessPathType.META, result.getType()); Assertions.assertFalse(result.isSetDataAccessPath()); Assertions.assertTrue(result.isSetMetaAccessPath()); - Assertions.assertEquals(Arrays.asList("col2", "field2"), result.getMetaAccessPath().getPath()); + Assertions.assertEquals(Arrays.asList("col2", "field2", "NULL"), result.getMetaAccessPath().getPath()); } @Test public void testColumnAccessPathListToThrift() { List paths = Arrays.asList( ColumnAccessPath.data(Arrays.asList("a", "b")), - ColumnAccessPath.meta(Arrays.asList("c")), + ColumnAccessPath.meta(Arrays.asList("c", "NULL")), ColumnAccessPath.data(Arrays.asList("d", "e", "f"))); List results = DescriptorToThriftConverter.toThrift(paths); Assertions.assertEquals(3, results.size()); + results.forEach(DescriptorToThriftConverterTest::assertTypedAccessPathVersion); Assertions.assertEquals(TAccessPathType.DATA, results.get(0).getType()); Assertions.assertEquals(Arrays.asList("a", "b"), results.get(0).getDataAccessPath().getPath()); Assertions.assertEquals(TAccessPathType.META, results.get(1).getType()); - Assertions.assertEquals(Arrays.asList("c"), results.get(1).getMetaAccessPath().getPath()); + Assertions.assertFalse(results.get(1).isSetDataAccessPath()); + Assertions.assertEquals(Arrays.asList("c", "NULL"), results.get(1).getMetaAccessPath().getPath()); Assertions.assertEquals(TAccessPathType.DATA, results.get(2).getType()); Assertions.assertEquals(Arrays.asList("d", "e", "f"), results.get(2).getDataAccessPath().getPath()); } @@ -331,6 +344,7 @@ public void testColumnAccessPathEmptyPathToThrift() { TColumnAccessPath result = DescriptorToThriftConverter.toThrift(accessPath); + assertTypedAccessPathVersion(result); Assertions.assertEquals(TAccessPathType.DATA, result.getType()); Assertions.assertTrue(result.isSetDataAccessPath()); Assertions.assertEquals(Arrays.asList(), result.getDataAccessPath().getPath()); @@ -342,17 +356,20 @@ public void testSlotDescriptorAccessPathsRoundTrip() { slotDesc.setType(Type.INT); List allPaths = Arrays.asList( ColumnAccessPath.data(Arrays.asList("x", "y")), - ColumnAccessPath.meta(Arrays.asList("z"))); + ColumnAccessPath.meta(Arrays.asList("z", "NULL"))); slotDesc.setAllAccessPaths(allPaths); TSlotDescriptor result = DescriptorToThriftConverter.toThrift(slotDesc); Assertions.assertEquals(2, result.getAllAccessPaths().size()); TColumnAccessPath first = result.getAllAccessPaths().get(0); + assertTypedAccessPathVersion(first); Assertions.assertEquals(TAccessPathType.DATA, first.getType()); Assertions.assertEquals(Arrays.asList("x", "y"), first.getDataAccessPath().getPath()); TColumnAccessPath second = result.getAllAccessPaths().get(1); + assertTypedAccessPathVersion(second); Assertions.assertEquals(TAccessPathType.META, second.getType()); - Assertions.assertEquals(Arrays.asList("z"), second.getMetaAccessPath().getPath()); + Assertions.assertFalse(second.isSetDataAccessPath()); + Assertions.assertEquals(Arrays.asList("z", "NULL"), second.getMetaAccessPath().getPath()); } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java index efbe72a75acecd..ca53366e5cc0bb 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java @@ -45,6 +45,9 @@ import org.apache.doris.nereids.types.DataType; import org.apache.doris.nereids.types.NestedColumnPrunable; import org.apache.doris.nereids.types.NullType; +import org.apache.doris.nereids.types.StringType; +import org.apache.doris.nereids.types.StructField; +import org.apache.doris.nereids.types.StructType; import org.apache.doris.nereids.util.MemoPatternMatchSupported; import org.apache.doris.nereids.util.PlanChecker; import org.apache.doris.planner.OlapScanNode; @@ -134,6 +137,11 @@ public void createTable() throws Exception { + " >\n" + ") properties ('replication_num'='1')"); + createTable("create table meta_name_tbl(\n" + + " id int,\n" + + " s struct<`NULL`: string, `OFFSET`: string>\n" + + ") properties ('replication_num'='1')"); + connectContext.getSessionVariable().setDisableNereidsRules(RuleType.PRUNE_EMPTY_PARTITION.name()); connectContext.getSessionVariable().enableNereidsTimeout = false; } @@ -166,7 +174,10 @@ public void testMap() throws Exception { public void testMapElementLengthWithMapValuesKeepsKeysPath() throws Exception { assertColumn("select length(map_col['a']), map_values(map_col)[1] from str_tbl", "map", - ImmutableList.of(path("map_col", "KEYS"), path("map_col", "VALUES"), path("map_col", "VALUES", "OFFSET")), + ImmutableList.of( + path("map_col", "KEYS"), + path("map_col", "VALUES"), + metaPath("map_col", "VALUES", "OFFSET")), ImmutableList.of() ); } @@ -175,7 +186,7 @@ public void testMapElementLengthWithMapValuesKeepsKeysPath() throws Exception { public void testCardinalityArrayElementKeepsOffsetPath() throws Exception { assertAllAccessPathsContain( "select cardinality(element_at(a, 1)) from nested_array_tbl", - ImmutableList.of(path("a", "*", "OFFSET")), + ImmutableList.of(metaPath("a", "*", "OFFSET")), ImmutableList.of(path("a", "*"))); } @@ -183,21 +194,21 @@ public void testCardinalityArrayElementKeepsOffsetPath() throws Exception { public void testCardinalityMapElementKeepsValueOffsetPath() throws Exception { assertColumn("select cardinality(map_arr_col['a']) from map_array_tbl", "map>", - ImmutableList.of(path("map_arr_col", "KEYS"), path("map_arr_col", "VALUES", "OFFSET")), + ImmutableList.of(path("map_arr_col", "KEYS"), metaPath("map_arr_col", "VALUES", "OFFSET")), ImmutableList.of()); } @Test - public void testFullFieldAccessStripsExactDataSkippingPath() throws Exception { + public void testFullFieldAccessKeepsExactMetadataPath() throws Exception { assertColumn("select element_at(s, 'city') from tbl " + "where element_at(s, 'city') is null", "struct", - ImmutableList.of(path("s", "city")), - ImmutableList.of(path("s", "city", "NULL"))); + ImmutableList.of(path("s", "city"), metaPath("s", "city", "NULL")), + ImmutableList.of(metaPath("s", "city", "NULL"))); assertColumn("select cardinality(element_at(s, 'data')), element_at(s, 'data') from tbl", "struct>>>", - ImmutableList.of(path("s", "data"), path("s", "data", "OFFSET")), + ImmutableList.of(path("s", "data"), metaPath("s", "data", "OFFSET")), ImmutableList.of()); assertColumn("select cardinality(a), a from nested_array_tbl", @@ -207,7 +218,10 @@ public void testFullFieldAccessStripsExactDataSkippingPath() throws Exception { assertColumn("select cardinality(map_arr_col['a']), map_arr_col['a'] from map_array_tbl", "map>", - ImmutableList.of(path("map_arr_col", "KEYS"), path("map_arr_col", "VALUES"), path("map_arr_col", "VALUES", "OFFSET")), + ImmutableList.of( + path("map_arr_col", "KEYS"), + path("map_arr_col", "VALUES"), + metaPath("map_arr_col", "VALUES", "OFFSET")), ImmutableList.of()); } @@ -223,17 +237,17 @@ public void testCardinalityMapElementOffsetPredicateStaysOutOfAllAccessPaths() t allAccessPaths.addAll(slotDescriptor.getAllAccessPaths()); predicateAccessPaths.addAll(slotDescriptor.getPredicateAccessPaths()); } - Assertions.assertFalse(allAccessPaths.contains(path("s", "m", "*", "OFFSET")), + Assertions.assertFalse(allAccessPaths.contains(metaPath("s", "m", "*", "OFFSET")), "allAccessPaths=" + allAccessPaths); Assertions.assertTrue(allAccessPaths.contains(path("s", "m", "KEYS")), "allAccessPaths=" + allAccessPaths); Assertions.assertTrue(allAccessPaths.contains(path("s", "m", "VALUES", "*", "verified")), "allAccessPaths=" + allAccessPaths); - Assertions.assertTrue(allAccessPaths.contains(path("s", "m", "VALUES", "OFFSET")), + Assertions.assertTrue(allAccessPaths.contains(metaPath("s", "m", "VALUES", "OFFSET")), "allAccessPaths=" + allAccessPaths); Assertions.assertTrue(predicateAccessPaths.contains(path("s", "m", "KEYS")), "predicateAccessPaths=" + predicateAccessPaths); - Assertions.assertTrue(predicateAccessPaths.contains(path("s", "m", "VALUES", "OFFSET")), + Assertions.assertTrue(predicateAccessPaths.contains(metaPath("s", "m", "VALUES", "OFFSET")), "predicateAccessPaths=" + predicateAccessPaths); } @@ -250,17 +264,17 @@ public void testMapElementArrayNullPredicateStaysOutOfAllAccessPaths() throws Ex allAccessPaths.addAll(slotDescriptor.getAllAccessPaths()); predicateAccessPaths.addAll(slotDescriptor.getPredicateAccessPaths()); } - Assertions.assertFalse(allAccessPaths.contains(path("s", "m", "*", "NULL")), + Assertions.assertFalse(allAccessPaths.contains(metaPath("s", "m", "*", "NULL")), "allAccessPaths=" + allAccessPaths); Assertions.assertTrue(allAccessPaths.contains(path("s", "m", "KEYS")), "allAccessPaths=" + allAccessPaths); Assertions.assertTrue(allAccessPaths.contains(path("s", "m", "VALUES", "*", "verified")), "allAccessPaths=" + allAccessPaths); - Assertions.assertTrue(allAccessPaths.contains(path("s", "m", "VALUES", "NULL")), + Assertions.assertTrue(allAccessPaths.contains(metaPath("s", "m", "VALUES", "NULL")), "allAccessPaths=" + allAccessPaths); Assertions.assertTrue(predicateAccessPaths.contains(path("s", "m", "KEYS")), "predicateAccessPaths=" + predicateAccessPaths); - Assertions.assertTrue(predicateAccessPaths.contains(path("s", "m", "VALUES", "NULL")), + Assertions.assertTrue(predicateAccessPaths.contains(metaPath("s", "m", "VALUES", "NULL")), "predicateAccessPaths=" + predicateAccessPaths); } @@ -291,6 +305,18 @@ public void testVariantPredicateAccessPath() throws Exception { ); } + @Test + public void testVariantRootNullCheckFallsBackToData() { + SlotReference slot = rewriteAndFindScanSlot( + "select 1 from variant_tbl where v is null", "v", false); + Assertions.assertEquals( + new TreeSet<>(ImmutableList.of(path("v"))), + new TreeSet<>(slot.getAllAccessPaths().get())); + Assertions.assertEquals( + new TreeSet<>(ImmutableList.of(path("v"))), + new TreeSet<>(slot.getPredicateAccessPaths().get())); + } + @Test public void testVariantProjectAndPredicateAccessPaths() throws Exception { assertVariantSubColumnSlots("select v['a'] from variant_tbl where v['b']['c'] = 1", @@ -409,6 +435,25 @@ public void testPruneCast() throws Exception { ImmutableList.of() ); + assertColumn("select length(element_at(cast(c_struct as struct), 'v')) from str_tbl", + "struct", + ImmutableList.of(metaPath("c_struct", "f3", "OFFSET")), + ImmutableList.of() + ); + + assertColumn("select length(element_at(cast(c_struct as struct), 'k')) from str_tbl", + "struct", + ImmutableList.of(path("c_struct")), + ImmutableList.of() + ); + + assertColumn("select 1 from str_tbl " + + "where element_at(cast(c_struct as struct), 'v') is null", + "struct", + ImmutableList.of(path("c_struct")), + ImmutableList.of(path("c_struct")) + ); + assertColumns("select element_at(s, 'city') from (select * from tbl union all select * from tbl2)t", ImmutableList.of( Triple.of( @@ -564,63 +609,63 @@ public void testProject() throws Exception { public void testFilter() throws Throwable { assertColumn("select 100 from tbl where s is not null", "struct>>>", - ImmutableList.of(path("s", "NULL")), - ImmutableList.of(path("s", "NULL")) + ImmutableList.of(metaPath("s", "NULL")), + ImmutableList.of(metaPath("s", "NULL")) ); - // The IF expression itself is not collected as a null-only parent access here; the - // struct_element predicate is covered by the stripped parent path in allPaths. + // The IF expression contributes a parent metadata path, while the struct_element + // predicate keeps its independent data path. assertColumn("select 100 from tbl where if(id = 1, null, s) is not null or element_at(s, 'city') = 'beijing'", - "struct>>>", - ImmutableList.of(path("s")), - ImmutableList.of(path("s", "NULL"), path("s", "city")) + "struct", + ImmutableList.of(metaPath("s", "NULL"), path("s", "city")), + ImmutableList.of(metaPath("s", "NULL"), path("s", "city")) ); assertColumn("select 100 from tbl where element_at(s, 'city') is not null", "struct", - ImmutableList.of(path("s", "city", "NULL")), - ImmutableList.of(path("s", "city", "NULL")) + ImmutableList.of(metaPath("s", "city", "NULL")), + ImmutableList.of(metaPath("s", "city", "NULL")) ); assertColumn("select 100 from tbl where element_at(s, 'data') is not null", "struct>>>", - ImmutableList.of(path("s", "data", "NULL")), - ImmutableList.of(path("s", "data", "NULL")) + ImmutableList.of(metaPath("s", "data", "NULL")), + ImmutableList.of(metaPath("s", "data", "NULL")) ); assertColumn("select 100 from tbl where element_at(s, 'data')[1] is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "NULL")), - ImmutableList.of(path("s", "data", "*", "NULL")) + ImmutableList.of(metaPath("s", "data", "*", "NULL")), + ImmutableList.of(metaPath("s", "data", "*", "NULL")) ); assertColumn("select 100 from tbl where map_keys(element_at(s, 'data')[1]) is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "NULL")), - ImmutableList.of(path("s", "data", "*", "NULL")) + ImmutableList.of(metaPath("s", "data", "*", "NULL")), + ImmutableList.of(metaPath("s", "data", "*", "NULL")) ); assertColumn("select 100 from tbl where map_values(element_at(s, 'data')[1]) is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "NULL")), - ImmutableList.of(path("s", "data", "*", "NULL")) + ImmutableList.of(metaPath("s", "data", "*", "NULL")), + ImmutableList.of(metaPath("s", "data", "*", "NULL")) ); assertColumn("select 100 from tbl where element_at(map_values(element_at(s, 'data')[1])[1], 'a') is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "VALUES", "a", "NULL")), - ImmutableList.of(path("s", "data", "*", "VALUES", "a", "NULL")) + ImmutableList.of(metaPath("s", "data", "*", "VALUES", "a", "NULL")), + ImmutableList.of(metaPath("s", "data", "*", "VALUES", "a", "NULL")) ); assertColumn("select 100 from tbl where element_at(s, 'data')[1][1] is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "KEYS"), path("s", "data", "*", "VALUES", "NULL")), - ImmutableList.of(path("s", "data", "*", "KEYS"), path("s", "data", "*", "VALUES", "NULL")) + ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "NULL")), + ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "NULL")) ); assertColumn("select 100 from tbl where element_at(element_at(s, 'data')[1][1], 'a') is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "KEYS"), path("s", "data", "*", "VALUES", "a", "NULL")), - ImmutableList.of(path("s", "data", "*", "KEYS"), path("s", "data", "*", "VALUES", "a", "NULL")) + ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "a", "NULL")), + ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "a", "NULL")) ); assertColumn("select 100 from tbl where element_at(element_at(s, 'data')[1][1], 'b') is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "KEYS"), path("s", "data", "*", "VALUES", "b", "NULL")), - ImmutableList.of(path("s", "data", "*", "KEYS"), path("s", "data", "*", "VALUES", "b", "NULL")) + ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "b", "NULL")), + ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "b", "NULL")) ); } @@ -631,24 +676,24 @@ public void testMapKeysAndValuesFunctionNullCheckUseParentMapNullPath() throws E // read the parent map null map, not the KEYS/VALUES child null maps. assertColumn("select 100 from str_tbl where map_keys(map_col) is null", "map", - ImmutableList.of(path("map_col", "NULL")), - ImmutableList.of(path("map_col", "NULL")) + ImmutableList.of(metaPath("map_col", "NULL")), + ImmutableList.of(metaPath("map_col", "NULL")) ); assertColumn("select 100 from str_tbl where map_values(map_col) is null", "map", - ImmutableList.of(path("map_col", "NULL")), - ImmutableList.of(path("map_col", "NULL")) + ImmutableList.of(metaPath("map_col", "NULL")), + ImmutableList.of(metaPath("map_col", "NULL")) ); assertColumn("select map_keys(map_col) from str_tbl where map_keys(map_col) is null", "map", - ImmutableList.of(path("map_col")), - ImmutableList.of(path("map_col", "NULL")) + ImmutableList.of(path("map_col", "KEYS"), metaPath("map_col", "NULL")), + ImmutableList.of(metaPath("map_col", "NULL")) ); assertColumn("select map_values(map_col) from str_tbl where map_values(map_col) is null", "map", - ImmutableList.of(path("map_col")), - ImmutableList.of(path("map_col", "NULL")) + ImmutableList.of(path("map_col", "VALUES"), metaPath("map_col", "NULL")), + ImmutableList.of(metaPath("map_col", "NULL")) ); } @@ -656,14 +701,19 @@ public void testMapKeysAndValuesFunctionNullCheckUseParentMapNullPath() throws E public void testProjectFilter() throws Throwable { assertColumn("select element_at(s, 'data') from tbl where element_at(s, 'city') is not null", "struct>>>", - ImmutableList.of(path("s", "city"), path("s", "data")), - ImmutableList.of(path("s", "city", "NULL")) + ImmutableList.of( + path("s", "data"), + metaPath("s", "city", "NULL")), + ImmutableList.of(metaPath("s", "city", "NULL")) ); assertColumn("select element_at(s, 'data') from tbl where element_at(s, 'city') is not null and element_at(s, 'data') is not null", "struct>>>", - ImmutableList.of(path("s", "city"), path("s", "data")), - ImmutableList.of(path("s", "city", "NULL"), path("s", "data", "NULL")) + ImmutableList.of( + path("s", "data"), + metaPath("s", "city", "NULL"), + metaPath("s", "data", "NULL")), + ImmutableList.of(metaPath("s", "city", "NULL"), metaPath("s", "data", "NULL")) ); } @@ -1041,6 +1091,51 @@ public void testDataTypeAccessTree() { ); } + @Test + public void testDataPathNamedLikeMetadataComponent() { + StructType structType = new StructType(ImmutableList.of( + new StructField("NULL", StringType.INSTANCE, true, ""), + new StructField("OFFSET", StringType.INSTANCE, true, ""))); + SlotReference slot = new SlotReference("s", structType); + + DataTypeAccessTree nullFieldTree = DataTypeAccessTree.ofRoot(slot, ColumnAccessPathType.DATA); + nullFieldTree.setAccessByPath(ImmutableList.of("s", "NULL"), 0, ColumnAccessPathType.DATA); + Assertions.assertEquals("STRUCT", nullFieldTree.pruneDataType().get().toSql()); + + DataTypeAccessTree offsetFieldTree = DataTypeAccessTree.ofRoot(slot, ColumnAccessPathType.DATA); + offsetFieldTree.setAccessByPath(ImmutableList.of("s", "OFFSET"), 0, ColumnAccessPathType.DATA); + Assertions.assertEquals("STRUCT", offsetFieldTree.pruneDataType().get().toSql()); + + DataTypeAccessTree nullMetadataTree = DataTypeAccessTree.ofRoot(slot, ColumnAccessPathType.META); + nullMetadataTree.setAccessByPath( + ImmutableList.of("s", "NULL", "NULL"), 0, ColumnAccessPathType.META); + Assertions.assertEquals("STRUCT", nullMetadataTree.pruneDataType().get().toSql()); + + DataTypeAccessTree offsetMetadataTree = DataTypeAccessTree.ofRoot(slot, ColumnAccessPathType.META); + offsetMetadataTree.setAccessByPath( + ImmutableList.of("s", "OFFSET", "OFFSET"), 0, ColumnAccessPathType.META); + Assertions.assertEquals("STRUCT", offsetMetadataTree.pruneDataType().get().toSql()); + + CollectAccessPathResult dataPath = new CollectAccessPathResult( + ImmutableList.of("s", "NULL"), false, ColumnAccessPathType.DATA); + CollectAccessPathResult metadataPath = new CollectAccessPathResult( + ImmutableList.of("s", "NULL"), false, ColumnAccessPathType.META); + Assertions.assertNotEquals(dataPath, metadataPath); + } + + @Test + public void testMetadataPathBelowSameNamedStructField() throws Exception { + assertColumn("select 1 from meta_name_tbl where element_at(s, 'NULL') is null", + "struct", + ImmutableList.of(metaPath("s", "null", "NULL")), + ImmutableList.of(metaPath("s", "null", "NULL"))); + + assertColumn("select length(element_at(s, 'OFFSET')) from meta_name_tbl", + "struct", + ImmutableList.of(metaPath("s", "offset", "OFFSET")), + ImmutableList.of()); + } + @Test public void testWithVariant() throws Exception { connectContext.getSessionVariable().enableDecimal256 = true; @@ -1345,8 +1440,8 @@ public void testStructIsNullPruning() throws Exception { // struct column IS NULL → null-only access, emit [s, NULL] path, type stays struct assertColumn("select 1 from tbl where s is null", "struct>>>", - ImmutableList.of(path("s", "NULL")), - ImmutableList.of(path("s", "NULL"))); + ImmutableList.of(metaPath("s", "NULL")), + ImmutableList.of(metaPath("s", "NULL"))); } @Test @@ -1354,34 +1449,45 @@ public void testStructIsNotNullPruning() throws Exception { // struct column IS NOT NULL → same null-only access pattern assertColumn("select 1 from tbl where s is not null", "struct>>>", - ImmutableList.of(path("s", "NULL")), - ImmutableList.of(path("s", "NULL"))); + ImmutableList.of(metaPath("s", "NULL")), + ImmutableList.of(metaPath("s", "NULL"))); + } + + @Test + public void testScalarIsNullProducesMetaPath() { + SlotReference slot = rewriteAndFindScanSlot("select 1 from tbl where id is null", "id", false); + Assertions.assertEquals( + new TreeSet<>(ImmutableList.of(metaPath("id", "NULL"))), + new TreeSet<>(slot.getAllAccessPaths().get())); + Assertions.assertEquals( + new TreeSet<>(ImmutableList.of(metaPath("id", "NULL"))), + new TreeSet<>(slot.getPredicateAccessPaths().get())); } @Test public void testStructIsNullMixedAccess() throws Exception { - // Predicate metadata paths stay in predicatePaths. allPaths keeps the stripped data path - // shape when ordinary data paths also exist, so older BEs do not switch the mixed read to - // current-level metadata-only mode. + // Predicate metadata paths stay typed in allPaths even when another child needs data. assertColumn("select element_at(s, 'city') from tbl where s is null", - "struct>>>", - ImmutableList.of(path("s")), - ImmutableList.of(path("s", "NULL"))); + "struct", + ImmutableList.of(path("s", "city"), metaPath("s", "NULL")), + ImmutableList.of(metaPath("s", "NULL"))); assertColumn("select s from tbl where element_at(s, 'city') is null", "struct>>>", ImmutableList.of(path("s")), - ImmutableList.of(path("s", "city", "NULL"))); + ImmutableList.of(metaPath("s", "city", "NULL"))); // This shape is closer to the production bug: one predicate needs the parent // null map, another predicate needs a child null map, and the projection needs - // a different child data path. allPaths strips predicate metadata to data paths and - // collapses to the whole struct for mixed-version safety. + // a different child data path. Keep all three requirements independent. assertColumn("select element_at(s, 'data') from tbl " + "where s is null or element_at(s, 'city') is null", "struct>>>", - ImmutableList.of(path("s")), - ImmutableList.of(path("s", "NULL"), path("s", "city", "NULL"))); + ImmutableList.of( + path("s", "data"), + metaPath("s", "NULL"), + metaPath("s", "city", "NULL")), + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL"))); } @Test @@ -1391,7 +1497,7 @@ public void testStringLengthPruning() throws Exception { "select length(str_col) from str_tbl", "str_col", true, - ImmutableList.of(path("str_col", "OFFSET"))); + ImmutableList.of(metaPath("str_col", "OFFSET"))); // ── Case 2: length(str_col) + direct projection of str_col ─ suppressed ───── assertStringColumn( @@ -1407,13 +1513,13 @@ public void testStringLengthPruning() throws Exception { false, ImmutableList.of()); - // ── Case 4: length applied to a struct field ─ struct pruned to bigint field ─ + // ── Case 4: length applied to a struct field ─ metadata-only field read ──── // c_struct has {f1:int, f3:string}; only f3 accessed offset-only → - // pruned type is struct, access path is DATA(["c_struct","f3","offset"]) + // pruned type is struct, access path is META(["c_struct","f3","OFFSET"]) assertColumn( "select length(element_at(c_struct, 'f3')) from str_tbl", "struct", - ImmutableList.of(path("c_struct", "f3", "OFFSET")), + ImmutableList.of(metaPath("c_struct", "f3", "OFFSET")), ImmutableList.of()); // ── Case 5: length(struct field) + direct read of same field ─ suppressed ─── @@ -1422,29 +1528,47 @@ public void testStringLengthPruning() throws Exception { assertColumn( "select length(element_at(c_struct, 'f3')), element_at(c_struct, 'f3') from str_tbl", "struct", - ImmutableList.of(path("c_struct", "f3"), path("c_struct", "f3", "OFFSET")), + ImmutableList.of(path("c_struct", "f3"), metaPath("c_struct", "f3", "OFFSET")), + ImmutableList.of()); + + assertColumn( + "select length(map_keys(map_col)[1]) from str_tbl", + "map", + ImmutableList.of(metaPath("map_col", "KEYS", "OFFSET")), + ImmutableList.of()); + + assertColumn( + "select length(map_values(map_col)[1]) from str_tbl", + "map", + ImmutableList.of(metaPath("map_col", "VALUES", "OFFSET")), ImmutableList.of()); } @Test - public void testNonOlapDataSkippingOnlyAccessPathFallback() { + public void testNonOlapMetadataAccessPathFallback() { List normalizedAccessPaths = AccessPathPlanCollector.normalizeDataSkippingOnlyAccessPaths(ImmutableList.of( + new CollectAccessPathResult( + ImmutableList.of("s", "city", "NULL"), true, ColumnAccessPathType.META), new CollectAccessPathResult( ImmutableList.of("s", "city", "NULL"), true, ColumnAccessPathType.DATA), new CollectAccessPathResult( - ImmutableList.of("array_column", "OFFSET"), false, ColumnAccessPathType.DATA), + ImmutableList.of("s", "NULL"), false, ColumnAccessPathType.DATA), + new CollectAccessPathResult( + ImmutableList.of("s", "OFFSET"), false, ColumnAccessPathType.DATA), new CollectAccessPathResult( ImmutableList.of("s", "city"), false, ColumnAccessPathType.DATA))); - Assertions.assertEquals(3, normalizedAccessPaths.size()); + Assertions.assertEquals(5, normalizedAccessPaths.size()); Assertions.assertEquals(ImmutableList.of("s", "city"), normalizedAccessPaths.get(0).getPath()); Assertions.assertTrue(normalizedAccessPaths.get(0).isPredicate()); Assertions.assertEquals(ColumnAccessPathType.DATA, normalizedAccessPaths.get(0).getType()); - Assertions.assertEquals(ImmutableList.of("array_column"), normalizedAccessPaths.get(1).getPath()); - Assertions.assertFalse(normalizedAccessPaths.get(1).isPredicate()); + Assertions.assertEquals(ImmutableList.of("s", "city", "NULL"), normalizedAccessPaths.get(1).getPath()); + Assertions.assertTrue(normalizedAccessPaths.get(1).isPredicate()); Assertions.assertEquals(ColumnAccessPathType.DATA, normalizedAccessPaths.get(1).getType()); - Assertions.assertEquals(ImmutableList.of("s", "city"), normalizedAccessPaths.get(2).getPath()); + Assertions.assertEquals(ImmutableList.of("s", "NULL"), normalizedAccessPaths.get(2).getPath()); + Assertions.assertEquals(ImmutableList.of("s", "OFFSET"), normalizedAccessPaths.get(3).getPath()); + Assertions.assertEquals(ImmutableList.of("s", "city"), normalizedAccessPaths.get(4).getPath()); } @Test @@ -1452,10 +1576,10 @@ public void testMvRewritePlanFragmentSkipsNullOnlyAccessPath() { SlotReference normalSlot = rewriteAndFindScanSlot( "select 1 from str_tbl where str_col is not null", "str_col", false); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(path("str_col", "NULL"))), + new TreeSet<>(ImmutableList.of(metaPath("str_col", "NULL"))), new TreeSet<>(normalSlot.getAllAccessPaths().get())); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(path("str_col", "NULL"))), + new TreeSet<>(ImmutableList.of(metaPath("str_col", "NULL"))), new TreeSet<>(normalSlot.getPredicateAccessPaths().get())); // MV fragment: IS NULL degrades to full column read via default visitor. @@ -1467,10 +1591,10 @@ public void testMvRewritePlanFragmentSkipsNullOnlyAccessPath() { SlotReference nestedNormalSlot = rewriteAndFindScanSlot( "select 1 from tbl where element_at(s, 'city') is not null", "s", false); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(path("s", "city", "NULL"))), + new TreeSet<>(ImmutableList.of(metaPath("s", "city", "NULL"))), new TreeSet<>(nestedNormalSlot.getAllAccessPaths().get())); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(path("s", "city", "NULL"))), + new TreeSet<>(ImmutableList.of(metaPath("s", "city", "NULL"))), new TreeSet<>(nestedNormalSlot.getPredicateAccessPaths().get())); // MV fragment: IS NULL degrades to element_at via default visitor, @@ -1490,10 +1614,10 @@ public void testMvRewritePlanFragmentSkipsOffsetOnlyAccessPath() { SlotReference normalSlot = rewriteAndFindScanSlot( "select 1 from str_tbl where length(str_col) > 0", "str_col", false); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(path("str_col", "OFFSET"))), + new TreeSet<>(ImmutableList.of(metaPath("str_col", "OFFSET"))), new TreeSet<>(normalSlot.getAllAccessPaths().get())); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(path("str_col", "OFFSET"))), + new TreeSet<>(ImmutableList.of(metaPath("str_col", "OFFSET"))), new TreeSet<>(normalSlot.getPredicateAccessPaths().get())); SlotReference fragmentSlot = rewriteAndFindScanSlot( @@ -1504,10 +1628,10 @@ public void testMvRewritePlanFragmentSkipsOffsetOnlyAccessPath() { "select 1 from str_tbl where length(element_at(c_struct, 'f3')) > 0", "c_struct", false); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(path("c_struct", "f3", "OFFSET"))), + new TreeSet<>(ImmutableList.of(metaPath("c_struct", "f3", "OFFSET"))), new TreeSet<>(nestedNormalSlot.getAllAccessPaths().get())); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(path("c_struct", "f3", "OFFSET"))), + new TreeSet<>(ImmutableList.of(metaPath("c_struct", "f3", "OFFSET"))), new TreeSet<>(nestedNormalSlot.getPredicateAccessPaths().get())); // MV fragment: length() degrades to element_at via default visitor, diff --git a/gensrc/proto/descriptors.proto b/gensrc/proto/descriptors.proto index 92fc3c0a1a35a1..81dc03ccfba36f 100644 --- a/gensrc/proto/descriptors.proto +++ b/gensrc/proto/descriptors.proto @@ -64,6 +64,9 @@ message PColumnAccessPath { required PAccessPathType type = 1; optional PDataAccessPath data_access_path = 2; optional PMetaAccessPath meta_access_path = 3; + // 0/absent is the legacy all-DATA encoding with special path components. Version 1 uses + // typed payloads and may carry meta_access_path. + optional int32 version = 4; } message PSlotDescriptor { diff --git a/gensrc/thrift/Descriptors.thrift b/gensrc/thrift/Descriptors.thrift index ca20687d9a90ed..91067d3302c2bd 100644 --- a/gensrc/thrift/Descriptors.thrift +++ b/gensrc/thrift/Descriptors.thrift @@ -64,10 +64,17 @@ struct TMetaAccessPath { 1: required list path } +const i32 TCOLUMN_ACCESS_PATH_VERSION_LEGACY = 0 +const i32 TCOLUMN_ACCESS_PATH_VERSION_TYPED = 1 + struct TColumnAccessPath { 1: required TAccessPathType type 2: optional TDataAccessPath data_access_path 3: optional TMetaAccessPath meta_access_path + // The version is absent for legacy senders. Legacy paths all have DATA type and encode + // KEYS/VALUES/* selectors plus NULL/OFFSET metadata in data_access_path. Starting from the + // typed version, type selects the authoritative payload and META may use meta_access_path. + 4: optional i32 version } struct TColumn { diff --git a/regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy b/regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy index 6e22ae943e0ebd..c18f03df12f4ac 100644 --- a/regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy +++ b/regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy @@ -18,7 +18,7 @@ // Regression tests for the IS NULL / IS NOT NULL column pruning optimization. // // When IS NULL (or IS NOT NULL) is the *only* use of a nullable column, the FE -// should emit a DATA access path with a "NULL" component so that the BE can +// should emit a META access path with a "NULL" component so that the BE can // satisfy the query by reading only the null flag instead of the full column data. // The EXPLAIN plan should show: // nested columns: : all access paths: [.NULL] @@ -183,23 +183,23 @@ suite("null_column_pruning") { order_qt_10 "select int_col from ncp_tbl where int_col is null"; // ─── Mixed: struct IS NULL + partial field access ─────────────────────────── - // struct_col IS NULL in WHERE + element_at in SELECT needs struct data for projection, while - // the predicate keeps the parent null map separately. + // struct_col IS NULL in WHERE + element_at in SELECT needs only the projected field data, + // while the predicate keeps the parent null map separately. explain { sql "select element_at(struct_col, 'city') from ncp_tbl where struct_col is null" contains "nested columns" - contains "all access paths: [struct_col]" + contains "all access paths: [struct_col.city, struct_col.NULL]" contains "predicate access paths: [struct_col.NULL]" } order_qt_11 "select element_at(struct_col, 'city') from ncp_tbl where struct_col is null"; // This query verifies the real correctness risk: predicate paths need both parent and child - // null maps, while allAccessPaths keeps the struct data path for projection. + // null maps, while allAccessPaths keeps only the projected field data path. explain { sql "select element_at(struct_col, 'zip') from ncp_tbl where struct_col is null or element_at(struct_col, 'city') is null" contains "nested columns" - contains "all access paths: [struct_col]" + contains "all access paths: [struct_col.zip, struct_col.NULL, struct_col.city.NULL]" contains "predicate access paths:" contains "struct_col.NULL" contains "struct_col.city.NULL" @@ -390,24 +390,24 @@ suite("null_column_pruning") { order_qt_24 "select count(1) from ncp_tbl where element_at(struct_col, 'city') is not null"; // ─── Mixed: map_keys IS NULL + map_keys projected ────────────────────────── - // Projection needs map data, while the predicate checks whether the parent map is NULL. The - // parent NULL path stays in predicateAccessPaths. + // Projection needs only map keys, while the predicate checks whether the parent map is NULL. + // Keep the parent NULL path independently in both path sets. explain { sql "select map_keys(map_col) from ncp_tbl where map_keys(map_col) is null" contains "nested columns" - contains "all access paths: [map_col]" + contains "all access paths: [map_col.KEYS, map_col.NULL]" contains "predicate access paths: [map_col.NULL]" } order_qt_25 "select map_keys(map_col) from ncp_tbl where map_keys(map_col) is null"; // ─── Mixed: map_values IS NULL + map_values projected ────────────────────── - // Projection needs map data, while the predicate checks whether the parent + // Projection needs only map values, while the predicate checks whether the parent // map is NULL. A NULL value element does not make map_values(map_col) NULL. explain { sql "select map_values(map_col) from ncp_tbl where map_values(map_col) is null" contains "nested columns" - contains "all access paths: [map_col]" + contains "all access paths: [map_col.VALUES, map_col.NULL]" contains "predicate access paths: [map_col.NULL]" } From 508ef86da7f2b3b82f9582dcf0e3986f557c1d7b Mon Sep 17 00:00:00 2001 From: Hu Shenggang Date: Sat, 18 Jul 2026 23:51:37 +0800 Subject: [PATCH 2/9] [refactor](be) Clarify descendant access path routing names ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: The descendant access-path router used generic route terminology for container-specific selection, rewriting, and distribution, and referred to the payload selected by the path type as normalized or active. Rename the internal helpers and result fields to describe their actual behavior, document selected-payload semantics, and keep routing behavior unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test - `./run-be-ut.sh --run --filter='ColumnReaderTest.*'` was attempted, but no test binary ran because `thirdparty/installed/lib64/liblance_c.a` is missing. - `build-support/check-format.sh` - Behavior changed: No. This only renames internal helpers, result fields, local variables, and tests, and clarifies comments. - Does this need documentation: No --- be/src/storage/segment/column_reader.cpp | 213 +++++++++--------- .../storage/segment/column_reader_test.cpp | 16 +- 2 files changed, 120 insertions(+), 109 deletions(-) diff --git a/be/src/storage/segment/column_reader.cpp b/be/src/storage/segment/column_reader.cpp index eb8e8a4d448285..82e7a41ffeea7b 100644 --- a/be/src/storage/segment/column_reader.cpp +++ b/be/src/storage/segment/column_reader.cpp @@ -103,106 +103,117 @@ bool uses_legacy_access_path_encoding(const TColumnAccessPath& path) { namespace { -// Routes only descendant paths produced by ColumnIterator::_split_access_paths(). Their versions -// have been validated, legacy paths have DATA type, and every active payload is non-empty. -// Routing rewrites only the payload selected by type. +// Routes descendant paths produced by ColumnIterator::_split_access_paths() to the immediate data +// children of a nested column. Their versions have been validated, legacy paths have DATA type, +// and every payload selected by type is non-empty. Routing never interprets or rewrites the +// unselected compatibility payload. class DescendantAccessPathRouter final { public: DescendantAccessPathRouter() = delete; - struct ChildRoutes { - TColumnAccessPaths all; - TColumnAccessPaths predicate; + // Preserve the two set_access_paths() input channels. all_paths originates from the + // all-access-path superset, while predicate_paths separately records predicate-phase paths. + // Routing may omit all_paths when the parent already requires complete child data. + struct ChildAccessPaths { + TColumnAccessPaths all_paths; + TColumnAccessPaths predicate_paths; - bool empty() const { return all.empty() && predicate.empty(); } + bool empty() const { return all_paths.empty() && predicate_paths.empty(); } }; - struct MapRoutes { - ChildRoutes key; - ChildRoutes value; + struct MapChildAccessPaths { + ChildAccessPaths key; + ChildAccessPaths value; }; - static Result route_map_descendants(TColumnAccessPaths all_paths, - TColumnAccessPaths predicate_paths, - const std::string& key_name, - const std::string& value_name) { - MapRoutes routes; - auto status = route_map_path_set(std::move(all_paths), key_name, value_name, routes.key.all, - routes.value.all); + // Map selectors are logical. Convert KEYS/VALUES to the actual key/value iterator names. For a + // wildcard, create a complete DATA path for keys and route trailing qualifiers to values. + static Result route_map_paths_to_children( + TColumnAccessPaths all_paths, TColumnAccessPaths predicate_paths, + const std::string& key_name, const std::string& value_name) { + MapChildAccessPaths child_paths; + auto status = distribute_map_paths(std::move(all_paths), key_name, value_name, + child_paths.key.all_paths, child_paths.value.all_paths); if (!status.ok()) { return ResultError(std::move(status)); } - status = route_map_path_set(std::move(predicate_paths), key_name, value_name, - routes.key.predicate, routes.value.predicate); + status = distribute_map_paths(std::move(predicate_paths), key_name, value_name, + child_paths.key.predicate_paths, + child_paths.value.predicate_paths); if (!status.ok()) { return ResultError(std::move(status)); } - return routes; + return child_paths; } - static Result route_array_descendants(TColumnAccessPaths all_paths, - TColumnAccessPaths predicate_paths, - const std::string& item_name) { - ChildRoutes routes {.all = std::move(all_paths), .predicate = std::move(predicate_paths)}; - auto rewrite_item_roots = [&](TColumnAccessPaths& paths) -> Status { + // Array has one data child. Retarget its logical wildcard selector to the item iterator name. + static Result route_array_paths_to_item(TColumnAccessPaths all_paths, + TColumnAccessPaths predicate_paths, + const std::string& item_name) { + ChildAccessPaths child_paths {.all_paths = std::move(all_paths), + .predicate_paths = std::move(predicate_paths)}; + auto retarget_wildcard_paths_to_item = [&](TColumnAccessPaths& paths) -> Status { for (auto& path : paths) { - const bool matches_access_all = DORIS_TRY( - root_matches(path, ColumnIterator::ACCESS_ALL, RootMatchMode::EXACT)); - if (matches_access_all) { - RETURN_IF_ERROR(rewrite_active_payload_root(path, item_name)); + const bool is_wildcard = DORIS_TRY(selected_payload_head_matches( + path, ColumnIterator::ACCESS_ALL, PathHeadMatchMode::EXACT)); + if (is_wildcard) { + RETURN_IF_ERROR(replace_selected_payload_head(path, item_name)); } } return Status::OK(); }; - auto status = rewrite_item_roots(routes.all); + auto status = retarget_wildcard_paths_to_item(child_paths.all_paths); if (!status.ok()) { return ResultError(std::move(status)); } - status = rewrite_item_roots(routes.predicate); + status = retarget_wildcard_paths_to_item(child_paths.predicate_paths); if (!status.ok()) { return ResultError(std::move(status)); } - return routes; + return child_paths; } - static Result route_struct_child(const TColumnAccessPaths& all_paths, - const TColumnAccessPaths& predicate_paths, - const std::string& child_name, - bool route_projection_paths) { - ChildRoutes routes; - auto route_path_set = [&](const TColumnAccessPaths& source_paths, - bool is_predicate) -> Status { - auto& child_paths = is_predicate ? routes.predicate : routes.all; + // Struct selectors already use child field names. Select the paths for one child without + // rewriting them, so that the child can validate and strip its own name. + static Result select_struct_paths_for_child( + const TColumnAccessPaths& all_paths, const TColumnAccessPaths& predicate_paths, + const std::string& child_name, bool include_all_paths) { + ChildAccessPaths child_paths; + auto select_matching_paths = [&](const TColumnAccessPaths& source_paths, + TColumnAccessPaths& child_paths) -> Status { for (const auto& path : source_paths) { - const bool routes_to_child = - DORIS_TRY(root_matches(path, child_name, RootMatchMode::CASE_INSENSITIVE)); - if (routes_to_child) { + const bool matches_child = DORIS_TRY(selected_payload_head_matches( + path, child_name, PathHeadMatchMode::CASE_INSENSITIVE)); + if (matches_child) { child_paths.emplace_back(path); } } return Status::OK(); }; - if (route_projection_paths) { - auto status = route_path_set(all_paths, false); + if (include_all_paths) { + auto status = select_matching_paths(all_paths, child_paths.all_paths); if (!status.ok()) { return ResultError(std::move(status)); } } - auto status = route_path_set(predicate_paths, true); + auto status = select_matching_paths(predicate_paths, child_paths.predicate_paths); if (!status.ok()) { return ResultError(std::move(status)); } - return routes; + return child_paths; } private: - enum class RootMatchMode { EXACT, CASE_INSENSITIVE }; - enum class MapTarget { NONE, WILDCARD, KEY, VALUE }; + enum class PathHeadMatchMode { EXACT, CASE_INSENSITIVE }; + enum class MapSelector { UNRECOGNIZED, WILDCARD, KEYS, VALUES }; + // TColumnAccessPath may carry both payload fields after compatibility forwarding. Selected + // means data_access_path for DATA and meta_access_path for META. Visit only that payload and + // leave the other payload untouched. template - static Status visit_normalized_payload(AccessPath& access_path, Visitor&& visitor) { + static Status visit_selected_payload(AccessPath& access_path, Visitor&& visitor) { switch (access_path.type) { case TAccessPathType::DATA: if (!access_path.__isset.data_access_path) { @@ -222,14 +233,15 @@ class DescendantAccessPathRouter final { } } - static Result root_matches(const TColumnAccessPath& access_path, - const std::string& expected_root, RootMatchMode match_mode) { + static Result selected_payload_head_matches(const TColumnAccessPath& access_path, + const std::string& expected_head, + PathHeadMatchMode match_mode) { bool matches = false; - auto status = visit_normalized_payload(access_path, [&](const auto& payload) { + auto status = visit_selected_payload(access_path, [&](const auto& payload) { DORIS_CHECK(!payload.path.empty()); - matches = match_mode == RootMatchMode::EXACT - ? payload.path.front() == expected_root - : StringCaseEqual()(payload.path.front(), expected_root); + matches = match_mode == PathHeadMatchMode::EXACT + ? payload.path.front() == expected_head + : StringCaseEqual()(payload.path.front(), expected_head); return Status::OK(); }); if (!status.ok()) { @@ -238,9 +250,9 @@ class DescendantAccessPathRouter final { return matches; } - static Status rewrite_active_payload_root(TColumnAccessPath& access_path, - const std::string& child_name) { - return visit_normalized_payload(access_path, [&](auto& payload) { + static Status replace_selected_payload_head(TColumnAccessPath& access_path, + const std::string& child_name) { + return visit_selected_payload(access_path, [&](auto& payload) { DORIS_CHECK(!payload.path.empty()); payload.path.front() = child_name; return Status::OK(); @@ -250,62 +262,61 @@ class DescendantAccessPathRouter final { // Map `*` applies trailing qualifiers only to values, while locating entries still requires // the complete key column as DATA. Construct a fresh key path because the source may be META, // and preserve its version so child routing keeps the same legacy/typed encoding. - static TColumnAccessPath create_version_preserving_whole_data_child_path( - const TColumnAccessPath& source_path, const std::string& child_name) { + static TColumnAccessPath make_full_data_path_for_child(const TColumnAccessPath& version_source, + const std::string& child_name) { TColumnAccessPath child_path; child_path.__set_type(TAccessPathType::DATA); TDataAccessPath data_path; data_path.__set_path({child_name}); child_path.__set_data_access_path(data_path); - if (source_path.__isset.version) { - child_path.__set_version(source_path.version); + if (version_source.__isset.version) { + child_path.__set_version(version_source.version); } return child_path; } - static Result map_target(const TColumnAccessPath& access_path) { - MapTarget target = MapTarget::NONE; - auto status = visit_normalized_payload(access_path, [&](const auto& payload) { + static Result classify_map_selector(const TColumnAccessPath& access_path) { + MapSelector selector = MapSelector::UNRECOGNIZED; + auto status = visit_selected_payload(access_path, [&](const auto& payload) { DORIS_CHECK(!payload.path.empty()); - const auto& root = payload.path.front(); - if (root == ColumnIterator::ACCESS_ALL) { - target = MapTarget::WILDCARD; - } else if (root == ColumnIterator::ACCESS_MAP_KEYS) { - target = MapTarget::KEY; - } else if (root == ColumnIterator::ACCESS_MAP_VALUES) { - target = MapTarget::VALUE; + const auto& head = payload.path.front(); + if (head == ColumnIterator::ACCESS_ALL) { + selector = MapSelector::WILDCARD; + } else if (head == ColumnIterator::ACCESS_MAP_KEYS) { + selector = MapSelector::KEYS; + } else if (head == ColumnIterator::ACCESS_MAP_VALUES) { + selector = MapSelector::VALUES; } return Status::OK(); }); if (!status.ok()) { return ResultError(std::move(status)); } - return target; + return selector; } - static Status route_map_path_set(TColumnAccessPaths source_paths, const std::string& key_name, - const std::string& value_name, TColumnAccessPaths& key_paths, - TColumnAccessPaths& value_paths) { + static Status distribute_map_paths(TColumnAccessPaths source_paths, const std::string& key_name, + const std::string& value_name, TColumnAccessPaths& key_paths, + TColumnAccessPaths& value_paths) { for (auto& path : source_paths) { - const auto target = DORIS_TRY(map_target(path)); - switch (target) { - case MapTarget::WILDCARD: + const auto selector = DORIS_TRY(classify_map_selector(path)); + switch (selector) { + case MapSelector::WILDCARD: // A wildcard needs complete keys for runtime lookup, while any remaining // qualifiers apply only to the value. The value keeps its type and version. - key_paths.emplace_back( - create_version_preserving_whole_data_child_path(path, key_name)); - RETURN_IF_ERROR(rewrite_active_payload_root(path, value_name)); + key_paths.emplace_back(make_full_data_path_for_child(path, key_name)); + RETURN_IF_ERROR(replace_selected_payload_head(path, value_name)); value_paths.emplace_back(std::move(path)); break; - case MapTarget::KEY: - RETURN_IF_ERROR(rewrite_active_payload_root(path, key_name)); + case MapSelector::KEYS: + RETURN_IF_ERROR(replace_selected_payload_head(path, key_name)); key_paths.emplace_back(std::move(path)); break; - case MapTarget::VALUE: - RETURN_IF_ERROR(rewrite_active_payload_root(path, value_name)); + case MapSelector::VALUES: + RETURN_IF_ERROR(replace_selected_payload_head(path, value_name)); value_paths.emplace_back(std::move(path)); break; - case MapTarget::NONE: + case MapSelector::UNRECOGNIZED: break; } } @@ -1751,13 +1762,13 @@ Status MapFileColumnIterator::set_access_paths(const TColumnAccessPaths& all_acc return Status::OK(); } - auto child_paths = DORIS_TRY(DescendantAccessPathRouter::route_map_descendants( + auto child_paths = DORIS_TRY(DescendantAccessPathRouter::route_map_paths_to_children( std::move(plan.all.descendant_paths), std::move(plan.predicate.descendant_paths), _key_iterator->column_name(), _val_iterator->column_name())); if (!child_paths.key.empty()) { - RETURN_IF_ERROR( - _key_iterator->set_access_paths(child_paths.key.all, child_paths.key.predicate)); + RETURN_IF_ERROR(_key_iterator->set_access_paths(child_paths.key.all_paths, + child_paths.key.predicate_paths)); // Apply LAZY_OUTPUT after child predicate paths have been handled. Read requirements are // monotonic, so a predicate-only child already promoted to PREDICATE will not // be downgraded, while a non-predicate child becomes a lazy materialization target. @@ -1768,8 +1779,8 @@ Status MapFileColumnIterator::set_access_paths(const TColumnAccessPaths& all_acc } if (!child_paths.value.empty()) { - RETURN_IF_ERROR(_val_iterator->set_access_paths(child_paths.value.all, - child_paths.value.predicate)); + RETURN_IF_ERROR(_val_iterator->set_access_paths(child_paths.value.all_paths, + child_paths.value.predicate_paths)); // Same as keys: predicate-only value paths stay PREDICATE because this // post-processing update cannot lower a stronger child requirement. _val_iterator->set_read_requirement_self(ReadRequirement::LAZY_OUTPUT); @@ -2039,15 +2050,14 @@ Status StructFileColumnIterator::set_access_paths( } const bool reads_all_sub_columns = plan.all.reads_current_data; - const bool route_projection_paths = !reads_all_sub_columns; + const bool include_all_paths = !reads_all_sub_columns; for (auto& sub_iterator : _sub_column_iterators) { const auto name = sub_iterator->column_name(); - auto paths = DORIS_TRY(DescendantAccessPathRouter::route_struct_child( + auto paths = DORIS_TRY(DescendantAccessPathRouter::select_struct_paths_for_child( plan.all.descendant_paths, plan.predicate.descendant_paths, name, - route_projection_paths)); + include_all_paths)); - // Predicate-only child paths still need to be routed to the child iterator - // even when the child is not requested by ordinary projection access paths. + // Predicate paths must still reach the child even when no non-predicate path selects it. const bool need_to_read = reads_all_sub_columns || !paths.empty(); if (!need_to_read) { set_read_requirement_self(ReadRequirement::SKIP); @@ -2056,7 +2066,7 @@ Status StructFileColumnIterator::set_access_paths( continue; } - RETURN_IF_ERROR(sub_iterator->set_access_paths(paths.all, paths.predicate)); + RETURN_IF_ERROR(sub_iterator->set_access_paths(paths.all_paths, paths.predicate_paths)); // Set LAZY_OUTPUT after routing child predicate paths. If the child was needed only for // predicate evaluation, set_access_paths() has already promoted it to // PREDICATE and this monotonic update will not downgrade it. Otherwise, this @@ -2440,12 +2450,13 @@ Status ArrayFileColumnIterator::set_access_paths(const TColumnAccessPaths& all_a const bool has_all_descendant = plan.all.has_descendant_paths(); const bool has_predicate_descendant = plan.predicate.has_descendant_paths(); - auto child_paths = DORIS_TRY(DescendantAccessPathRouter::route_array_descendants( + auto child_paths = DORIS_TRY(DescendantAccessPathRouter::route_array_paths_to_item( std::move(plan.all.descendant_paths), std::move(plan.predicate.descendant_paths), _item_iterator->column_name())); if (has_all_descendant || has_predicate_descendant) { - RETURN_IF_ERROR(_item_iterator->set_access_paths(child_paths.all, child_paths.predicate)); + RETURN_IF_ERROR(_item_iterator->set_access_paths(child_paths.all_paths, + child_paths.predicate_paths)); // Predicate-only item paths stay PREDICATE because this update runs after // child set_access_paths() and read requirements are monotonic. Non-predicate item paths are // marked as lazy materialization targets. diff --git a/be/test/storage/segment/column_reader_test.cpp b/be/test/storage/segment/column_reader_test.cpp index d46e04f629ce9b..6a8e58b50374a7 100644 --- a/be/test/storage/segment/column_reader_test.cpp +++ b/be/test/storage/segment/column_reader_test.cpp @@ -1341,7 +1341,7 @@ TEST_F(ColumnReaderTest, SplitAccessPathsClassifiesCurrentAndDescendantPaths) { EXPECT_EQ(iterator._read_requirement, ColumnIterator::ReadRequirement::NORMAL); } -TEST_F(ColumnReaderTest, MapDescendantRoutingPreservesActivePayloadAndVersion) { +TEST_F(ColumnReaderTest, MapChildPathRoutingRewritesOnlySelectedPayloadAndPreservesVersion) { auto key_iterator = std::make_unique(); key_iterator->set_column_name("key"); auto* key = key_iterator.get(); @@ -1356,9 +1356,9 @@ TEST_F(ColumnReaderTest, MapDescendantRoutingPreservesActivePayloadAndVersion) { auto path = create_meta_access_path( {"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET}); - TDataAccessPath inactive_data_payload; - inactive_data_payload.__set_path(path.meta_access_path.path); - path.__set_data_access_path(std::move(inactive_data_payload)); + TDataAccessPath unselected_data_payload; + unselected_data_payload.__set_path(path.meta_access_path.path); + path.__set_data_access_path(std::move(unselected_data_payload)); auto legacy_path = create_data_access_path( {"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET}); legacy_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); @@ -1399,7 +1399,7 @@ TEST_F(ColumnReaderTest, MapDescendantRoutingPreservesActivePayloadAndVersion) { (std::vector {"value", ColumnIterator::ACCESS_OFFSET})); } -TEST_F(ColumnReaderTest, ArrayDescendantRoutingRewritesOnlyActivePayload) { +TEST_F(ColumnReaderTest, ArrayItemPathRoutingRewritesOnlySelectedPayload) { auto item_iterator = std::make_unique(); item_iterator->set_column_name("item"); auto* item = item_iterator.get(); @@ -1411,9 +1411,9 @@ TEST_F(ColumnReaderTest, ArrayDescendantRoutingRewritesOnlyActivePayload) { auto path = create_meta_access_path( {"a", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET}); - TDataAccessPath inactive_data_payload; - inactive_data_payload.__set_path(path.meta_access_path.path); - path.__set_data_access_path(std::move(inactive_data_payload)); + TDataAccessPath unselected_data_payload; + unselected_data_payload.__set_path(path.meta_access_path.path); + path.__set_data_access_path(std::move(unselected_data_payload)); auto st = array_iterator.set_access_paths({path}, {}); ASSERT_TRUE(st.ok()) << st.to_string(); From c976910fb84e887e502df16e360ca9a0da32b50c Mon Sep 17 00:00:00 2001 From: Hu Shenggang Date: Sun, 19 Jul 2026 10:18:14 +0800 Subject: [PATCH 3/9] [refactor](be) Use logical selectors for map access paths ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Map descendant routing rewrote logical KEYS and VALUES selectors to physical key/value child column names before forwarding paths. Those physical names are storage/schema details and should not participate in the access-path contract. Make Map child iterators consume KEYS/VALUES as their stable logical names, keep explicit selectors unchanged, and expand wildcard paths directly to KEYS/VALUES. Add focused coverage for physical-name independence and nested Map wildcard routing. ### Release note None ### Check List (For Author) - Test: Unit Test - `build-support/check-format.sh` - `./run-be-ut.sh --run --filter='ColumnReaderTest.*'` (blocked before compilation because `thirdparty/installed/lib64/liblance_c.a` is missing) - Behavior changed: No. Map routing uses the same key/value children but no longer depends on their physical names. - Does this need documentation: No --- be/src/storage/segment/column_reader.cpp | 45 +++++----- .../storage/segment/column_reader_test.cpp | 84 +++++++++++-------- 2 files changed, 73 insertions(+), 56 deletions(-) diff --git a/be/src/storage/segment/column_reader.cpp b/be/src/storage/segment/column_reader.cpp index 82e7a41ffeea7b..c17c488fb9a0a5 100644 --- a/be/src/storage/segment/column_reader.cpp +++ b/be/src/storage/segment/column_reader.cpp @@ -126,19 +126,18 @@ class DescendantAccessPathRouter final { ChildAccessPaths value; }; - // Map selectors are logical. Convert KEYS/VALUES to the actual key/value iterator names. For a - // wildcard, create a complete DATA path for keys and route trailing qualifiers to values. + // Map children use the logical KEYS/VALUES selectors as their access-path names, independent + // of physical child column names. Expand a wildcard to complete keys and route its trailing + // qualifiers to values. static Result route_map_paths_to_children( - TColumnAccessPaths all_paths, TColumnAccessPaths predicate_paths, - const std::string& key_name, const std::string& value_name) { + TColumnAccessPaths all_paths, TColumnAccessPaths predicate_paths) { MapChildAccessPaths child_paths; - auto status = distribute_map_paths(std::move(all_paths), key_name, value_name, - child_paths.key.all_paths, child_paths.value.all_paths); + auto status = distribute_map_paths(std::move(all_paths), child_paths.key.all_paths, + child_paths.value.all_paths); if (!status.ok()) { return ResultError(std::move(status)); } - status = distribute_map_paths(std::move(predicate_paths), key_name, value_name, - child_paths.key.predicate_paths, + status = distribute_map_paths(std::move(predicate_paths), child_paths.key.predicate_paths, child_paths.value.predicate_paths); if (!status.ok()) { return ResultError(std::move(status)); @@ -260,14 +259,14 @@ class DescendantAccessPathRouter final { } // Map `*` applies trailing qualifiers only to values, while locating entries still requires - // the complete key column as DATA. Construct a fresh key path because the source may be META, - // and preserve its version so child routing keeps the same legacy/typed encoding. - static TColumnAccessPath make_full_data_path_for_child(const TColumnAccessPath& version_source, - const std::string& child_name) { + // complete KEYS as DATA. Construct a fresh key path because the source may be META, and + // preserve its version so child routing keeps the same legacy/typed encoding. + static TColumnAccessPath make_full_data_path_for_map_keys( + const TColumnAccessPath& version_source) { TColumnAccessPath child_path; child_path.__set_type(TAccessPathType::DATA); TDataAccessPath data_path; - data_path.__set_path({child_name}); + data_path.__set_path({ColumnIterator::ACCESS_MAP_KEYS}); child_path.__set_data_access_path(data_path); if (version_source.__isset.version) { child_path.__set_version(version_source.version); @@ -295,8 +294,8 @@ class DescendantAccessPathRouter final { return selector; } - static Status distribute_map_paths(TColumnAccessPaths source_paths, const std::string& key_name, - const std::string& value_name, TColumnAccessPaths& key_paths, + static Status distribute_map_paths(TColumnAccessPaths source_paths, + TColumnAccessPaths& key_paths, TColumnAccessPaths& value_paths) { for (auto& path : source_paths) { const auto selector = DORIS_TRY(classify_map_selector(path)); @@ -304,16 +303,15 @@ class DescendantAccessPathRouter final { case MapSelector::WILDCARD: // A wildcard needs complete keys for runtime lookup, while any remaining // qualifiers apply only to the value. The value keeps its type and version. - key_paths.emplace_back(make_full_data_path_for_child(path, key_name)); - RETURN_IF_ERROR(replace_selected_payload_head(path, value_name)); + key_paths.emplace_back(make_full_data_path_for_map_keys(path)); + RETURN_IF_ERROR( + replace_selected_payload_head(path, ColumnIterator::ACCESS_MAP_VALUES)); value_paths.emplace_back(std::move(path)); break; case MapSelector::KEYS: - RETURN_IF_ERROR(replace_selected_payload_head(path, key_name)); key_paths.emplace_back(std::move(path)); break; case MapSelector::VALUES: - RETURN_IF_ERROR(replace_selected_payload_head(path, value_name)); value_paths.emplace_back(std::move(path)); break; case MapSelector::UNRECOGNIZED: @@ -1088,13 +1086,11 @@ Status ColumnReader::new_map_iterator(ColumnIteratorUPtr* iterator, &key_iterator, tablet_column && tablet_column->get_subtype_count() > 1 ? &tablet_column->get_sub_column(0) : nullptr)); - key_iterator->set_column_name(tablet_column ? tablet_column->get_sub_column(0).name() : ""); ColumnIteratorUPtr val_iterator; RETURN_IF_ERROR(_sub_readers[1]->new_iterator( &val_iterator, tablet_column && tablet_column->get_subtype_count() > 1 ? &tablet_column->get_sub_column(1) : nullptr)); - val_iterator->set_column_name(tablet_column ? tablet_column->get_sub_column(1).name() : ""); ColumnIteratorUPtr offsets_iterator; RETURN_IF_ERROR(_sub_readers[2]->new_iterator(&offsets_iterator, nullptr)); auto* file_iter = static_cast(offsets_iterator.release()); @@ -1298,6 +1294,10 @@ MapFileColumnIterator::MapFileColumnIterator(std::shared_ptr reade if (_map_reader->is_nullable()) { _null_iterator = std::move(null_iterator); } + // Access paths identify map children by logical selectors rather than storage/schema child + // names. These names are consumed by each child's _split_access_paths(). + _key_iterator->set_column_name(ACCESS_MAP_KEYS); + _val_iterator->set_column_name(ACCESS_MAP_VALUES); } Status MapFileColumnIterator::init(const ColumnIteratorOptions& opts) { @@ -1763,8 +1763,7 @@ Status MapFileColumnIterator::set_access_paths(const TColumnAccessPaths& all_acc } auto child_paths = DORIS_TRY(DescendantAccessPathRouter::route_map_paths_to_children( - std::move(plan.all.descendant_paths), std::move(plan.predicate.descendant_paths), - _key_iterator->column_name(), _val_iterator->column_name())); + std::move(plan.all.descendant_paths), std::move(plan.predicate.descendant_paths))); if (!child_paths.key.empty()) { RETURN_IF_ERROR(_key_iterator->set_access_paths(child_paths.key.all_paths, diff --git a/be/test/storage/segment/column_reader_test.cpp b/be/test/storage/segment/column_reader_test.cpp index 6a8e58b50374a7..6fd6cc0263f67e 100644 --- a/be/test/storage/segment/column_reader_test.cpp +++ b/be/test/storage/segment/column_reader_test.cpp @@ -742,9 +742,7 @@ TEST_F(ColumnReaderTest, LegacyDataSpecialComponentsRemainDataSelectors) { auto offsets_iterator = std::make_unique( std::make_unique(create_test_reader())); auto key_iterator = std::make_unique(create_test_reader()); - key_iterator->set_column_name("key"); auto value_iterator = std::make_unique(create_test_reader()); - value_iterator->set_column_name("value"); auto map_iterator = std::make_unique( create_test_reader(), nullptr, std::move(offsets_iterator), std::move(key_iterator), std::move(value_iterator)); @@ -824,9 +822,7 @@ TEST_F(ColumnReaderTest, TypedMetaPathsRouteThroughScalarAndComplexIterators) { auto offsets_iterator = std::make_unique( std::make_unique(create_test_reader())); auto key_iterator = std::make_unique(create_test_reader()); - key_iterator->set_column_name("key"); auto value_iterator = std::make_unique(create_test_reader()); - value_iterator->set_column_name("value"); MapFileColumnIterator map_iterator(create_test_reader(), nullptr, std::move(offsets_iterator), std::move(key_iterator), std::move(value_iterator)); @@ -883,9 +879,7 @@ TEST_F(ColumnReaderTest, TypedMetaPathsRouteThroughScalarAndComplexIterators) { auto offsets_iterator = std::make_unique( std::make_unique(create_test_reader())); auto key_iterator = std::make_unique(create_test_reader()); - key_iterator->set_column_name("key"); auto value_iterator = std::make_unique(create_test_reader()); - value_iterator->set_column_name("value"); MapFileColumnIterator map_iterator(create_test_reader(), nullptr, std::move(offsets_iterator), std::move(key_iterator), std::move(value_iterator)); @@ -1341,18 +1335,20 @@ TEST_F(ColumnReaderTest, SplitAccessPathsClassifiesCurrentAndDescendantPaths) { EXPECT_EQ(iterator._read_requirement, ColumnIterator::ReadRequirement::NORMAL); } -TEST_F(ColumnReaderTest, MapChildPathRoutingRewritesOnlySelectedPayloadAndPreservesVersion) { +TEST_F(ColumnReaderTest, MapChildPathRoutingUsesLogicalSelectorsAndPreservesVersion) { auto key_iterator = std::make_unique(); - key_iterator->set_column_name("key"); + key_iterator->set_column_name("physical_key"); auto* key = key_iterator.get(); auto value_iterator = std::make_unique(); - value_iterator->set_column_name("value"); + value_iterator->set_column_name("physical_value"); auto* value = value_iterator.get(); auto offsets_iterator = std::make_unique( std::make_unique(create_test_reader())); MapFileColumnIterator map_iterator(create_test_reader(), nullptr, std::move(offsets_iterator), std::move(key_iterator), std::move(value_iterator)); map_iterator.set_column_name("m"); + EXPECT_EQ(key->column_name(), ColumnIterator::ACCESS_MAP_KEYS); + EXPECT_EQ(value->column_name(), ColumnIterator::ACCESS_MAP_VALUES); auto path = create_meta_access_path( {"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET}); @@ -1371,13 +1367,15 @@ TEST_F(ColumnReaderTest, MapChildPathRoutingRewritesOnlySelectedPayloadAndPreser EXPECT_EQ(key_path.type, TAccessPathType::DATA); ASSERT_TRUE(key_path.__isset.version); EXPECT_EQ(key_path.version, g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); - EXPECT_EQ(key_path.data_access_path.path, (std::vector {"key"})); + EXPECT_EQ(key_path.data_access_path.path, + (std::vector {ColumnIterator::ACCESS_MAP_KEYS})); ASSERT_EQ(key->routed_predicate_access_paths.size(), 1); const auto& legacy_key_path = key->routed_predicate_access_paths[0]; EXPECT_EQ(legacy_key_path.type, TAccessPathType::DATA); ASSERT_TRUE(legacy_key_path.__isset.version); EXPECT_EQ(legacy_key_path.version, g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); - EXPECT_EQ(legacy_key_path.data_access_path.path, (std::vector {"key"})); + EXPECT_EQ(legacy_key_path.data_access_path.path, + (std::vector {ColumnIterator::ACCESS_MAP_KEYS})); ASSERT_EQ(value->routed_all_access_paths.size(), 1); const auto& value_path = value->routed_all_access_paths[0]; @@ -1385,7 +1383,8 @@ TEST_F(ColumnReaderTest, MapChildPathRoutingRewritesOnlySelectedPayloadAndPreser ASSERT_TRUE(value_path.__isset.version); EXPECT_EQ(value_path.version, g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_TYPED); EXPECT_EQ(value_path.meta_access_path.path, - (std::vector {"value", ColumnIterator::ACCESS_OFFSET})); + (std::vector {ColumnIterator::ACCESS_MAP_VALUES, + ColumnIterator::ACCESS_OFFSET})); EXPECT_EQ(value_path.data_access_path.path, (std::vector {"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_OFFSET})); @@ -1396,7 +1395,46 @@ TEST_F(ColumnReaderTest, MapChildPathRoutingRewritesOnlySelectedPayloadAndPreser EXPECT_EQ(legacy_value_path.version, g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); EXPECT_EQ(legacy_value_path.data_access_path.path, - (std::vector {"value", ColumnIterator::ACCESS_OFFSET})); + (std::vector {ColumnIterator::ACCESS_MAP_VALUES, + ColumnIterator::ACCESS_OFFSET})); +} + +TEST_F(ColumnReaderTest, NestedMapWildcardRoutingUsesLogicalSelectorsWithoutPhysicalNames) { + auto inner_key_iterator = std::make_unique(); + auto* inner_key = inner_key_iterator.get(); + auto inner_value_iterator = std::make_unique(); + auto* inner_value = inner_value_iterator.get(); + auto inner_offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + auto inner_map_iterator = std::make_unique( + create_test_reader(), nullptr, std::move(inner_offsets_iterator), + std::move(inner_key_iterator), std::move(inner_value_iterator)); + + auto outer_key_iterator = std::make_unique(); + auto* outer_key = outer_key_iterator.get(); + auto outer_offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + MapFileColumnIterator outer_map_iterator( + create_test_reader(), nullptr, std::move(outer_offsets_iterator), + std::move(outer_key_iterator), std::move(inner_map_iterator)); + outer_map_iterator.set_column_name("m"); + + auto path = + create_meta_access_path({"m", ColumnIterator::ACCESS_ALL, ColumnIterator::ACCESS_ALL, + ColumnIterator::ACCESS_OFFSET}); + auto st = outer_map_iterator.set_access_paths({path}, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + + ASSERT_EQ(outer_key->routed_all_access_paths.size(), 1); + EXPECT_EQ(outer_key->routed_all_access_paths[0].data_access_path.path, + (std::vector {ColumnIterator::ACCESS_MAP_KEYS})); + ASSERT_EQ(inner_key->routed_all_access_paths.size(), 1); + EXPECT_EQ(inner_key->routed_all_access_paths[0].data_access_path.path, + (std::vector {ColumnIterator::ACCESS_MAP_KEYS})); + ASSERT_EQ(inner_value->routed_all_access_paths.size(), 1); + EXPECT_EQ(inner_value->routed_all_access_paths[0].meta_access_path.path, + (std::vector {ColumnIterator::ACCESS_MAP_VALUES, + ColumnIterator::ACCESS_OFFSET})); } TEST_F(ColumnReaderTest, ArrayItemPathRoutingRewritesOnlySelectedPayload) { @@ -1828,10 +1866,8 @@ TEST_F(ColumnReaderTest, MapPredicateMetaPathDoesNotOverrideExistingDataNeed) { auto offsets_iter = std::make_unique( std::make_unique(std::make_shared())); auto key_iter = std::make_unique(std::make_shared()); - key_iter->set_column_name("key"); auto value_iter = std::make_unique(std::make_shared()); - value_iter->set_column_name("value"); auto map_iterator = std::make_unique( std::make_shared(), std::move(null_iter), std::move(offsets_iter), std::move(key_iter), std::move(value_iter)); @@ -1880,7 +1916,6 @@ TEST_F(ColumnReaderTest, MapFullProjectionStillRoutesPredicateSubPaths) { auto value_struct = std::make_unique( std::make_shared(), std::move(null_iter), std::move(sub_iters)); - value_struct->set_column_name("value"); return value_struct; }; @@ -1888,7 +1923,6 @@ TEST_F(ColumnReaderTest, MapFullProjectionStillRoutesPredicateSubPaths) { auto map_offsets_iter = std::make_unique( std::make_unique(std::make_shared())); auto map_key_iter = std::make_unique(std::make_shared()); - map_key_iter->set_column_name("key"); auto map_iterator = std::make_unique( std::make_shared(), std::move(map_null_iter), std::move(map_offsets_iter), std::move(map_key_iter), make_value_struct()); @@ -2013,7 +2047,6 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPaths) { auto value_struct = std::make_unique( std::make_shared(), std::move(null_iter), std::move(sub_iters)); - value_struct->set_column_name("value"); return value_struct; }; @@ -2021,7 +2054,6 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPaths) { auto map_offsets_iter = std::make_unique( std::make_unique(std::make_shared())); auto map_key_iter = std::make_unique(std::make_shared()); - map_key_iter->set_column_name("key"); auto map_val_iter = make_value_struct(); auto map_iterator = std::make_unique( std::make_shared(), std::move(map_null_iter), std::move(map_offsets_iter), @@ -2089,7 +2121,6 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto value_struct = std::make_unique( std::make_shared(), std::move(null_iter), std::move(sub_iters)); - value_struct->set_column_name("value"); return value_struct; }; @@ -2097,7 +2128,6 @@ TEST_F(ColumnReaderTest, NestedStructArrayMapStructAccessPathsVariants) { auto map_offsets_iter = std::make_unique( std::make_unique(std::make_shared())); auto map_key_iter = std::make_unique(std::make_shared()); - map_key_iter->set_column_name("key"); auto map_val_iter = make_value_struct(); auto map_iterator = std::make_unique( std::make_shared(), std::move(map_null_iter), @@ -2384,7 +2414,6 @@ TEST_F(ColumnReaderTest, DeepNestedAccessPathsFiveLevels) { auto value_struct = std::make_unique( std::make_shared(), std::move(null_iter), std::move(sub_iters)); - value_struct->set_column_name("value"); return value_struct; }; @@ -2392,7 +2421,6 @@ TEST_F(ColumnReaderTest, DeepNestedAccessPathsFiveLevels) { auto map_offsets_iter = std::make_unique( std::make_unique(std::make_shared())); auto map_key_iter = std::make_unique(std::make_shared()); - map_key_iter->set_column_name("key"); auto map_val_iter = make_value_struct(); auto map_iter = std::make_unique( std::make_shared(), std::move(map_null_iter), std::move(map_offsets_iter), @@ -2501,7 +2529,6 @@ TEST_F(ColumnReaderTest, NestedReadPhaseLazyOutputMatrix) { auto value_struct = std::make_unique( std::make_shared(), std::move(null_iter), std::move(sub_iters)); - value_struct->set_column_name("value"); return value_struct; }; @@ -2509,7 +2536,6 @@ TEST_F(ColumnReaderTest, NestedReadPhaseLazyOutputMatrix) { auto map_offsets_iter = std::make_unique( std::make_unique(std::make_shared())); auto map_key_iter = std::make_unique(std::make_shared()); - map_key_iter->set_column_name("key"); auto map_val_iter = make_value_struct(); auto map_iterator = std::make_unique( std::make_shared(), std::move(map_null_iter), @@ -3045,11 +3071,9 @@ TEST_F(ColumnReaderTest, MapPredicateAccessAllWithOffsetKeepsKeysReadable) { auto map_reader = create_test_reader(false, 0, FieldType::OLAP_FIELD_TYPE_MAP); auto key_iter = std::make_unique( create_test_reader(false, 0, FieldType::OLAP_FIELD_TYPE_STRING)); - key_iter->set_column_name("key"); auto* key_ptr = key_iter.get(); auto val_iter = std::make_unique( create_test_reader(false, 0, FieldType::OLAP_FIELD_TYPE_STRING)); - val_iter->set_column_name("value"); auto* val_ptr = val_iter.get(); auto offset_iterator = create_tracking_offset_iterator(); @@ -3157,10 +3181,8 @@ TEST_F(ColumnReaderTest, MapLazyReadByRowidsFillsSkippedKeysForValuesOnlyPath) { auto offsets_iter = std::make_unique( std::make_unique()); auto key_iter = std::make_unique(); - key_iter->set_column_name("key"); auto* key_ptr = key_iter.get(); auto val_iter = std::make_unique(); - val_iter->set_column_name("value"); auto* val_ptr = val_iter.get(); MapFileColumnIterator map_iter(map_reader, nullptr, std::move(offsets_iter), @@ -3198,10 +3220,8 @@ TEST_F(ColumnReaderTest, MapLazyReadByRowidsPreservesPredicateKeysForValuesOnlyP auto offsets_iter = std::make_unique( std::make_unique()); auto key_iter = std::make_unique(); - key_iter->set_column_name("key"); auto* key_ptr = key_iter.get(); auto val_iter = std::make_unique(); - val_iter->set_column_name("value"); auto* val_ptr = val_iter.get(); MapFileColumnIterator map_iter(map_reader, nullptr, std::move(offsets_iter), @@ -3251,9 +3271,7 @@ TEST_F(ColumnReaderTest, MapAccessAllWithOffsetDoesNotPropagateOffsetToKey) { auto offsets_iter = std::make_unique( std::make_unique(std::make_shared())); auto key_iter = std::make_unique(std::make_shared()); - key_iter->set_column_name("key"); auto val_iter = std::make_unique(std::make_shared()); - val_iter->set_column_name("value"); MapFileColumnIterator map_iter(map_reader, std::move(null_iter), std::move(offsets_iter), std::move(key_iter), std::move(val_iter)); From 0f316df374896eb02328f712574c4f1f22d85a50 Mon Sep 17 00:00:00 2001 From: Hu Shenggang Date: Sun, 19 Jul 2026 11:53:00 +0800 Subject: [PATCH 4/9] [doc](be) Document nested access path routing flow ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: DescendantAccessPathRouter documented its input assumptions but not the complete per-level processing flow. Add a class-level comment describing common path splitting, current DATA and metadata handling, container-specific Struct/Array/Map routing, child read requirements, and recursive nested processing. ### Release note None ### Check List (For Author) - Test: No need to test. Comment-only change. - `build-support/check-format.sh` - `git diff --check` - Behavior changed: No. - Does this need documentation: No --- be/src/storage/segment/column_reader.cpp | 27 ++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/be/src/storage/segment/column_reader.cpp b/be/src/storage/segment/column_reader.cpp index c17c488fb9a0a5..17dd9f164f4cb2 100644 --- a/be/src/storage/segment/column_reader.cpp +++ b/be/src/storage/segment/column_reader.cpp @@ -103,10 +103,29 @@ bool uses_legacy_access_path_encoding(const TColumnAccessPath& path) { namespace { -// Routes descendant paths produced by ColumnIterator::_split_access_paths() to the immediate data -// children of a nested column. Their versions have been validated, legacy paths have DATA type, -// and every payload selected by type is non-empty. Routing never interprets or rewrites the -// unselected compatibility payload. +// Nested access paths are processed one container level at a time: +// +// 1. Each Map/Array/Struct iterator's set_access_paths() calls _prepare_nested_access_paths(). For +// the all-path and predicate-path channels independently, _split_access_paths() validates the +// wire encoding and type-selected payload, removes the current iterator name, and separates +// requests consumed by the current iterator from paths that still address a data descendant. A +// current DATA request requires all data children. Struct owns current-level NULL metadata; +// Map and Array own NULL and OFFSET metadata. A supported metadata-only request can stop before +// descendant routing and mark every data child SKIP. +// 2. This router interprets only the first remaining component and routes the path according to +// the container topology: +// - Struct components already name fields. Select the paths for each field without rewriting. +// - Array `*` names its only item. Retarget `*` to the item iterator name. +// - Map `KEYS` and `VALUES` directly name its logical children. Map `*` creates a complete DATA +// path for `KEYS` and routes any trailing components through `VALUES`. +// 3. The container forwards the routed all-path and predicate-path channels to each selected +// child's set_access_paths(), then finalizes that child's PREDICATE/LAZY_OUTPUT/SKIP +// requirement. The child repeats the same flow, which handles arbitrary Map/Array/Struct +// nesting. +// +// At this point versions have been validated, legacy paths have DATA type, and every payload +// selected by type is non-empty. Routing never interprets or rewrites an unselected compatibility +// payload. class DescendantAccessPathRouter final { public: DescendantAccessPathRouter() = delete; From f01b15a0a7d03bd3a0e29ad31d9a48476fa1d0fe Mon Sep 17 00:00:00 2001 From: minghong Date: Mon, 20 Jul 2026 14:27:13 +0800 Subject: [PATCH 5/9] (fe)Emit struct-level NULL path for element_at over struct IS NULL --- .../AccessPathExpressionCollector.java | 40 ++++++ .../rules/rewrite/PruneNestedColumnTest.java | 127 +++++++++++++++--- 2 files changed, 148 insertions(+), 19 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java index 8abe242c3e1d69..5a4c4e1580feb3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java @@ -338,6 +338,23 @@ public Void visitElementAt(ElementAt elementAt, CollectorContext context) { Expression fieldName = arguments.get(1); DataType fieldType = fieldName.getDataType(); if (fieldName.isLiteral() && (fieldType.isIntegerLikeType() || fieldType.isStringLikeType())) { + // element_at(s, 'field') IS NULL has two-layer null semantics: + // s IS NULL OR s.field IS NULL + // Always emit META [s, NULL] for the struct-level null map. + // Only emit META [s, field, NULL] when the selected field itself is nullable. + if (context.type == ColumnAccessPathType.META + && isUnderIsNull(context.accessPathBuilder.getPathList())) { + StructField field = resolveStructField( + (StructType) first.getDataType(), fieldName); + // Path 1: struct-level null check — bypass field prefix + continueCollectAccessPath(first, copyContext(context)); + if (field != null && !field.isNullable()) { + // Non-nullable leaf: struct-level NULL is sufficient + return null; + } + // Nullable leaf: fall through to add field prefix → META [s, field, NULL] + } + if (fieldType.isIntegerLikeType()) { int fieldIndex = ((Number) ((Literal) fieldName).getValue()).intValue(); List fields = ((StructType) first.getDataType()).getFields(); @@ -654,6 +671,29 @@ public Void visitIsNull(IsNull isNull, CollectorContext context) { return visit(isNull, context); } + /** + * Resolve the StructField selected by a constant int/string literal. + * Returns null when the selector is not a recognized literal or index is out of bounds. + */ + static StructField resolveStructField(StructType structType, Expression fieldExpr) { + if (!fieldExpr.isLiteral()) { + return null; + } + if (fieldExpr.getDataType().isIntegerLikeType()) { + int index = ((Number) ((Literal) fieldExpr).getValue()).intValue(); + List fields = structType.getFields(); + if (index >= 1 && index <= fields.size()) { + return fields.get(index - 1); + } + return null; + } + if (fieldExpr.getDataType().isStringLikeType()) { + String name = ((Literal) fieldExpr).getStringValue().toLowerCase(); + return structType.getField(name); + } + return null; + } + @Override public Void visitIf(If ifExpr, CollectorContext context) { if (isUnderIsNull(context.accessPathBuilder.accessPath)) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java index ca53366e5cc0bb..30c21d975696c6 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java @@ -142,6 +142,13 @@ public void createTable() throws Exception { + " s struct<`NULL`: string, `OFFSET`: string>\n" + ") properties ('replication_num'='1')"); + // Nested struct for verifying multi-level META NULL path generation. + // All fields are nullable by default (no NOT NULL in DDL). + createTable("create table nested_struct_tbl(\n" + + " id int,\n" + + " s struct<`outer`: struct<`a`: string, `inner_f`: string>>\n" + + ") properties ('replication_num'='1')"); + connectContext.getSessionVariable().setDisableNereidsRules(RuleType.PRUNE_EMPTY_PARTITION.name()); connectContext.getSessionVariable().enableNereidsTimeout = false; } @@ -203,8 +210,8 @@ public void testFullFieldAccessKeepsExactMetadataPath() throws Exception { assertColumn("select element_at(s, 'city') from tbl " + "where element_at(s, 'city') is null", "struct", - ImmutableList.of(path("s", "city"), metaPath("s", "city", "NULL")), - ImmutableList.of(metaPath("s", "city", "NULL"))); + ImmutableList.of(path("s", "city"), metaPath("s", "NULL"), metaPath("s", "city", "NULL")), + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL"))); assertColumn("select cardinality(element_at(s, 'data')), element_at(s, 'data') from tbl", "struct>>>", @@ -623,14 +630,14 @@ public void testFilter() throws Throwable { assertColumn("select 100 from tbl where element_at(s, 'city') is not null", "struct", - ImmutableList.of(metaPath("s", "city", "NULL")), - ImmutableList.of(metaPath("s", "city", "NULL")) + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL")), + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL")) ); assertColumn("select 100 from tbl where element_at(s, 'data') is not null", "struct>>>", - ImmutableList.of(metaPath("s", "data", "NULL")), - ImmutableList.of(metaPath("s", "data", "NULL")) + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "data", "NULL")), + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "data", "NULL")) ); assertColumn("select 100 from tbl where element_at(s, 'data')[1] is not null", "struct>>>", @@ -649,8 +656,10 @@ public void testFilter() throws Throwable { ); assertColumn("select 100 from tbl where element_at(map_values(element_at(s, 'data')[1])[1], 'a') is not null", "struct>>>", - ImmutableList.of(metaPath("s", "data", "*", "VALUES", "a", "NULL")), - ImmutableList.of(metaPath("s", "data", "*", "VALUES", "a", "NULL")) + ImmutableList.of(metaPath("s", "data", "*", "VALUES", "NULL"), + metaPath("s", "data", "*", "VALUES", "a", "NULL")), + ImmutableList.of(metaPath("s", "data", "*", "VALUES", "NULL"), + metaPath("s", "data", "*", "VALUES", "a", "NULL")) ); assertColumn("select 100 from tbl where element_at(s, 'data')[1][1] is not null", "struct>>>", @@ -659,13 +668,21 @@ public void testFilter() throws Throwable { ); assertColumn("select 100 from tbl where element_at(element_at(s, 'data')[1][1], 'a') is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "a", "NULL")), - ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "a", "NULL")) + ImmutableList.of(path("s", "data", "*", "KEYS"), + metaPath("s", "data", "*", "VALUES", "NULL"), + metaPath("s", "data", "*", "VALUES", "a", "NULL")), + ImmutableList.of(path("s", "data", "*", "KEYS"), + metaPath("s", "data", "*", "VALUES", "NULL"), + metaPath("s", "data", "*", "VALUES", "a", "NULL")) ); assertColumn("select 100 from tbl where element_at(element_at(s, 'data')[1][1], 'b') is not null", "struct>>>", - ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "b", "NULL")), - ImmutableList.of(path("s", "data", "*", "KEYS"), metaPath("s", "data", "*", "VALUES", "b", "NULL")) + ImmutableList.of(path("s", "data", "*", "KEYS"), + metaPath("s", "data", "*", "VALUES", "NULL"), + metaPath("s", "data", "*", "VALUES", "b", "NULL")), + ImmutableList.of(path("s", "data", "*", "KEYS"), + metaPath("s", "data", "*", "VALUES", "NULL"), + metaPath("s", "data", "*", "VALUES", "b", "NULL")) ); } @@ -703,17 +720,19 @@ public void testProjectFilter() throws Throwable { "struct>>>", ImmutableList.of( path("s", "data"), + metaPath("s", "NULL"), metaPath("s", "city", "NULL")), - ImmutableList.of(metaPath("s", "city", "NULL")) + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL")) ); assertColumn("select element_at(s, 'data') from tbl where element_at(s, 'city') is not null and element_at(s, 'data') is not null", "struct>>>", ImmutableList.of( path("s", "data"), + metaPath("s", "NULL"), metaPath("s", "city", "NULL"), metaPath("s", "data", "NULL")), - ImmutableList.of(metaPath("s", "city", "NULL"), metaPath("s", "data", "NULL")) + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL"), metaPath("s", "data", "NULL")) ); } @@ -1127,8 +1146,8 @@ public void testDataPathNamedLikeMetadataComponent() { public void testMetadataPathBelowSameNamedStructField() throws Exception { assertColumn("select 1 from meta_name_tbl where element_at(s, 'NULL') is null", "struct", - ImmutableList.of(metaPath("s", "null", "NULL")), - ImmutableList.of(metaPath("s", "null", "NULL"))); + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "null", "NULL")), + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "null", "NULL"))); assertColumn("select length(element_at(s, 'OFFSET')) from meta_name_tbl", "struct", @@ -1453,6 +1472,76 @@ public void testStructIsNotNullPruning() throws Exception { ImmutableList.of(metaPath("s", "NULL"))); } + @Test + public void testResolveStructFieldNullable() { + StructType type = new StructType(ImmutableList.of( + new StructField("not_null_f", StringType.INSTANCE, false, ""), + new StructField("nullable_f", StringType.INSTANCE, true, "") + )); + // String-like literal: select by name + StructField notNullField = AccessPathExpressionCollector.resolveStructField( + type, new org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral("not_null_f")); + Assertions.assertNotNull(notNullField); + Assertions.assertFalse(notNullField.isNullable()); + + StructField nullableField = AccessPathExpressionCollector.resolveStructField( + type, new org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral("nullable_f")); + Assertions.assertNotNull(nullableField); + Assertions.assertTrue(nullableField.isNullable()); + + // Integer-like literal: select by 1-based index + StructField fieldByIndex = AccessPathExpressionCollector.resolveStructField( + type, new org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral(1)); + Assertions.assertNotNull(fieldByIndex); + Assertions.assertEquals("not_null_f", fieldByIndex.getName()); + Assertions.assertFalse(fieldByIndex.isNullable()); + + // Out-of-bounds index returns null + StructField outOfBounds = AccessPathExpressionCollector.resolveStructField( + type, new org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral(99)); + Assertions.assertNull(outOfBounds); + + // Non-literal returns null + StructField nonLiteral = AccessPathExpressionCollector.resolveStructField( + type, NullLiteral.INSTANCE); + Assertions.assertNull(nonLiteral); + } + + @Test + public void testNestedStructFieldNullAccess() throws Exception { + // element_at(element_at(s, 'outer'), 'a') IS NULL generates three META NULL paths: + // one for each nullable level in the chain. + // s IS NULL → META [s, NULL] + // s.outer IS NULL → META [s, outer, NULL] + // s.outer.a IS NULL → META [s, outer, a, NULL] + assertColumn("select 1 from nested_struct_tbl " + + "where element_at(element_at(s, 'outer'), 'a') is null", + "struct>", + ImmutableList.of( + metaPath("s", "NULL"), + metaPath("s", "outer", "NULL"), + metaPath("s", "outer", "a", "NULL")), + ImmutableList.of( + metaPath("s", "NULL"), + metaPath("s", "outer", "NULL"), + metaPath("s", "outer", "a", "NULL"))); + + // With IS NOT NULL on a field that's the only predicate access. + // SELECT element_at(s, 'outer') reads the full outer sub-struct → both a and inner_f. + assertColumn("select element_at(s, 'outer') from nested_struct_tbl " + + "where element_at(element_at(s, 'outer'), 'inner_f') is not null", + "struct>", + ImmutableList.of( + path("s", "outer"), + metaPath("s", "NULL"), + metaPath("s", "outer", "NULL"), + metaPath("s", "outer", "inner_f", "NULL")), + ImmutableList.of( + metaPath("s", "NULL"), + metaPath("s", "outer", "NULL"), + metaPath("s", "outer", "inner_f", "NULL"))); + } + @Test public void testScalarIsNullProducesMetaPath() { SlotReference slot = rewriteAndFindScanSlot("select 1 from tbl where id is null", "id", false); @@ -1475,7 +1564,7 @@ public void testStructIsNullMixedAccess() throws Exception { assertColumn("select s from tbl where element_at(s, 'city') is null", "struct>>>", ImmutableList.of(path("s")), - ImmutableList.of(metaPath("s", "city", "NULL"))); + ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL"))); // This shape is closer to the production bug: one predicate needs the parent // null map, another predicate needs a child null map, and the projection needs @@ -1591,10 +1680,10 @@ public void testMvRewritePlanFragmentSkipsNullOnlyAccessPath() { SlotReference nestedNormalSlot = rewriteAndFindScanSlot( "select 1 from tbl where element_at(s, 'city') is not null", "s", false); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(metaPath("s", "city", "NULL"))), + new TreeSet<>(ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL"))), new TreeSet<>(nestedNormalSlot.getAllAccessPaths().get())); Assertions.assertEquals( - new TreeSet<>(ImmutableList.of(metaPath("s", "city", "NULL"))), + new TreeSet<>(ImmutableList.of(metaPath("s", "NULL"), metaPath("s", "city", "NULL"))), new TreeSet<>(nestedNormalSlot.getPredicateAccessPaths().get())); // MV fragment: IS NULL degrades to element_at via default visitor, From ff3bfef2791a78df1f190919995bff2aeb7997d5 Mon Sep 17 00:00:00 2001 From: Hu Shenggang Date: Mon, 20 Jul 2026 11:22:23 +0800 Subject: [PATCH 6/9] [fix](be) Reject invalid map access path selectors ### What problem does this PR solve? Issue Number: None Related PR: #65591 Problem Summary: Map descendant access-path routing silently ignored selectors other than `*`, `KEYS`, and `VALUES`. This could drop the only descendant path and prune both key and value iterators instead of reporting an invalid FE-BE access-path contract. Reject invalid selectors during classification while continuing to inspect only the payload selected by the path type. ### Release note None ### Check List (For Author) - Test: Unit Test - Added `ColumnReaderTest.MapRejectsUnrecognizedDescendantSelectors` - Focused UT execution was blocked before linking because `thirdparty/installed/lib64/liblance_c.a` is missing - `build-support/check-format.sh` - `git diff --check` - Behavior changed: Yes. Invalid Map descendant access-path selectors now return an internal error instead of being silently ignored. - Does this need documentation: No --- be/src/storage/segment/column_reader.cpp | 14 +++++--- .../storage/segment/column_reader_test.cpp | 36 +++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/be/src/storage/segment/column_reader.cpp b/be/src/storage/segment/column_reader.cpp index 17dd9f164f4cb2..ffd96ebc257183 100644 --- a/be/src/storage/segment/column_reader.cpp +++ b/be/src/storage/segment/column_reader.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -225,7 +226,7 @@ class DescendantAccessPathRouter final { private: enum class PathHeadMatchMode { EXACT, CASE_INSENSITIVE }; - enum class MapSelector { UNRECOGNIZED, WILDCARD, KEYS, VALUES }; + enum class MapSelector { WILDCARD, KEYS, VALUES }; // TColumnAccessPath may carry both payload fields after compatibility forwarding. Selected // means data_access_path for DATA and meta_access_path for META. Visit only that payload and @@ -294,7 +295,7 @@ class DescendantAccessPathRouter final { } static Result classify_map_selector(const TColumnAccessPath& access_path) { - MapSelector selector = MapSelector::UNRECOGNIZED; + std::optional selector; auto status = visit_selected_payload(access_path, [&](const auto& payload) { DORIS_CHECK(!payload.path.empty()); const auto& head = payload.path.front(); @@ -304,13 +305,18 @@ class DescendantAccessPathRouter final { selector = MapSelector::KEYS; } else if (head == ColumnIterator::ACCESS_MAP_VALUES) { selector = MapSelector::VALUES; + } else { + return Status::InternalError( + "Invalid map access path selector '{}': expected '*', 'KEYS', or 'VALUES'", + head); } return Status::OK(); }); if (!status.ok()) { return ResultError(std::move(status)); } - return selector; + DORIS_CHECK(selector.has_value()); + return *selector; } static Status distribute_map_paths(TColumnAccessPaths source_paths, @@ -333,8 +339,6 @@ class DescendantAccessPathRouter final { case MapSelector::VALUES: value_paths.emplace_back(std::move(path)); break; - case MapSelector::UNRECOGNIZED: - break; } } return Status::OK(); diff --git a/be/test/storage/segment/column_reader_test.cpp b/be/test/storage/segment/column_reader_test.cpp index 6fd6cc0263f67e..fe6c6961272ed0 100644 --- a/be/test/storage/segment/column_reader_test.cpp +++ b/be/test/storage/segment/column_reader_test.cpp @@ -1335,6 +1335,42 @@ TEST_F(ColumnReaderTest, SplitAccessPathsClassifiesCurrentAndDescendantPaths) { EXPECT_EQ(iterator._read_requirement, ColumnIterator::ReadRequirement::NORMAL); } +TEST_F(ColumnReaderTest, MapRejectsUnrecognizedDescendantSelectors) { + auto make_map_iterator = []() { + auto offsets_iterator = std::make_unique( + std::make_unique(create_test_reader())); + auto map_iterator = std::make_unique( + create_test_reader(), nullptr, std::move(offsets_iterator), + std::make_unique(), + std::make_unique()); + map_iterator->set_column_name("m"); + return map_iterator; + }; + + std::vector invalid_paths; + invalid_paths.emplace_back(create_data_access_path({"m", "UNKNOWN"})); + invalid_paths.emplace_back( + create_meta_access_path({"m", "UNKNOWN", ColumnIterator::ACCESS_NULL})); + invalid_paths.emplace_back(create_legacy_data_access_path({"m", "UNKNOWN"})); + + for (const auto& path : invalid_paths) { + auto map_iterator = make_map_iterator(); + auto st = map_iterator->set_access_paths({path}, {}); + EXPECT_FALSE(st.ok()); + EXPECT_THAT(st.to_string(), ::testing::HasSubstr("Invalid map access path selector")); + EXPECT_THAT(st.to_string(), ::testing::HasSubstr("UNKNOWN")); + } + + auto path = create_meta_access_path( + {"m", ColumnIterator::ACCESS_MAP_VALUES, ColumnIterator::ACCESS_OFFSET}); + TDataAccessPath unselected_data_payload; + unselected_data_payload.__set_path({"m", "UNKNOWN"}); + path.__set_data_access_path(std::move(unselected_data_payload)); + auto map_iterator = make_map_iterator(); + auto st = map_iterator->set_access_paths({path}, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); +} + TEST_F(ColumnReaderTest, MapChildPathRoutingUsesLogicalSelectorsAndPreservesVersion) { auto key_iterator = std::make_unique(); key_iterator->set_column_name("physical_key"); From 93a6288c4c892b404df9e9a692a079d5434f9b5b Mon Sep 17 00:00:00 2001 From: Hu Shenggang Date: Mon, 20 Jul 2026 15:35:18 +0800 Subject: [PATCH 7/9] [test](fe) Validate struct parent null access paths Issue Number: None Related PR: #65591 Problem Summary: Emitting the parent Struct null path for `element_at` null predicates changes the nested-column plan from a child-only predicate path to parent and child null paths. Update the regression assertions and comments to validate both paths. Also replace a misplaced Javadoc marker that caused FE Checkstyle to fail. None - Test: Regression test / Manual test - `./build.sh --fe` - `./run-regression-test.sh --conf output/local-regression/regression-conf-20000.groovy --run -d nereids_rules_p0/column_pruning -s null_column_pruning` - `git diff --check` - Behavior changed: No. This updates validation for the existing Struct null-path behavior. - Does this need documentation: No --- .../rules/rewrite/AccessPathExpressionCollector.java | 1 + .../column_pruning/null_column_pruning.groovy | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java index 5a4c4e1580feb3..5223f8d96c7838 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java @@ -675,6 +675,7 @@ public Void visitIsNull(IsNull isNull, CollectorContext context) { * Resolve the StructField selected by a constant int/string literal. * Returns null when the selector is not a recognized literal or index is out of bounds. */ + // VisibleForTesting static StructField resolveStructField(StructType structType, Expression fieldExpr) { if (!fieldExpr.isLiteral()) { return null; diff --git a/regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy b/regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy index c18f03df12f4ac..99898f2e29d82e 100644 --- a/regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy +++ b/regression-test/suites/nereids_rules_p0/column_pruning/null_column_pruning.groovy @@ -220,14 +220,13 @@ suite("null_column_pruning") { order_qt_12 "select struct_col from ncp_tbl where struct_col is null"; // ─── Nested struct field IS NULL ──────────────────────────────────────────── - // element_at(struct_col, 'city') IS NULL should produce a null-flag-only - // predicate path [struct_col.city.NULL] while the projection reads city data. - // [struct_col.city.NULL] remains in predicateAccessPaths beside the projected city data path. + // element_at(struct_col, 'city') IS NULL needs both the parent Struct null map and the + // selected field null map, while the projection reads city data. explain { sql "select element_at(struct_col, 'city') from ncp_tbl where element_at(struct_col, 'city') is null" contains "nested columns" contains "struct_col.city" - contains "predicate access paths: [struct_col.city.NULL]" + contains "predicate access paths: [struct_col.NULL, struct_col.city.NULL]" } order_qt_13 "select element_at(struct_col, 'city') from ncp_tbl where element_at(struct_col, 'city') is null"; @@ -385,6 +384,7 @@ suite("null_column_pruning") { sql "select count(1) from ncp_tbl where element_at(struct_col, 'city') is not null" contains "nested columns" contains "struct_col.city.NULL" + contains "struct_col.NULL" } order_qt_24 "select count(1) from ncp_tbl where element_at(struct_col, 'city') is not null"; From 53f73f0e240341bc38b7e81d5af231771ce723b6 Mon Sep 17 00:00:00 2001 From: minghong Date: Mon, 20 Jul 2026 19:49:24 +0800 Subject: [PATCH 8/9] [P1] Do not map expression nullability to a non-null Struct --- .../AccessPathExpressionCollector.java | 42 +++++++++++++++++-- .../rules/rewrite/PruneNestedColumnTest.java | 40 ++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java index 5223f8d96c7838..a591ec6d9201b7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java @@ -18,6 +18,7 @@ package org.apache.doris.nereids.rules.rewrite; import org.apache.doris.analysis.ColumnAccessPathType; +import org.apache.doris.catalog.Column; import org.apache.doris.nereids.StatementContext; import org.apache.doris.nereids.rules.rewrite.AccessPathExpressionCollector.CollectorContext; import org.apache.doris.nereids.rules.rewrite.NestedColumnPruning.DataTypeAccessTree; @@ -138,16 +139,33 @@ public Void visitSlotReference(SlotReference slotReference, CollectorContext con return null; } if (dataType instanceof NestedColumnPrunable) { + // A META path ending in NULL directly on the slot means "read the slot's null map". + // Check the physical column's nullability (via getOriginalColumn), NOT the slot's + // nullability which may be synthetic (e.g. from outer join). If the physical column + // has no null map or is unknown, suppress this path. + // (Field-level null paths like [s, field, NULL] were already validated upstream.) + if (context.type == ColumnAccessPathType.META + && isUnderIsNull(context.accessPathBuilder.accessPath) + && !hasPhysicalNullMap(slotReference)) { + return null; + } context.accessPathBuilder.addPrefix(slotReference.getName().toLowerCase()); ImmutableList path = Utils.fastToImmutableList(context.accessPathBuilder.accessPath); int slotId = slotReference.getExprId().asInt(); slotToAccessPaths.put(slotId, new CollectAccessPathResult(path, context.bottomFilter, context.type)); + return null; } if (dataType.isStringLikeType()) { int slotId = slotReference.getExprId().asInt(); if (!context.accessPathBuilder.isEmpty()) { // Accessed via an offset-only function (e.g. length()) or null-check (IS NULL). // Builder already has "OFFSET"/"NULL" at the tail; add the column name as prefix. + // For META NULL paths, suppress when the physical column has no null map. + if (context.type == ColumnAccessPathType.META + && isUnderIsNull(context.accessPathBuilder.accessPath) + && !hasPhysicalNullMap(slotReference)) { + return null; + } context.accessPathBuilder.addPrefix(slotReference.getName()); ImmutableList path = ImmutableList.copyOf(context.accessPathBuilder.accessPath); slotToAccessPaths.put(slotId, @@ -164,8 +182,12 @@ public Void visitSlotReference(SlotReference slotReference, CollectorContext con // For any other nullable column type (e.g. INT, BIGINT) accessed via IS NULL / IS NOT NULL: // record the [col_name, NULL] path so NestedColumnPruning can emit null-only access paths. // Skip NestedColumnPrunable types (already handled above) and string types (handled above). + // Check getOriginalColumn() rather than slotReference.nullable(): the latter may be + // inflated by outer join, while the former reflects the physical column's null map. + // Only NULL paths need the null-map check; OFFSET paths don't depend on nullability. if (!(dataType instanceof NestedColumnPrunable) && !dataType.isStringLikeType() - && !context.accessPathBuilder.isEmpty() && slotReference.nullable()) { + && isUnderIsNull(context.accessPathBuilder.accessPath) + && hasPhysicalNullMap(slotReference)) { context.accessPathBuilder.addPrefix(slotReference.getName()); ImmutableList path = ImmutableList.copyOf(context.accessPathBuilder.accessPath); int slotId = slotReference.getExprId().asInt(); @@ -173,8 +195,10 @@ public Void visitSlotReference(SlotReference slotReference, CollectorContext con new CollectAccessPathResult(path, context.bottomFilter, context.type)); } // For any other nullable column type accessed directly (not via IS NULL / length / etc.): - // record a [col_name] full-access path so that when the column is also used via IS NULL, - // stripNullSuffixPaths correctly suppresses the null-only optimization. + // record a [col_name] full-access path. When the same column also has a META NULL + // path (e.g. from IS NULL), both paths are sent to BE. The presence of a DATA path + // signals that full column data is needed, preventing the BE from entering + // NULL_MAP_ONLY mode which would read only the null bitmap. if (!(dataType instanceof NestedColumnPrunable) && !dataType.isStringLikeType() && !(dataType instanceof VariantType) && context.accessPathBuilder.isEmpty() && slotReference.nullable()) { @@ -898,6 +922,18 @@ public int hashCode() { } } + /** + * Check whether the physical column backing a SlotReference has a null map on disk. + * Uses the catalog Column's isAllowNull, not the slot's nullable() which may be + * inflated by outer join (via withNullable(true)). Returns false when the slot has + * no physical column (conservative: assume no null map). + */ + private static boolean hasPhysicalNullMap(SlotReference slot) { + return slot.getOriginalColumn() + .map(Column::isAllowNull) + .orElse(false); + } + // if the map type is changed, we can not prune the type, because the map type need distinct the keys, // e.g. select map_values(cast(map(3.0, 1, 3.1, 2) as map)); // the result is [2] because the keys: 3.0 and 3.1 will cast to 3 and the second entry remained. diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java index 30c21d975696c6..5fb7ea7a3b7ad4 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java @@ -149,6 +149,17 @@ public void createTable() throws Exception { + " s struct<`outer`: struct<`a`: string, `inner_f`: string>>\n" + ") properties ('replication_num'='1')"); + // Tables for outer-join nullability test: verifying that synthetic nullability + // from outer join does NOT cause META NULL paths on physically NOT NULL columns. + createTable("create table driving_tbl(\n" + + " id int\n" + + ") properties ('replication_num'='1')"); + + createTable("create table not_null_struct_tbl(\n" + + " id int,\n" + + " s struct not null\n" + + ") properties ('replication_num'='1')"); + connectContext.getSessionVariable().setDisableNereidsRules(RuleType.PRUNE_EMPTY_PARTITION.name()); connectContext.getSessionVariable().enableNereidsTimeout = false; } @@ -1887,4 +1898,33 @@ private void assertVariantSubColumnSlotCount(String sql, List expectedSu Assertions.assertEquals(expectedCount, actualCount); } + + /** + * Verify that synthetic nullability from outer join does NOT cause META NULL paths + * on physically NOT NULL columns. When a NOT NULL struct sits on the nullable side + * of a LEFT JOIN, the slot's {@code nullable()} returns true (from outer join + * semantics), but {@code getOriginalColumn().isAllowNull()} returns false + * (physical column has no null map). The fix in AccessPathExpressionCollector + * should suppress the {@code [s, NULL]} META path in this case. + */ + @Test + public void testNotNullStructOnOuterJoinNullableSide() throws Exception { + // driving_tbl LEFT JOIN not_null_struct_tbl: + // not_null_struct_tbl.s is NOT NULL in the schema, but after LEFT JOIN the + // slot becomes nullable (right side of LEFT JOIN → withNullable(true)). + // element_at(s, 'f') IS NULL in WHERE: + // - s.nullable() = true (synthetic, from outer join) + // - s.getOriginalColumn().isAllowNull() = false (physical, no null map) + // Expected: [s, f] DATA is present (field is read for IS NULL evaluation), + // [s, NULL] META must NOT be present (no physical null map). + assertAllAccessPathsContain( + "select driving_tbl.id from driving_tbl" + + " left join not_null_struct_tbl" + + " on driving_tbl.id = not_null_struct_tbl.id" + + " where element_at(not_null_struct_tbl.s, 'f') is null", + // expect-contain: field is read (DATA path) + ImmutableList.of(path("s", "f")), + // expect-NOT-contain: struct-level NULL path must be suppressed + ImmutableList.of(metaPath("s", "NULL"))); + } } From aa59c38ee5bdbfc08b2d70f56e385e93e8644c0e Mon Sep 17 00:00:00 2001 From: minghong Date: Mon, 20 Jul 2026 20:34:56 +0800 Subject: [PATCH 9/9] [P1] Keep the field referenced by the unchanged predicate --- .../AccessPathExpressionCollector.java | 10 +- .../rules/rewrite/PruneNestedColumnTest.java | 136 ++++++++++++++++++ 2 files changed, 144 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java index a591ec6d9201b7..dff0f3b0ec125f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java @@ -373,8 +373,14 @@ && isUnderIsNull(context.accessPathBuilder.getPathList())) { // Path 1: struct-level null check — bypass field prefix continueCollectAccessPath(first, copyContext(context)); if (field != null && !field.isNullable()) { - // Non-nullable leaf: struct-level NULL is sufficient - return null; + // Non-nullable leaf: struct-level NULL is sufficient for the + // IS NULL check, so no META NULL path for the field is needed. + // However the field must still appear in the type so pruneDataType + // preserves it — the filter expression still references + // element_at(s, 'f') IS NULL and won't be rewritten to s IS NULL. + // Fall through with a fresh DATA context to emit [s, f] DATA. + context = new CollectorContext( + context.statementContext, context.bottomFilter); } // Nullable leaf: fall through to add field prefix → META [s, field, NULL] } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java index 5fb7ea7a3b7ad4..ee70671a89d2b3 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java @@ -30,6 +30,7 @@ import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.ArrayItemReference; import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.IsNull; import org.apache.doris.nereids.trees.expressions.NamedExpression; import org.apache.doris.nereids.trees.expressions.Slot; import org.apache.doris.nereids.trees.expressions.SlotReference; @@ -43,6 +44,7 @@ import org.apache.doris.nereids.trees.plans.physical.PhysicalUnion; import org.apache.doris.nereids.types.BigIntType; import org.apache.doris.nereids.types.DataType; +import org.apache.doris.nereids.types.IntegerType; import org.apache.doris.nereids.types.NestedColumnPrunable; import org.apache.doris.nereids.types.NullType; import org.apache.doris.nereids.types.StringType; @@ -54,13 +56,16 @@ import org.apache.doris.planner.PlanFragment; import org.apache.doris.utframe.TestWithFeService; +import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Multimap; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -160,6 +165,16 @@ public void createTable() throws Exception { + " s struct not null\n" + ") properties ('replication_num'='1')"); + // Table for verifying that NOT NULL struct FIELD is preserved in pruned type + // when a sibling field is also accessed. + // Doris DDL does not support NOT NULL on individual struct fields, so the + // NOT NULL branch is tested via testNotNullFieldPreservedInAccessPaths + // which constructs the StructType programmatically. + createTable("create table nullable_struct_tbl_two_fields(\n" + + " id int,\n" + + " s struct\n" + + ") properties ('replication_num'='1')"); + connectContext.getSessionVariable().setDisableNereidsRules(RuleType.PRUNE_EMPTY_PARTITION.name()); connectContext.getSessionVariable().enableNereidsTimeout = false; } @@ -1927,4 +1942,125 @@ public void testNotNullStructOnOuterJoinNullableSide() throws Exception { // expect-NOT-contain: struct-level NULL path must be suppressed ImmutableList.of(metaPath("s", "NULL"))); } + + /** + * Verifies that a NOT NULL struct field is preserved in the pruned type when a + * sibling field is accessed via SELECT and the IS NULL check emits struct-level + * META NULL. Before the fix, the early return at visitElementAt dropped the NOT NULL + * field's path, causing pruneDataType to remove it from the struct type. The filter + * expression still referenced element_at(s, 'f') IS NULL and could not be rebuilt. + */ + @Test + public void testNullableFieldPreservedWithSiblingProjection() throws Exception { + // s STRUCT NULL (both fields nullable by default) + // SELECT element_at(s, 'g') → [s, g] DATA + // WHERE element_at(s, 'f') IS NULL → [s, NULL] META + [s, f, NULL] META + // Both fields preserved in pruned type. + assertColumn( + "select element_at(s, 'g') from nullable_struct_tbl_two_fields" + + " where element_at(s, 'f') is null", + "struct", + ImmutableList.of( + path("s", "g"), + metaPath("s", "NULL"), + metaPath("s", "f", "NULL")), + ImmutableList.of( + metaPath("s", "NULL"), + metaPath("s", "f", "NULL"))); + } + + /** + * Tests the NOT NULL struct field branch in visitElementAt line 375-384, + * complementing {@link #testNotNullStructOnOuterJoinNullableSide()} and + * {@link #testNullableFieldPreservedWithSiblingProjection()}. + * + *

Relationship with other tests

+ *
    + *
  • {@code testNotNullStructOnOuterJoinNullableSide}: the struct itself is + * NOT NULL → no struct null map → [s, NULL] META must be suppressed.
    • + *
  • {@code testNullableFieldPreservedWithSiblingProjection}: the struct IS + * nullable AND the field IS nullable → [s, NULL] META + [s, f, NULL] META + * both emitted, field preserved via the META path.
    • + *
  • This test: the struct IS nullable BUT the field is NOT NULL → + * [s, NULL] META is emitted for the struct null map, but [s, f, NULL] + * META is NOT emitted (no field-level null map). The fix must still emit + * [s, f] DATA so pruneDataType preserves the field in the struct type — + * the filter expression {@code element_at(s, 'f') IS NULL} still + * references 'f' and won't be rewritten to {@code s IS NULL}.
    • + *
+ * + *

Why manual construction instead of SQL

+ * Doris DDL does not support {@code NOT NULL} on individual struct fields + * ({@code struct} triggers a syntax error). This test + * therefore constructs the {@link StructType} with a nullable=false field + * programmatically and calls {@link AccessPathExpressionCollector} directly. + * Because the {@link SlotReference} lacks an {@code originalColumn}, + * {@code hasPhysicalNullMap} returns false, so [s, NULL] META is suppressed + * by the slot-level guard — that path is covered by the SQL-based + * {@code testNullableFieldPreservedWithSiblingProjection}. + */ + @Test + public void testNotNullFieldPreservedInAccessPaths() { + // Scenario from Review 2: + // Project(element_at(s, 'g')) + // Filter(element_at(s, 'f') IS NULL) + // Scan(s STRUCT NULL) + // + // Before fix: visitElementAt emitted [s, NULL] META then returned null, + // dropping field 'f'. pruneDataType removed 'f' from the struct type + // because no path referenced it. The filter still referenced + // element_at(s, 'f') IS NULL → rebuild failed. + // + // After fix: visitElementAt emits [s, NULL] META, then falls through + // with a fresh DATA context to emit [s, f] DATA, preserving 'f'. + + // s STRUCT + StructType structType = new StructType(ImmutableList.of( + new StructField("f", IntegerType.INSTANCE, false, ""), // NOT NULL + new StructField("g", IntegerType.INSTANCE, true, ""))); // nullable + SlotReference slot = new SlotReference("s", structType, true); // struct is nullable + + // SELECT element_at(s, 'g') → collector emits [s, g] DATA + ElementAt selectG = new ElementAt(slot, new org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral("g")); + Multimap paths1 = ArrayListMultimap.create(); + new AccessPathExpressionCollector( + connectContext.getStatementContext(), paths1, false, false) + .collect(selectG); + + // WHERE element_at(s, 'f') IS NULL → should emit [s, NULL] META + [s, f] DATA + ElementAt whereF = new ElementAt(slot, new org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral("f")); + IsNull isNull = new IsNull(whereF); + Multimap paths2 = ArrayListMultimap.create(); + new AccessPathExpressionCollector( + connectContext.getStatementContext(), paths2, true, false) + .collect(isNull); + + // Merge results + TreeSet allPaths = new TreeSet<>( + Comparator.comparing(CollectAccessPathResult::toString)); + allPaths.addAll(paths1.get(slot.getExprId().asInt())); + allPaths.addAll(paths2.get(slot.getExprId().asInt())); + + // [s, g] DATA must exist (from SELECT) + Assertions.assertTrue(allPaths.contains( + new CollectAccessPathResult( + ImmutableList.of("s", "g"), false, ColumnAccessPathType.DATA)), + "expected [s,g] DATA in: " + allPaths); + // [s, f] DATA must exist (NOT NULL field preserved — the fix) + // isPredicate=true because it comes from the WHERE clause. + Assertions.assertTrue(allPaths.contains( + new CollectAccessPathResult( + ImmutableList.of("s", "f"), true, ColumnAccessPathType.DATA)), + "expected [s,f] DATA (NOT NULL field preserved) in: " + allPaths); + // NOTE: [s, NULL] META is not asserted here because the manually constructed + // SlotReference has no originalColumn, so hasPhysicalNullMap returns false and + // the META NULL path is suppressed at visitSlotReference. The [s, NULL] META + // behavior is covered by testNullableFieldPreservedWithSiblingProjection. + // + // [s, f, NULL] META must NOT exist (f is NOT NULL, no field-level null map) + Assertions.assertFalse(allPaths.contains( + new CollectAccessPathResult( + ImmutableList.of("s", "f", "NULL"), true, ColumnAccessPathType.META)), + "f is NOT NULL, expected NO [s,f,NULL] META in: " + allPaths); + } }