diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java index 4f4a8bebfdf562..7c78779b8b6f6f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriver.java @@ -371,8 +371,9 @@ public PhysicalProperties visitPhysicalRepeat(PhysicalRepeat rep intersectGroupingKeys, Utils.fastToImmutableSet(groupingSets.get(i)) ); } - if (!intersectGroupingKeys.isEmpty()) { - List orderedShuffledColumns = distributionSpecHash.getOrderedShuffledColumns(); + List orderedShuffledColumns = distributionSpecHash.getOrderedShuffledColumns(); + if (!intersectGroupingKeys.isEmpty() && intersectGroupingKeys.size() + >= Sets.newHashSet(orderedShuffledColumns).size()) { boolean hashColumnsChanged = false; for (Expression intersectGroupingKey : intersectGroupingKeys) { if (!(intersectGroupingKey instanceof SlotReference)) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriverTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriverTest.java index 91738e3cca457e..a39bc664e1c1de 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriverTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildOutputPropertyDeriverTest.java @@ -47,9 +47,11 @@ import org.apache.doris.nereids.trees.plans.physical.PhysicalLimit; import org.apache.doris.nereids.trees.plans.physical.PhysicalNestedLoopJoin; import org.apache.doris.nereids.trees.plans.physical.PhysicalQuickSort; +import org.apache.doris.nereids.trees.plans.physical.PhysicalRepeat; import org.apache.doris.nereids.trees.plans.physical.PhysicalTopN; import org.apache.doris.nereids.types.BigIntType; import org.apache.doris.nereids.types.IntegerType; +import org.apache.doris.nereids.types.TinyIntType; import org.apache.doris.nereids.util.ExpressionUtils; import org.apache.doris.nereids.util.JoinUtils; import org.apache.doris.qe.ConnectContext; @@ -133,7 +135,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); // check merged @@ -172,7 +174,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); // check merged @@ -211,7 +213,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); // check merged @@ -251,7 +253,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); // check merged @@ -291,7 +293,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); // check merged @@ -331,7 +333,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); // check merged @@ -371,7 +373,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); Assertions.assertEquals(-1, actual.getTableId()); @@ -412,7 +414,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); Assertions.assertEquals(-1, actual.getTableId()); @@ -453,7 +455,7 @@ ExpressionUtils.EMPTY_CONDITION, ExpressionUtils.EMPTY_CONDITION, new Distribute PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); Assertions.assertEquals(-1, actual.getTableId()); @@ -576,7 +578,7 @@ ExpressionUtils.EMPTY_CONDITION, new DistributeHint(DistributeType.NONE), PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.NATURAL, actual.getShuffleType()); // check merged @@ -626,7 +628,7 @@ Pair, List> getOnClauseUsedSlots( PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.EXECUTION_BUCKETED, actual.getShuffleType()); // check merged @@ -659,7 +661,7 @@ void testNestedLoopJoin() { PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(leftHash, actual); } @@ -712,7 +714,7 @@ void testGlobalPhaseAggregate() { ChildOutputPropertyDeriver deriver = new ChildOutputPropertyDeriver(Lists.newArrayList(child)); PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertTrue(result.getOrderSpec().getOrderKeys().isEmpty()); - Assertions.assertTrue(result.getDistributionSpec() instanceof DistributionSpecHash); + Assertions.assertInstanceOf(DistributionSpecHash.class, result.getDistributionSpec()); DistributionSpecHash actual = (DistributionSpecHash) result.getDistributionSpec(); Assertions.assertEquals(ShuffleType.EXECUTION_BUCKETED, actual.getShuffleType()); Assertions.assertEquals(Lists.newArrayList(partition).stream() @@ -837,4 +839,50 @@ void testAssertNumRows() { PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); Assertions.assertEquals(PhysicalProperties.GATHER, result); } + + @Test + void testRepeatReturnAny() { + SlotReference c1 = new SlotReference( + new ExprId(1), "c1", TinyIntType.INSTANCE, true, ImmutableList.of()); + SlotReference c2 = new SlotReference( + new ExprId(2), "c2", TinyIntType.INSTANCE, true, ImmutableList.of()); + SlotReference c3 = new SlotReference( + new ExprId(3), "c3", TinyIntType.INSTANCE, true, ImmutableList.of()); + PhysicalRepeat repeat = new PhysicalRepeat<>( + ImmutableList.of(ImmutableList.of(c1, c2), ImmutableList.of(c1), ImmutableList.of(c1, c3)), + ImmutableList.of(c1, c2, c3), + logicalProperties, + groupPlan + ); + GroupExpression groupExpression = new GroupExpression(repeat); + new Group(null, groupExpression, null); + PhysicalProperties child = PhysicalProperties.createHash( + ImmutableList.of(new ExprId(1), new ExprId(2)), ShuffleType.EXECUTION_BUCKETED); + ChildOutputPropertyDeriver deriver = new ChildOutputPropertyDeriver(Lists.newArrayList(child)); + PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); + Assertions.assertEquals(PhysicalProperties.ANY, result); + } + + @Test + void testRepeatReturnChild() { + SlotReference c1 = new SlotReference( + new ExprId(1), "c1", TinyIntType.INSTANCE, true, ImmutableList.of()); + SlotReference c2 = new SlotReference( + new ExprId(2), "c2", TinyIntType.INSTANCE, true, ImmutableList.of()); + SlotReference c3 = new SlotReference( + new ExprId(3), "c3", TinyIntType.INSTANCE, true, ImmutableList.of()); + PhysicalRepeat repeat = new PhysicalRepeat<>( + ImmutableList.of(ImmutableList.of(c1, c2), ImmutableList.of(c1), ImmutableList.of(c1, c3)), + ImmutableList.of(c1, c2, c3), + logicalProperties, + groupPlan + ); + GroupExpression groupExpression = new GroupExpression(repeat); + new Group(null, groupExpression, null); + PhysicalProperties child = PhysicalProperties.createHash( + ImmutableList.of(new ExprId(1)), ShuffleType.EXECUTION_BUCKETED); + ChildOutputPropertyDeriver deriver = new ChildOutputPropertyDeriver(Lists.newArrayList(child)); + PhysicalProperties result = deriver.getOutputProperties(null, groupExpression); + Assertions.assertEquals(child, result); + } }