This is an automated email from the ASF dual-hosted git repository. mrhhsg pushed a commit to branch meta-path in repository https://gitbox.apache.org/repos/asf/doris.git
commit 1f8b01ab46e6e38cfae7356cd3520d890cfb9d72 Author: Hu Shenggang <[email protected]> AuthorDate: Thu Sep 10 09:48:25 2026 +0800 [fix](be) Keep legacy struct OFFSET path routed to the field named offset An old FE emits `element_at(s, 'offset')` as the legacy DATA path [s, offset]. Struct owns no offset metadata, so the new _split_access_paths() must not consume that tail as OFFSET_ONLY; doing so left descendant_paths empty, skipped every field and silently returned defaults during the BE-first rolling upgrade window. Pass owns_offset_meta into _split_access_paths(): Struct routes a legacy OFFSET tail to the field named "offset" exactly as before, and rejects a typed META OFFSET request as an FE/BE contract violation. Scalar, Map and Array keep treating OFFSET as current-level metadata. Add BE unit tests for the legacy routing and the typed rejection, plus a data-level regression case that projects and filters struct fields named `null` and `offset` with nested pruning enabled. Claude-Session: https://claude.ai/code/session_01PmLCbS1cGD5pe5vQdtBG5U --- be/src/storage/segment/column_reader.cpp | 39 ++++++++--- be/src/storage/segment/column_reader.h | 6 +- be/test/storage/segment/column_reader_test.cpp | 79 +++++++++++++++++++++- .../column_pruning/null_column_pruning.out | 25 +++++++ .../column_pruning/null_column_pruning.groovy | 42 ++++++++++++ 5 files changed, 177 insertions(+), 14 deletions(-) diff --git a/be/src/storage/segment/column_reader.cpp b/be/src/storage/segment/column_reader.cpp index 765b79324ab..ce927de2229 100644 --- a/be/src/storage/segment/column_reader.cpp +++ b/be/src/storage/segment/column_reader.cpp @@ -114,7 +114,9 @@ namespace { // 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. +// descendant routing and mark every data child SKIP. A legacy DATA path whose tail is OFFSET at +// a Struct level is not metadata but a field literally named "offset"; it keeps the pre-typed +// routing to that field, while a typed META OFFSET request on a Struct is rejected. // 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. @@ -1210,7 +1212,7 @@ void ColumnIterator::_recovery_from_place_holder_column(MutableColumnPtr& dst) { } Result<ColumnIterator::AccessPathSplit> ColumnIterator::_split_access_paths( - TColumnAccessPaths access_paths) const { + TColumnAccessPaths access_paths, bool owns_offset_meta) const { AccessPathSplit split; for (auto& path : access_paths) { const bool uses_legacy_encoding = uses_legacy_access_path_encoding(path); @@ -1274,7 +1276,22 @@ Result<ColumnIterator::AccessPathSplit> ColumnIterator::_split_access_paths( 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)) { + const bool is_offset = StringCaseEqual()((*components)[0], ACCESS_OFFSET); + if (is_offset && !owns_offset_meta) { + if (uses_legacy_encoding) { + // A legacy sender never requests offsets from an iterator that has none, so a + // trailing OFFSET on a Struct can only name a data field literally called + // "offset". Keep the pre-typed routing and forward it to that field instead of + // consuming it as metadata, which would silently skip every field. + split.descendant_paths.emplace_back(std::move(path)); + continue; + } + return ResultError(Status::InternalError( + "Invalid META access path for column '{}': OFFSET metadata is not " + "supported at this level", + _column_name)); + } + if (is_offset) { split.current_meta_mode = MetaReadMode::OFFSET_ONLY; } else if (split.current_meta_mode == MetaReadMode::DEFAULT) { split.current_meta_mode = MetaReadMode::NULL_MAP_ONLY; @@ -1297,13 +1314,14 @@ Result<ColumnIterator::NestedAccessPathPlan> ColumnIterator::_prepare_nested_acc } NestedAccessPathPlan plan; - auto all_split = _split_access_paths(all_access_paths); + const bool owns_offset_meta = meta_support == NestedMetaSupport::NULL_MAP_AND_OFFSET; + auto all_split = _split_access_paths(all_access_paths, owns_offset_meta); 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); + auto predicate_split = _split_access_paths(predicate_access_paths, owns_offset_meta); if (!predicate_split.has_value()) { return ResultError(std::move(predicate_split).error()); } @@ -1317,9 +1335,7 @@ Result<ColumnIterator::NestedAccessPathPlan> ColumnIterator::_prepare_nested_acc 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()); + plan.skip_data_descendants = read_null_map_only() || read_offset_only(); if (plan.skip_data_descendants) { set_all_data_descendants_read_requirement(ReadRequirement::SKIP); } @@ -2577,8 +2593,11 @@ Status FileColumnIterator::set_access_paths(const TColumnAccessPaths& all_access 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)); + // Scalar iterators have no data children, so a NULL/OFFSET tail is always current-level + // metadata regardless of the encoding. + auto all_split = DORIS_TRY(_split_access_paths(all_access_paths, /*owns_offset_meta=*/true)); + auto predicate_split = + DORIS_TRY(_split_access_paths(predicate_access_paths, /*owns_offset_meta=*/true)); if (all_split.reads_current_data) { set_lazy_output_requirement(); } diff --git a/be/src/storage/segment/column_reader.h b/be/src/storage/segment/column_reader.h index 33f10df173e..f350b470c9c 100644 --- a/be/src/storage/segment/column_reader.h +++ b/be/src/storage/segment/column_reader.h @@ -542,7 +542,11 @@ protected: // 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<AccessPathSplit> _split_access_paths(TColumnAccessPaths access_paths) const; + // owns_offset_meta tells whether this iterator has current-level offsets. When it does not + // (Struct), a legacy OFFSET tail is a data field named "offset" and stays a descendant path, + // and a typed META OFFSET request is an FE/BE contract violation. + Result<AccessPathSplit> _split_access_paths(TColumnAccessPaths access_paths, + bool owns_offset_meta) const; ColumnIteratorOptions _opts; ReadRequirement _read_requirement {ReadRequirement::NORMAL}; diff --git a/be/test/storage/segment/column_reader_test.cpp b/be/test/storage/segment/column_reader_test.cpp index fe6c6961272..f5b78b05fe8 100644 --- a/be/test/storage/segment/column_reader_test.cpp +++ b/be/test/storage/segment/column_reader_test.cpp @@ -76,13 +76,14 @@ public: using ColumnIterator::AccessPathSplit; - Result<AccessPathSplit> split_access_paths(const TColumnAccessPaths& access_paths) const { - return _split_access_paths(access_paths); + Result<AccessPathSplit> split_access_paths(const TColumnAccessPaths& access_paths, + bool owns_offset_meta = true) const { + return _split_access_paths(access_paths, owns_offset_meta); } 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)); + auto split = DORIS_TRY(_split_access_paths(access_paths, /*owns_offset_meta=*/true)); return _check_and_set_meta_read_mode(requirement_before_access_path, split); } @@ -954,6 +955,78 @@ TEST_F(ColumnReaderTest, LegacyStructMetaComponentsRemainSentinels) { ColumnIterator::ReadRequirement::SKIP); } +TEST_F(ColumnReaderTest, LegacyStructOffsetComponentRoutesToDataField) { + // An old FE emits `element_at(s, 'OFFSET')` as the legacy DATA path [s, offset] (struct field + // names are lowercased). Struct has no offsets, so the component must keep naming the field + // instead of being consumed as OFFSET_ONLY metadata that skips every field. + auto make_struct_iterator = [](TrackingColumnIterator** offset_field, + TrackingColumnIterator** other_field) { + std::vector<ColumnIteratorUPtr> sub_iterators; + auto offset_field_iterator = std::make_unique<TrackingColumnIterator>(); + offset_field_iterator->set_column_name("offset"); + *offset_field = offset_field_iterator.get(); + sub_iterators.emplace_back(std::move(offset_field_iterator)); + auto other_field_iterator = std::make_unique<TrackingColumnIterator>(); + other_field_iterator->set_column_name("other"); + *other_field = other_field_iterator.get(); + sub_iterators.emplace_back(std::move(other_field_iterator)); + auto struct_iterator = std::make_unique<StructFileColumnIterator>( + create_test_reader(), nullptr, std::move(sub_iterators)); + struct_iterator->set_column_name("s"); + return struct_iterator; + }; + + for (const bool explicit_legacy_version : {false, true}) { + SCOPED_TRACE(explicit_legacy_version ? "explicit-version-0" : "missing-version"); + TrackingColumnIterator* offset_field = nullptr; + TrackingColumnIterator* other_field = nullptr; + auto struct_iterator = make_struct_iterator(&offset_field, &other_field); + auto legacy_path = create_legacy_data_access_path({"s", "offset"}); + if (explicit_legacy_version) { + legacy_path.__set_version(g_Descriptors_constants.TCOLUMN_ACCESS_PATH_VERSION_LEGACY); + } + + auto st = struct_iterator->set_access_paths({legacy_path}, {}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_FALSE(struct_iterator->read_offset_only()); + EXPECT_FALSE(struct_iterator->read_null_map_only()); + ASSERT_EQ(offset_field->routed_all_access_paths.size(), 1); + EXPECT_TRUE(offset_field->routed_predicate_access_paths.empty()); + EXPECT_EQ(offset_field->read_requirement(), ColumnIterator::ReadRequirement::LAZY_OUTPUT); + EXPECT_EQ(other_field->read_requirement(), ColumnIterator::ReadRequirement::SKIP); + } + + // The same legacy path used as a predicate keeps the field readable in the predicate phase. + TrackingColumnIterator* offset_field = nullptr; + TrackingColumnIterator* other_field = nullptr; + auto struct_iterator = make_struct_iterator(&offset_field, &other_field); + auto legacy_path = create_legacy_data_access_path({"s", ColumnIterator::ACCESS_OFFSET}); + auto st = struct_iterator->set_access_paths({legacy_path}, {legacy_path}); + ASSERT_TRUE(st.ok()) << st.to_string(); + EXPECT_FALSE(struct_iterator->read_offset_only()); + ASSERT_EQ(offset_field->routed_all_access_paths.size(), 1); + ASSERT_EQ(offset_field->routed_predicate_access_paths.size(), 1); + EXPECT_EQ(offset_field->read_requirement(), ColumnIterator::ReadRequirement::PREDICATE); + EXPECT_EQ(other_field->read_requirement(), ColumnIterator::ReadRequirement::SKIP); +} + +TEST_F(ColumnReaderTest, TypedMetaOffsetPathOnStructIsRejected) { + std::vector<ColumnIteratorUPtr> sub_iterators; + auto field_iterator = std::make_unique<FileColumnIterator>(create_test_reader()); + field_iterator->set_column_name("offset"); + sub_iterators.emplace_back(std::move(field_iterator)); + StructFileColumnIterator struct_iterator(create_test_reader(), nullptr, + std::move(sub_iterators)); + struct_iterator.set_column_name("s"); + + TColumnAccessPaths meta_path {create_meta_access_path({"s", ColumnIterator::ACCESS_OFFSET})}; + auto st = struct_iterator.set_access_paths(meta_path, {}); + ASSERT_FALSE(st.ok()); + EXPECT_TRUE(st.is<ErrorCode::INTERNAL_ERROR>()) << st.to_string(); + EXPECT_NE(st.to_string().find("OFFSET metadata is not supported"), std::string::npos) + << st.to_string(); +} + TEST_F(ColumnReaderTest, PlaceHolderLifecycleInLazyMode) { TestColumnIterator iterator; iterator.force_set_read_requirement(ColumnIterator::ReadRequirement::LAZY_OUTPUT); diff --git a/regression-test/data/nereids_rules_p0/column_pruning/null_column_pruning.out b/regression-test/data/nereids_rules_p0/column_pruning/null_column_pruning.out index 5e18f989b1a..0e98f899e22 100644 --- a/regression-test/data/nereids_rules_p0/column_pruning/null_column_pruning.out +++ b/regression-test/data/nereids_rules_p0/column_pruning/null_column_pruning.out @@ -105,3 +105,28 @@ -- !34 -- +-- !35 -- +1 4 +2 13 +3 \N +4 \N + +-- !36 -- +1 n1 off1 +2 \N longer_offset +3 n3 \N +4 \N \N + +-- !37 -- +2 +4 + +-- !38 -- +1 +2 + +-- !39 -- +1 false off1 +2 false longer_offset +3 false \N + 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 e4ddfecf338..baa64b05311 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 @@ -524,4 +524,46 @@ suite("null_column_pruning") { } order_qt_34 "select 1 from ncp_tbl where length(str_col) = 0 or str_col is null"; + + // ─── Struct fields literally named `null` / `offset` ─────────────────────── + // The field names collide with the NULL/OFFSET metadata components. The typed + // DATA/META protocol must keep routing them as data fields, so projections and + // predicates on these fields return real data instead of pruned defaults. + sql """ DROP TABLE IF EXISTS ncp_meta_name_tbl """ + sql """ + CREATE TABLE ncp_meta_name_tbl ( + id INT, + s STRUCT<`null`: STRING, `offset`: STRING> NULL + ) ENGINE = OLAP + DUPLICATE KEY(id) + DISTRIBUTED BY HASH(id) BUCKETS 1 + PROPERTIES ("replication_allocation" = "tag.location.default: 1") + """ + sql """ + INSERT INTO ncp_meta_name_tbl VALUES + (1, named_struct('null', 'n1', 'offset', 'off1')), + (2, named_struct('null', null, 'offset', 'longer_offset')), + (3, named_struct('null', 'n3', 'offset', null)), + (4, null) + """ + + explain { + sql "select length(element_at(s, 'offset')) from ncp_meta_name_tbl" + contains "nested columns" + contains "s.offset.OFFSET" + } + order_qt_35 "select id, length(element_at(s, 'offset')) from ncp_meta_name_tbl" + + order_qt_36 "select id, element_at(s, 'null'), element_at(s, 'offset') from ncp_meta_name_tbl" + + explain { + sql "select 1 from ncp_meta_name_tbl where element_at(s, 'null') is null" + contains "nested columns" + contains "s.null.NULL" + } + order_qt_37 "select id from ncp_meta_name_tbl where element_at(s, 'null') is null" + + order_qt_38 "select id from ncp_meta_name_tbl where element_at(s, 'offset') is not null" + + order_qt_39 "select id, s is null, element_at(s, 'offset') from ncp_meta_name_tbl where s is not null" } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
