This is an automated email from the ASF dual-hosted git repository.
mrhhsg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 9de831aeb82 [fix](be) Keep legacy struct OFFSET access path routed to
the field named offset (#67757)
9de831aeb82 is described below
commit 9de831aeb82b00e6625716e0f4711a98ca88c40f
Author: Jerry Hu <[email protected]>
AuthorDate: Wed Sep 23 21:37:26 2026 +0800
[fix](be) Keep legacy struct OFFSET access path routed to the field named
offset (#67757)
### What problem does this PR solve?
Issue Number: None
Related PR: #65805
Problem Summary:
#65805 introduced the typed DATA/META access-path protocol and keeps
decoding the legacy all-DATA
encoding from an old FE. In that legacy decoding,
`_split_access_paths()` treats any trailing
`NULL`/`OFFSET` component as current-level metadata. A Struct owns no
offset metadata, so a legacy
`[s, offset]` path, which is exactly what an old FE emits for
`element_at(s, 'offset')` on a struct
whose field is literally named `offset`, was consumed as `OFFSET_ONLY`.
`descendant_paths` became
empty, every field iterator was set to SKIP, and the query silently
returned default values.
Before #65805 that path was routed to the field named `offset`, so this
is a wrong-result regression
in the supported BE-first rolling upgrade window (old FE + new BE).
Fix:
- `_split_access_paths()` takes `owns_offset_meta`. Struct passes
`false`: a legacy `OFFSET` tail
stays a descendant path and reaches the field named `offset`, and a
typed META `OFFSET` request on
a Struct is rejected as an FE/BE contract violation instead of silently
skipping data. Scalar, Map
and Array pass `true` and keep treating `OFFSET` as current-level
metadata.
- A legacy `[s, NULL]` keeps its sentinel meaning because an old FE
really emits it for `s IS NULL`.
Upgrade notes (unchanged from #65805, documented here): BE first, then
FE. A new FE against an old
BE fails every nested `IS NULL` / `length()` / `cardinality()` query
with "path is empty" because the
old BE reads `data_access_path` from a META path.
Validation:
```
sh run-be-ut.sh --run --filter='ColumnReaderTest.*'
# [ PASSED ] 57 tests, including the new
# LegacyStructOffsetComponentRoutesToDataField /
TypedMetaOffsetPathOnStructIsRejected
./run-regression-test.sh --conf <worktree conf> --run -d
nereids_rules_p0/column_pruning -s null_column_pruning -forceGenOut
./run-regression-test.sh --conf <worktree conf> --run -d
nereids_rules_p0/column_pruning -s null_column_pruning
# All suites success, new cases !35-!39 on ncp_meta_name_tbl
(struct<`null`: string, `offset`: string>)
```
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
https://claude.ai/code/session_01L4PAEhhJm3BvMHghsT77HA
---
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 f6703586859..8bd5d0f2073 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.
@@ -1207,7 +1209,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);
@@ -1271,7 +1273,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;
@@ -1294,13 +1311,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());
}
@@ -1314,9 +1332,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);
}
@@ -2574,8 +2590,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 177099a6ec3..f2b4eec5a3a 100644
--- a/be/test/storage/segment/column_reader_test.cpp
+++ b/be/test/storage/segment/column_reader_test.cpp
@@ -78,13 +78,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);
}
@@ -956,6 +957,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]
