From 7f29b568445263d44be89b72f7947d82824eec3b Mon Sep 17 00:00:00 2001 From: englefly Date: Mon, 27 Apr 2026 15:49:14 +0800 Subject: [PATCH] [fix](fe) Disable row-store lazy fetch for shared base columns ### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Prevent topn lazy materialization from using row-store fetch when multiple lazy slots share the same base column, such as variant root/subcolumn accesses. This avoids wrong results in the non-two-phase row-store path. ### Release note None ### Check List (For Author) - Test: No need to test (waiting for manual confirmation) - Behavior changed: Yes (lazy materialization now falls back from row-store fetch to column fetch when lazy slots share a base column) - Does this need documentation: No Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../translator/PhysicalPlanTranslator.java | 20 +++++++++++--- .../physical/PhysicalLazyMaterialize.java | 4 +++ .../org/apache/doris/qe/SessionVariable.java | 2 +- .../PhysicalPlanTranslatorTest.java | 26 +++++++++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) 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 aaa1b00309f389..ddb6f67d204233 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 @@ -2804,7 +2804,7 @@ public PlanFragment visitPhysicalLazyMaterialize(PhysicalLazyMaterialize rowStoreFlags = new ArrayList<>(); for (Relation relation : materialize.getRelations()) { - rowStoreFlags.add(shouldUseRowStore(relation)); + rowStoreFlags.add(shouldUseRowStore(relation, materialize.getLazySlots(relation))); } materializeNode.setRowStoreFlags(rowStoreFlags); @@ -2817,14 +2817,28 @@ public PlanFragment visitPhysicalLazyMaterialize(PhysicalLazyMaterialize lazySlots) { + Set originalColumnUniqueIds = new HashSet<>(); + for (Slot lazySlot : lazySlots) { + if (!(lazySlot instanceof SlotReference)) { + return false; + } + Optional originalColumn = ((SlotReference) lazySlot).getOriginalColumn(); + if (!originalColumn.isPresent() || !originalColumnUniqueIds.add(originalColumn.get().getUniqueId())) { + return false; + } + } + return true; + } + + private boolean shouldUseRowStore(Relation rel, List lazySlots) { boolean useRowStore = false; if (rel instanceof PhysicalOlapScan) { OlapTable olapTable = ((PhysicalOlapScan) rel).getTable(); useRowStore = olapTable.storeRowColumn() && CollectionUtils.isEmpty(olapTable.getTableProperty().getCopiedRowStoreColumns()); } - return useRowStore; + return useRowStore && canUseRowStoreForLazySlots(lazySlots); } @Override 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 25d39cc02b1257..3a310d4ead28b9 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 @@ -294,4 +294,8 @@ public List> getLazyBaseColumnIndices() { public List getRowIds() { return rowIdList; } + + public List getLazySlots(Relation relation) { + return relationToLazySlotMap.getOrDefault(relation, ImmutableList.of()); + } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java index f0124581df76dc..1f6d2a4074b569 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java @@ -2194,7 +2194,7 @@ public boolean isEnableHboNonStrictMatchingMode() { // 1. read related rowids along with necessary column data // 2. spawn fetch RPC to other nodes to get related data by sorted rowids @VarAttrDef.VarAttr(name = ENABLE_TWO_PHASE_READ_OPT, fuzzy = true) - public boolean enableTwoPhaseReadOpt = true; + public boolean enableTwoPhaseReadOpt = false; @VarAttrDef.VarAttr(name = TOPN_OPT_LIMIT_THRESHOLD) public long topnOptLimitThreshold = 1024; @VarAttrDef.VarAttr(name = TOPN_FILTER_RATIO) diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslatorTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslatorTest.java index c12d76da8291a7..b07e6c1c1c86c5 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslatorTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslatorTest.java @@ -166,4 +166,30 @@ public void testRepeatInputOutputOrder() throws Exception { } ); } + + @Test + public void testCanUseRowStoreForLazySlots() { + Column distinctA = new Column("a", org.apache.doris.catalog.Type.INT); + distinctA.setUniqueId(1); + Column distinctB = new Column("b", org.apache.doris.catalog.Type.INT); + distinctB.setUniqueId(2); + Column sharedVariant = new Column("kv", org.apache.doris.catalog.Type.VARIANT); + sharedVariant.setUniqueId(3); + + SlotReference distinctSlotA = new SlotReference(StatementScopeIdGenerator.newExprId(), "a", + IntegerType.INSTANCE, true, ImmutableList.of(), null, distinctA, null, distinctA); + SlotReference distinctSlotB = new SlotReference(StatementScopeIdGenerator.newExprId(), "b", + IntegerType.INSTANCE, true, ImmutableList.of(), null, distinctB, null, distinctB); + SlotReference variantRootSlot = new SlotReference(StatementScopeIdGenerator.newExprId(), "kv", + org.apache.doris.nereids.types.VariantType.INSTANCE, true, ImmutableList.of(), + null, sharedVariant, null, sharedVariant); + SlotReference variantSubColumnSlot = new SlotReference(StatementScopeIdGenerator.newExprId(), "kv", + org.apache.doris.nereids.types.VariantType.INSTANCE, true, ImmutableList.of(), + null, sharedVariant, null, sharedVariant, ImmutableList.of("ssl")); + + Assertions.assertTrue(PhysicalPlanTranslator.canUseRowStoreForLazySlots( + ImmutableList.of(distinctSlotA, distinctSlotB))); + Assertions.assertFalse(PhysicalPlanTranslator.canUseRowStoreForLazySlots( + ImmutableList.of(variantRootSlot, variantSubColumnSlot))); + } }