This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 908e484f018 [fix](be) Preserve nested paths for lazy rowid fetch
(#67205)
908e484f018 is described below
commit 908e484f01870a18a0e5f0ba0ab87b519860a319
Author: Jerry Hu <[email protected]>
AuthorDate: Fri Aug 28 21:01:42 2026 +0800
[fix](be) Preserve nested paths for lazy rowid fetch (#67205)
### What problem does this PR solve?
Issue Number: None
Related PR: #64242
Problem Summary:
TopN lazy materialization can fetch a nested-pruned column by row ID
after the scan has reduced its child layout. On branch-4.1, the
materialization probe may remap to a slot without the relation's
access-path metadata, and the row-store fetch path cannot honor nested
access paths. That can make the storage iterator layout disagree with
the pruned result column layout.
This backport:
- preserves relation access paths through the lazy-materialization
output;
- routes nested lazy slots away from row-store fetch;
- applies the slot access paths before branch-4.1's per-row
`seek_and_read_by_rowid` calls;
- keeps the parent struct iterator readable while skipped child
iterators are marked individually.
The FE output-slot propagation is included because master already had
that prerequisite when #64242 merged, while branch-4.1 does not.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- `./run-fe-ut.sh --run
org.apache.doris.nereids.processor.post.materialize.MaterializeProbeVisitorTest`
(5 tests passed)
- [x] Manual test (add detailed scripts or steps below)
- Executed the nested STRUCT/MAP TopN lazy-rowid reproduction on the
branch-4.1 adapted backport with
`topn_lazy_materialization_threshold=1024`.
- The query returned the expected primary keys `25, 21, 17, 13, 9`, and
the BE remained alive.
- [ ] 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? -->
A full current-head BE rebuild was attempted but was blocked by stale
local Lance C++ third-party headers (`LanceScanStatistics` is missing
from the installed headers). It is not counted as validation; CI is
still required.
- Behavior changed:
- [ ] No.
- [x] Yes. Nested-pruned lazy columns use the normal storage rowid fetch
path instead of row-store fetch.
- 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 -->
---
be/src/exec/rowid_fetcher.cpp | 24 ++++
be/src/storage/segment/column_reader.cpp | 1 -
.../glue/translator/PhysicalPlanTranslator.java | 27 ++++-
.../post/materialize/MaterializeProbeVisitor.java | 20 +++-
.../plans/physical/PhysicalLazyMaterialize.java | 13 ++-
.../materialize/MaterializeProbeVisitorTest.java | 128 +++++++++++++++++++++
6 files changed, 205 insertions(+), 8 deletions(-)
diff --git a/be/src/exec/rowid_fetcher.cpp b/be/src/exec/rowid_fetcher.cpp
index a8c3689d578..61c5a529d73 100644
--- a/be/src/exec/rowid_fetcher.cpp
+++ b/be/src/exec/rowid_fetcher.cpp
@@ -346,6 +346,28 @@ struct IteratorItem {
StorageReadOptions storage_read_options;
};
+static void set_slot_access_paths(const SlotDescriptor& slot, const
TabletSchema& schema,
+ StorageReadOptions& storage_read_options) {
+ int32_t unique_id = slot.col_unique_id();
+ const int field_index =
+ unique_id >= 0 ? schema.field_index(unique_id) :
schema.field_index(slot.col_name());
+ if (field_index >= 0) {
+ const auto& column = schema.column(field_index);
+ unique_id = column.unique_id() >= 0 ? column.unique_id() :
column.parent_unique_id();
+ }
+ if (unique_id < 0) {
+ return;
+ }
+
+ if (!slot.all_access_paths().empty()) {
+ storage_read_options.all_access_paths[unique_id] =
slot.all_access_paths();
+ }
+
+ if (!slot.predicate_access_paths().empty()) {
+ storage_read_options.predicate_access_paths[unique_id] =
slot.predicate_access_paths();
+ }
+}
+
struct SegItem {
BaseTabletSPtr tablet;
BetaRowsetSharedPtr rowset;
@@ -473,6 +495,7 @@ Status RowIdStorageReader::read_by_rowids(const
PMultiGetRequest& request,
iterator_item.storage_read_options.io_ctx.reader_type =
ReaderType::READER_QUERY;
}
segment = iterator_item.segment;
+ set_slot_access_paths(slots[x], full_read_schema,
iterator_item.storage_read_options);
RETURN_IF_ERROR(segment->seek_and_read_by_rowid(
full_read_schema, &slots[x], row_id, column,
iterator_item.storage_read_options,
iterator_item.iterator));
@@ -1283,6 +1306,7 @@ Status RowIdStorageReader::read_doris_format_row(
iterator_item.storage_read_options.io_ctx.file_cache_miss_policy =
file_cache_miss_policy;
}
+ set_slot_access_paths(slots[x], full_read_schema,
iterator_item.storage_read_options);
for (auto row_id : row_ids) {
RETURN_IF_ERROR(segment->seek_and_read_by_rowid(
full_read_schema, &slots[x], row_id, column,
diff --git a/be/src/storage/segment/column_reader.cpp
b/be/src/storage/segment/column_reader.cpp
index 1a4c42cc7bf..71c0eae2546 100644
--- a/be/src/storage/segment/column_reader.cpp
+++ b/be/src/storage/segment/column_reader.cpp
@@ -1491,7 +1491,6 @@ Status StructFileColumnIterator::set_access_paths(
}
if (!need_to_read) {
- set_reading_flag(ReadingFlag::SKIP_READING);
sub_iterator->set_reading_flag(ReadingFlag::SKIP_READING);
DLOG(INFO) << "Struct column iterator set sub-column " << name <<
" to SKIP_READING";
continue;
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
index 214f4badf69..4b63cc08636 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
@@ -2968,7 +2968,32 @@ public class PhysicalPlanTranslator extends
DefaultPlanVisitor<PlanFragment, Pla
useRowStore = olapTable.storeRowColumn()
&&
CollectionUtils.isEmpty(olapTable.getTableProperty().getCopiedRowStoreColumns());
}
- return useRowStore && canUseRowStoreForLazySlots(lazySlots);
+ return useRowStore && canUseRowStoreForLazySlots(lazySlots)
+ && !hasNestedAccessPaths(rel, lazySlots);
+ }
+
+ private boolean hasNestedAccessPaths(Relation rel, List<Slot> lazySlots) {
+ Set<Integer> lazyColumnUniqueIds = new HashSet<>();
+ for (Slot lazySlot : lazySlots) {
+ SlotReference slotReference = (SlotReference) lazySlot;
+
lazyColumnUniqueIds.add(slotReference.getOriginalColumn().get().getUniqueId());
+ }
+ for (Slot outputSlot : rel.getOutput()) {
+ if (outputSlot instanceof SlotReference) {
+ SlotReference slotReference = (SlotReference) outputSlot;
+ if (slotReference.getOriginalColumn().isPresent()
+ &&
lazyColumnUniqueIds.contains(slotReference.getOriginalColumn().get().getUniqueId())
+ && hasNestedAccessPaths(slotReference)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private boolean hasNestedAccessPaths(SlotReference slotReference) {
+ return slotReference.getAllAccessPaths().map(paths ->
!paths.isEmpty()).orElse(false)
+ || slotReference.getPredicateAccessPaths().map(paths ->
!paths.isEmpty()).orElse(false);
}
@Override
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/materialize/MaterializeProbeVisitor.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/materialize/MaterializeProbeVisitor.java
index 8336105a004..feed5e456ac 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/materialize/MaterializeProbeVisitor.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/materialize/MaterializeProbeVisitor.java
@@ -85,7 +85,9 @@ public class MaterializeProbeVisitor extends
DefaultPlanVisitor<Optional<Materia
return Optional.empty();
}
if (filter.getInputSlots().contains(context.slot)) {
- return Optional.of(new MaterializeSource((Relation)
filter.child(), context.slot));
+ Relation relation = (Relation) filter.child();
+ return Optional.of(new MaterializeSource(
+ relation, findRelationOutputSlot(relation,
context.slot).orElse(context.slot)));
} else {
return filter.child().accept(this, context);
}
@@ -161,7 +163,8 @@ public class MaterializeProbeVisitor extends
DefaultPlanVisitor<Optional<Materia
if (scan.getOperativeSlots().contains(context.slot)) {
return Optional.empty();
}
- return Optional.of(new MaterializeSource(scan, context.slot));
+ return Optional.of(
+ new MaterializeSource(scan, findRelationOutputSlot(scan,
context.slot).orElse(context.slot)));
}
@Override
@@ -172,7 +175,8 @@ public class MaterializeProbeVisitor extends
DefaultPlanVisitor<Optional<Materia
&& !relation.getOperativeSlots().contains(context.slot)) {
// lazy materialize slot must be a passive slot
if (context.slot.getOriginalColumn().isPresent()) {
- return Optional.of(new MaterializeSource(relation,
context.slot));
+ return Optional.of(new MaterializeSource(
+ relation, findRelationOutputSlot(relation,
context.slot).orElse(context.slot)));
} else {
LOG.info("lazy materialize {} failed, because its column is
empty", context.slot);
}
@@ -192,7 +196,8 @@ public class MaterializeProbeVisitor extends
DefaultPlanVisitor<Optional<Materia
&& !tvfRelation.getOperativeSlots().contains(context.slot)) {
// lazy materialize slot must be a passive slot
if (context.slot.getOriginalColumn().isPresent()) {
- return Optional.of(new MaterializeSource(tvfRelation,
context.slot));
+ return Optional.of(new MaterializeSource(
+ tvfRelation, findRelationOutputSlot(tvfRelation,
context.slot).orElse(context.slot)));
} else {
LOG.info("lazy materialize {} failed, because its column is
empty", context.slot);
}
@@ -239,4 +244,11 @@ public class MaterializeProbeVisitor extends
DefaultPlanVisitor<Optional<Materia
}
}
+ private Optional<SlotReference> findRelationOutputSlot(Relation relation,
SlotReference contextSlot) {
+ return relation.getOutput().stream()
+ .filter(slot -> slot instanceof SlotReference &&
slot.equals(contextSlot))
+ .map(slot -> (SlotReference) slot)
+ .findFirst();
+ }
+
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalLazyMaterialize.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalLazyMaterialize.java
index 51a8bf054f5..bf9cf215960 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalLazyMaterialize.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalLazyMaterialize.java
@@ -167,8 +167,17 @@ public class PhysicalLazyMaterialize<CHILD_TYPE extends
Plan> extends PhysicalUn
// Set originalColumn on the lazy slot so that createSlotDesc
can write
// colUniqueId into the thrift SlotDescriptor — BE needs it to
resolve
// the column during remote fetch.
- Column originalColumn =
materializeMap.get(lazySlot).baseSlot.getOriginalColumn().get();
- outputBuilder.add(((SlotReference)
lazySlot).withColumn(originalColumn));
+ SlotReference baseSlot = materializeMap.get(lazySlot).baseSlot;
+ Column originalColumn = baseSlot.getOriginalColumn().get();
+ SlotReference outputSlot = ((SlotReference)
lazySlot).withColumn(originalColumn);
+ if (baseSlot.getAllAccessPaths().isPresent()) {
+ outputSlot = outputSlot.withAccessPaths(
+ baseSlot.getAllAccessPaths().get(),
+
baseSlot.getPredicateAccessPaths().orElse(ImmutableList.of()),
+
baseSlot.getDisplayAllAccessPaths().orElse(ImmutableList.of()),
+
baseSlot.getDisplayPredicateAccessPaths().orElse(ImmutableList.of()));
+ }
+ outputBuilder.add(outputSlot);
lazyColumnForRel.add(originalColumn);
lazyBaseColumnIdxForRel.add(relationTable.getBaseColumnIdxByName(originalColumn.getName()));
lazySlotLocationForRel.add(loc);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/processor/post/materialize/MaterializeProbeVisitorTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/processor/post/materialize/MaterializeProbeVisitorTest.java
index de143cea0a3..94d24536693 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/processor/post/materialize/MaterializeProbeVisitorTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/processor/post/materialize/MaterializeProbeVisitorTest.java
@@ -17,15 +17,38 @@
package org.apache.doris.nereids.processor.post.materialize;
+import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.KeysType;
+import org.apache.doris.catalog.OlapTable;
+import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.functions.table.VectorSearch;
+import org.apache.doris.nereids.trees.plans.algebra.Relation;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalFilter;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalLazyMaterialize;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalOlapScan;
import org.apache.doris.nereids.trees.plans.physical.PhysicalTVFRelation;
+import org.apache.doris.nereids.types.IntegerType;
+import org.apache.doris.qe.ConnectContext;
import org.apache.doris.tablefunction.VectorSearchTableValuedFunction;
+import org.apache.doris.thrift.TAccessPathType;
+import org.apache.doris.thrift.TColumnAccessPath;
+import org.apache.doris.thrift.TDataAccessPath;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
+import java.util.BitSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
class MaterializeProbeVisitorTest {
@Test
@@ -47,6 +70,111 @@ class MaterializeProbeVisitorTest {
relation, new
MaterializeProbeVisitor.ProbeContext(nestedSlot)).isPresent());
}
+ @Test
+ void testOlapScanUsesRelationSlotWithAccessPaths() {
+ SlotReference contextSlot = new SlotReference("a",
IntegerType.INSTANCE);
+ SlotReference relationSlot = contextSlot.withAccessPaths(
+ ImmutableList.of(dataPath("nested")), ImmutableList.of());
+ contextSlot = (SlotReference) contextSlot.withNullable(false);
+ PhysicalOlapScan scan = mockBaseOlapScan(relationSlot);
+
+ MaterializeProbeVisitor.ProbeContext context = new
MaterializeProbeVisitor.ProbeContext(contextSlot);
+ Optional<MaterializeSource> source = new
MaterializeProbeVisitor().visitPhysicalOlapScan(scan, context);
+
+ Assertions.assertTrue(source.isPresent());
+ Assertions.assertSame(relationSlot, source.get().baseSlot);
+ Assertions.assertEquals(relationSlot.getAllAccessPaths(),
source.get().baseSlot.getAllAccessPaths());
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ void testFilterUsingIndexUsesRelationSlotWithAccessPaths() {
+ ConnectContext oldContext = ConnectContext.get();
+ ConnectContext context = new ConnectContext();
+ context.getSessionVariable().topNLazyMaterializationUsingIndex = true;
+ context.setThreadLocalInfo();
+ try {
+ SlotReference contextSlot = new SlotReference("a",
IntegerType.INSTANCE);
+ SlotReference relationSlot = contextSlot.withAccessPaths(
+ ImmutableList.of(dataPath("nested")), ImmutableList.of());
+ contextSlot = (SlotReference) contextSlot.withNullable(false);
+ PhysicalOlapScan scan = mockBaseOlapScan(relationSlot);
+
+ PhysicalFilter<PhysicalOlapScan> filter =
Mockito.mock(PhysicalFilter.class);
+ Mockito.when(filter.child()).thenReturn(scan);
+
Mockito.when(filter.getInputSlots()).thenReturn(ImmutableSet.of(contextSlot));
+
+ MaterializeProbeVisitor.ProbeContext probeContext = new
MaterializeProbeVisitor.ProbeContext(contextSlot);
+ Optional<MaterializeSource> source =
+ new MaterializeProbeVisitor().visitPhysicalFilter(filter,
probeContext);
+
+ Assertions.assertTrue(source.isPresent());
+ Assertions.assertSame(relationSlot, source.get().baseSlot);
+ Assertions.assertEquals(relationSlot.getAllAccessPaths(),
source.get().baseSlot.getAllAccessPaths());
+ } finally {
+ if (oldContext == null) {
+ ConnectContext.remove();
+ } else {
+ oldContext.setThreadLocalInfo();
+ }
+ }
+ }
+
+ @Test
+ void testLazyMaterializeOutputKeepsBaseSlotAccessPaths() {
+ Column column = Mockito.mock(Column.class);
+ Mockito.when(column.getName()).thenReturn("a");
+ OlapTable table = Mockito.mock(OlapTable.class);
+ Mockito.when(table.getBaseColumnIdxByName("a")).thenReturn(0);
+ PhysicalOlapScan relation = Mockito.mock(PhysicalOlapScan.class);
+ Mockito.when(relation.getTable()).thenReturn(table);
+ Mockito.when(relation.getAllChildrenTypes()).thenReturn(new BitSet());
+
+ List<TColumnAccessPath> allPaths = ImmutableList.of(dataPath("all"));
+ List<TColumnAccessPath> predicatePaths =
ImmutableList.of(dataPath("predicate"));
+ List<TColumnAccessPath> displayAllPaths =
ImmutableList.of(dataPath("display_all"));
+ List<TColumnAccessPath> displayPredicatePaths =
ImmutableList.of(dataPath("display_predicate"));
+ SlotReference baseSlot = new SlotReference("a", IntegerType.INSTANCE)
+ .withColumn(column)
+ .withAccessPaths(allPaths, predicatePaths, displayAllPaths,
displayPredicatePaths);
+ SlotReference lazySlot = new SlotReference("a", IntegerType.INSTANCE);
+ SlotReference rowId = new SlotReference("__DORIS_ROWID_COL__",
IntegerType.INSTANCE);
+
+ BiMap<Relation, SlotReference> relationToRowId = HashBiMap.create();
+ relationToRowId.put(relation, rowId);
+ Map<Relation, List<Slot>> relationToLazySlotMap = ImmutableMap.of(
+ relation, ImmutableList.<Slot>of(lazySlot));
+ Map<Slot, MaterializeSource> materializeMap = ImmutableMap.of(
+ lazySlot, new MaterializeSource(relation, baseSlot));
+ PhysicalLazyMaterialize<PhysicalOlapScan> materialize = new
PhysicalLazyMaterialize<>(
+ relation, ImmutableList.of(rowId), ImmutableList.of(),
relationToLazySlotMap,
+ relationToRowId, materializeMap);
+
+ SlotReference outputSlot = (SlotReference)
materialize.getOutput().get(0);
+ Assertions.assertEquals(Optional.of(allPaths),
outputSlot.getAllAccessPaths());
+ Assertions.assertEquals(Optional.of(predicatePaths),
outputSlot.getPredicateAccessPaths());
+ Assertions.assertEquals(Optional.of(displayAllPaths),
outputSlot.getDisplayAllAccessPaths());
+ Assertions.assertEquals(Optional.of(displayPredicatePaths),
outputSlot.getDisplayPredicateAccessPaths());
+ }
+
+ private TColumnAccessPath dataPath(String... path) {
+ TColumnAccessPath accessPath = new
TColumnAccessPath(TAccessPathType.DATA);
+ accessPath.data_access_path = new
TDataAccessPath(ImmutableList.copyOf(path));
+ return accessPath;
+ }
+
+ private PhysicalOlapScan mockBaseOlapScan(SlotReference outputSlot) {
+ OlapTable table = Mockito.mock(OlapTable.class);
+ Mockito.when(table.getBaseIndexId()).thenReturn(1L);
+ Mockito.when(table.getKeysType()).thenReturn(KeysType.DUP_KEYS);
+ PhysicalOlapScan scan = Mockito.mock(PhysicalOlapScan.class);
+ Mockito.when(scan.getSelectedIndexId()).thenReturn(1L);
+ Mockito.when(scan.getTable()).thenReturn(table);
+
Mockito.when(scan.getOutput()).thenReturn(ImmutableList.of(outputSlot));
+ Mockito.when(scan.getOperativeSlots()).thenReturn(ImmutableList.of());
+ return scan;
+ }
+
private PhysicalTVFRelation mockVectorSearchRelation() {
PhysicalTVFRelation relation = Mockito.mock(PhysicalTVFRelation.class);
VectorSearch function = Mockito.mock(VectorSearch.class);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
