From f21bdd45fce24c7a0e681633ebede16af35899be Mon Sep 17 00:00:00 2001 From: yujun Date: Fri, 10 Jul 2026 21:41:33 +0800 Subject: [PATCH 1/2] [fix](fe) Reject mismatched project outputs in PullUpJoinFromUnionAll Avoid comparing and remapping join inputs as equivalent when the common-side projects expose different output arity. This keeps the union pull-up rewrite aligned with the actual plan shape and adds focused coverage for the comparator and rewrite guard. Key changes: - Add an output-size guard before comparing plan nodes in PullUpJoinFromUnionAll. - Add a FE unit test that covers both the comparator and the rewrite path. Unit Test: - PullUpJoinFromUnionAllTest --- .../rules/rewrite/PullUpJoinFromUnionAll.java | 6 +- .../rewrite/PullUpJoinFromUnionAllTest.java | 137 ++++++++++++++++++ 2 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAllTest.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAll.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAll.java index f7197516011bc7..86ac3a8691d71d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAll.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAll.java @@ -577,14 +577,14 @@ boolean isNotSupported(Plan plan) { } boolean comparePlan(Plan plan1, Plan plan2) { + if (plan1.getOutput().size() != plan2.getOutput().size()) { + return false; + } boolean isEqual = true; if (plan1 instanceof LogicalCatalogRelation && plan2 instanceof LogicalCatalogRelation) { isEqual = new TableIdentifier(((LogicalCatalogRelation) plan1).getTable()) .equals(new TableIdentifier(((LogicalCatalogRelation) plan2).getTable())); } else if (plan1 instanceof LogicalProject && plan2 instanceof LogicalProject) { - if (plan1.getOutput().size() != plan2.getOutput().size()) { - isEqual = false; - } for (int i = 0; isEqual && i < plan2.getOutput().size(); i++) { Expression expr1 = ((LogicalProject) plan1).getProjects().get(i); Expression expr2 = ((LogicalProject) plan2).getProjects().get(i); diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAllTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAllTest.java new file mode 100644 index 00000000000000..244aef7c80d536 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAllTest.java @@ -0,0 +1,137 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.nereids.rules.rewrite; + +import org.apache.doris.nereids.trees.expressions.Alias; +import org.apache.doris.nereids.trees.expressions.EqualTo; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.NamedExpression; +import org.apache.doris.nereids.trees.expressions.Slot; +import org.apache.doris.nereids.trees.expressions.SlotReference; +import org.apache.doris.nereids.trees.plans.JoinType; +import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.algebra.SetOperation.Qualifier; +import org.apache.doris.nereids.trees.plans.logical.LogicalJoin; +import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan; +import org.apache.doris.nereids.trees.plans.logical.LogicalProject; +import org.apache.doris.nereids.trees.plans.logical.LogicalUnion; +import org.apache.doris.nereids.util.MemoTestUtils; +import org.apache.doris.nereids.util.PlanChecker; +import org.apache.doris.nereids.util.PlanConstructor; + +import com.google.common.collect.ImmutableList; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.List; + +class PullUpJoinFromUnionAllTest { + + @Test + void comparatorRejectsDifferentProjectOutputSizes() { + LogicalOlapScan smallScan = newScan(1, "common_small"); + LogicalOlapScan largeScan = newScan(1, "common_large"); + LogicalProject smallProject = project(selectSlots(smallScan.getOutput(), 0), smallScan); + LogicalProject largeProject = project(selectSlots(largeScan.getOutput(), 0, 1), largeScan); + + PullUpJoinFromUnionAll.LogicalPlanComparator comparator = + new PullUpJoinFromUnionAll().new LogicalPlanComparator(); + Assertions.assertFalse(comparator.isLogicalEqual(largeProject, smallProject)); + } + + @Test + void ruleSkipsJoinChildrenWithDifferentCommonSideOutputs() { + LogicalOlapScan commonSmallScan = newScan(10, "common_small"); + LogicalOlapScan commonLargeScan = newScan(10, "common_large"); + LogicalProject commonSmall = project(selectSlots(commonSmallScan.getOutput(), 0), + commonSmallScan); + LogicalProject commonLarge = project(selectSlots(commonLargeScan.getOutput(), 0, 1), + commonLargeScan); + LogicalOlapScan otherLeft = newScan(20, "other_left"); + LogicalOlapScan otherRight = newScan(30, "other_right"); + + LogicalProject unionChild1 = outputProject( + join(commonSmall, otherLeft), + "x", commonSmall.getOutput().get(0), + "y", otherLeft.getOutput().get(1)); + LogicalProject unionChild2 = outputProject( + join(commonLarge, otherRight), + "x", commonLarge.getOutput().get(0), + "y", otherRight.getOutput().get(1)); + + LogicalUnion union = union(unionChild1, unionChild2); + + Plan rewritten = PlanChecker.from(MemoTestUtils.createConnectContext(), union) + .applyTopDown(new PullUpJoinFromUnionAll()) + .getPlan(); + + Assertions.assertInstanceOf(LogicalUnion.class, rewritten); + LogicalUnion rewrittenUnion = (LogicalUnion) rewritten; + Assertions.assertEquals(2, rewrittenUnion.children().size()); + Assertions.assertInstanceOf(LogicalProject.class, rewrittenUnion.child(0)); + Assertions.assertInstanceOf(LogicalProject.class, rewrittenUnion.child(1)); + } + + private static LogicalOlapScan newScan(long tableId, String tableName) { + return PlanConstructor.newLogicalOlapScan(tableId, tableName, 0); + } + + private static LogicalJoin join(LogicalProject commonSide, + LogicalOlapScan otherSide) { + Expression joinCondition = new EqualTo(commonSide.getOutput().get(0), otherSide.getOutput().get(0)); + return new LogicalJoin<>(JoinType.INNER_JOIN, ImmutableList.of(joinCondition), ImmutableList.of(), + commonSide, otherSide, null); + } + + private static LogicalProject outputProject(Plan child, String leftAlias, Slot leftSlot, + String rightAlias, Slot rightSlot) { + return new LogicalProject<>(ImmutableList.of( + new Alias(leftSlot, leftAlias), + new Alias(rightSlot, rightAlias)), child); + } + + private static LogicalUnion union(LogicalProject left, LogicalProject right) { + ImmutableList.Builder outputs = ImmutableList.builder(); + for (Slot slot : left.getOutput()) { + outputs.add(new SlotReference(slot.getName(), slot.getDataType(), slot.nullable())); + } + return new LogicalUnion(Qualifier.ALL, outputs.build(), ImmutableList.of( + toSlotReferences(left.getOutput()), + toSlotReferences(right.getOutput())), ImmutableList.of(), false, ImmutableList.of(left, right)); + } + + private static LogicalProject project(List outputs, LogicalOlapScan child) { + return new LogicalProject<>(outputs, child); + } + + private static List selectSlots(List output, int... indexes) { + ImmutableList.Builder selected = ImmutableList.builder(); + for (int index : indexes) { + selected.add(output.get(index)); + } + return selected.build(); + } + + private static List toSlotReferences(List slots) { + ImmutableList.Builder references = ImmutableList.builder(); + for (Slot slot : slots) { + references.add((SlotReference) slot); + } + return references.build(); + } +} From ea8cf174a34b3baaeb377b7ffd502613999c4141 Mon Sep 17 00:00:00 2001 From: yujun Date: Mon, 13 Jul 2026 15:27:04 +0800 Subject: [PATCH 2/2] [test](fe) Cover non-project output mismatch in PullUpJoinFromUnionAll Add non-project coverage for the output-arity guard in PullUpJoinFromUnionAll so the new check is validated on filtered scan shapes instead of only on LogicalProject-local paths. Key changes: - add a comparator test for LogicalFilter over same-table scans with different cached output arity - rewrite the rewrite-skip test to use filtered common sides backed by cached scan outputs Unit Test: - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.rewrite.PullUpJoinFromUnionAllTest --- .../rewrite/PullUpJoinFromUnionAllTest.java | 42 ++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAllTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAllTest.java index 244aef7c80d536..2b456674e52682 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAllTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PullUpJoinFromUnionAllTest.java @@ -26,6 +26,7 @@ import org.apache.doris.nereids.trees.plans.JoinType; import org.apache.doris.nereids.trees.plans.Plan; import org.apache.doris.nereids.trees.plans.algebra.SetOperation.Qualifier; +import org.apache.doris.nereids.trees.plans.logical.LogicalFilter; import org.apache.doris.nereids.trees.plans.logical.LogicalJoin; import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan; import org.apache.doris.nereids.trees.plans.logical.LogicalProject; @@ -35,6 +36,7 @@ import org.apache.doris.nereids.util.PlanConstructor; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -55,13 +57,23 @@ void comparatorRejectsDifferentProjectOutputSizes() { } @Test - void ruleSkipsJoinChildrenWithDifferentCommonSideOutputs() { + void comparatorRejectsDifferentFilterChildOutputSizes() { + LogicalFilter smallFilter = filter(newCachedOutputScan(1, "common_filter", 0)); + LogicalFilter largeFilter = filter(newCachedOutputScan(1, "common_filter", 0, 1)); + + PullUpJoinFromUnionAll.LogicalPlanComparator comparator = + new PullUpJoinFromUnionAll().new LogicalPlanComparator(); + Assertions.assertFalse(comparator.isLogicalEqual(largeFilter, smallFilter)); + } + + @Test + void ruleSkipsJoinChildrenWithDifferentFilteredCommonSideOutputs() { LogicalOlapScan commonSmallScan = newScan(10, "common_small"); LogicalOlapScan commonLargeScan = newScan(10, "common_large"); - LogicalProject commonSmall = project(selectSlots(commonSmallScan.getOutput(), 0), - commonSmallScan); - LogicalProject commonLarge = project(selectSlots(commonLargeScan.getOutput(), 0, 1), - commonLargeScan); + LogicalFilter commonSmall = filter(commonSmallScan.withCachedOutput( + selectSlotsAsSlots(commonSmallScan.getOutput(), 0))); + LogicalFilter commonLarge = filter(commonLargeScan.withCachedOutput( + selectSlotsAsSlots(commonLargeScan.getOutput(), 0, 1))); LogicalOlapScan otherLeft = newScan(20, "other_left"); LogicalOlapScan otherRight = newScan(30, "other_right"); @@ -91,7 +103,17 @@ private static LogicalOlapScan newScan(long tableId, String tableName) { return PlanConstructor.newLogicalOlapScan(tableId, tableName, 0); } - private static LogicalJoin join(LogicalProject commonSide, + private static LogicalOlapScan newCachedOutputScan(long tableId, String tableName, int... indexes) { + LogicalOlapScan scan = newScan(tableId, tableName); + return scan.withCachedOutput(selectSlotsAsSlots(scan.getOutput(), indexes)); + } + + private static LogicalFilter filter(LogicalOlapScan child) { + return new LogicalFilter<>(ImmutableSet.of(new EqualTo(child.getOutput().get(0), child.getOutput().get(0))), + child); + } + + private static LogicalJoin join(Plan commonSide, LogicalOlapScan otherSide) { Expression joinCondition = new EqualTo(commonSide.getOutput().get(0), otherSide.getOutput().get(0)); return new LogicalJoin<>(JoinType.INNER_JOIN, ImmutableList.of(joinCondition), ImmutableList.of(), @@ -127,6 +149,14 @@ private static List selectSlots(List output, int... index return selected.build(); } + private static List selectSlotsAsSlots(List output, int... indexes) { + ImmutableList.Builder selected = ImmutableList.builder(); + for (int index : indexes) { + selected.add(output.get(index)); + } + return selected.build(); + } + private static List toSlotReferences(List slots) { ImmutableList.Builder references = ImmutableList.builder(); for (Slot slot : slots) {