This is an automated email from the ASF dual-hosted git repository.
morrysnow 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 28b2d23ecfa [chore](cte) use a better way to get child in enforce
regulator (#59395)
28b2d23ecfa is described below
commit 28b2d23ecfa6b0e04b79d8d59670373bc2bb7234
Author: morrySnow <[email protected]>
AuthorDate: Wed Dec 31 11:56:36 2025 +0800
[chore](cte) use a better way to get child in enforce regulator (#59395)
related PR #58964
Problem Summary:
This pull request refactors how child physical plans are accessed in the
`ChildrenPropertiesRegulator` class, simplifying the code and improving
test clarity. The main change is the removal of the
`getChildPhysicalPlan` helper method, replacing its usage with direct
access to the plan from the `children` list. The tests are also updated
to build child mocks more locally, improving test isolation and
readability.
Refactoring and Simplification:
* Removed the `getChildPhysicalPlan` method from
`ChildrenPropertiesRegulator`, and replaced its usage in
`visitPhysicalFilter` and `visitPhysicalProject` with direct access to
the child plan via `children.get(0).getPlan()`. This simplifies the code
by eliminating unnecessary indirection.
---
.../properties/ChildrenPropertiesRegulator.java | 19 ++--------
.../ChildrenPropertiesRegulatorTest.java | 41 ++++++++++++++--------
2 files changed, 28 insertions(+), 32 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java
index 32a5202bee4..5e1346c2991 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulator.java
@@ -281,8 +281,7 @@ public class ChildrenPropertiesRegulator extends
PlanVisitor<List<List<PhysicalP
DistributionSpec distributionSpec =
originChildrenProperties.get(0).getDistributionSpec();
// process must shuffle
if (distributionSpec instanceof DistributionSpecMustShuffle) {
- Plan child = filter.child();
- Plan realChild = getChildPhysicalPlan(child);
+ Plan realChild = children.get(0).getPlan();
if (realChild instanceof PhysicalProject
|| realChild instanceof PhysicalFilter
|| realChild instanceof PhysicalLimit) {
@@ -320,19 +319,6 @@ public class ChildrenPropertiesRegulator extends
PlanVisitor<List<List<PhysicalP
}
}
- private Plan getChildPhysicalPlan(Plan plan) {
- if (!(plan instanceof GroupPlan)) {
- return null;
- }
- GroupPlan groupPlan = (GroupPlan) plan;
- if (groupPlan == null || groupPlan.getGroup() == null
- || groupPlan.getGroup().getPhysicalExpressions().isEmpty()) {
- return null;
- } else {
- return
groupPlan.getGroup().getPhysicalExpressions().get(0).getPlan();
- }
- }
-
private PhysicalOlapScan findDownGradeBucketShuffleCandidate(GroupPlan
groupPlan) {
if (groupPlan == null || groupPlan.getGroup() == null
|| groupPlan.getGroup().getPhysicalExpressions().isEmpty()) {
@@ -602,8 +588,7 @@ public class ChildrenPropertiesRegulator extends
PlanVisitor<List<List<PhysicalP
DistributionSpec distributionSpec =
originChildrenProperties.get(0).getDistributionSpec();
// process must shuffle
if (distributionSpec instanceof DistributionSpecMustShuffle) {
- Plan child = project.child();
- Plan realChild = getChildPhysicalPlan(child);
+ Plan realChild = children.get(0).getPlan();
if (realChild instanceof PhysicalLimit) {
visit(project, context);
} else if (realChild instanceof PhysicalProject) {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulatorTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulatorTest.java
index eab66c026a5..f63cc5d8d45 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulatorTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/properties/ChildrenPropertiesRegulatorTest.java
@@ -47,44 +47,29 @@ import java.util.Optional;
public class ChildrenPropertiesRegulatorTest {
- private List<GroupExpression> children;
private JobContext mockedJobContext;
private List<PhysicalProperties> originOutputChildrenProperties
= Lists.newArrayList(PhysicalProperties.MUST_SHUFFLE);
@BeforeEach
public void setUp() {
- Group childGroup = Mockito.mock(Group.class);
-
Mockito.when(childGroup.getLogicalProperties()).thenReturn(Mockito.mock(LogicalProperties.class));
- GroupExpression child = Mockito.mock(GroupExpression.class);
-
Mockito.when(child.getOutputProperties(Mockito.any())).thenReturn(PhysicalProperties.MUST_SHUFFLE);
- Mockito.when(child.getOwnerGroup()).thenReturn(childGroup);
- Map<PhysicalProperties, Pair<Cost, List<PhysicalProperties>>> lct =
Maps.newHashMap();
- lct.put(PhysicalProperties.MUST_SHUFFLE, Pair.of(Cost.zero(),
Lists.newArrayList()));
- Mockito.when(child.getLowestCostTable()).thenReturn(lct);
- children = Lists.newArrayList(child);
-
mockedJobContext = Mockito.mock(JobContext.class);
Mockito.when(mockedJobContext.getCascadesContext()).thenReturn(Mockito.mock(CascadesContext.class));
-
}
@Test
public void testMustShuffleProjectProjectCanNotMerge() {
testMustShuffleProject(PhysicalProject.class,
DistributionSpecExecutionAny.class, false);
-
}
@Test
public void testMustShuffleProjectProjectCanMerge() {
testMustShuffleProject(PhysicalProject.class,
DistributionSpecMustShuffle.class, true);
-
}
@Test
public void testMustShuffleProjectFilter() {
testMustShuffleProject(PhysicalFilter.class,
DistributionSpecMustShuffle.class, true);
-
}
@Test
@@ -111,6 +96,19 @@ public class ChildrenPropertiesRegulatorTest {
Mockito.when(mockedGroupPlan.getGroup()).thenReturn(mockedGroup);
// let AbstractTreeNode's init happy
Mockito.when(mockedGroupPlan.getAllChildrenTypes()).thenReturn(new
BitSet());
+
+ List<GroupExpression> children;
+ Group childGroup = Mockito.mock(Group.class);
+
Mockito.when(childGroup.getLogicalProperties()).thenReturn(Mockito.mock(LogicalProperties.class));
+ GroupExpression child = Mockito.mock(GroupExpression.class);
+
Mockito.when(child.getOutputProperties(Mockito.any())).thenReturn(PhysicalProperties.MUST_SHUFFLE);
+ Mockito.when(child.getOwnerGroup()).thenReturn(childGroup);
+ Map<PhysicalProperties, Pair<Cost, List<PhysicalProperties>>> lct
= Maps.newHashMap();
+ lct.put(PhysicalProperties.MUST_SHUFFLE, Pair.of(Cost.zero(),
Lists.newArrayList()));
+ Mockito.when(child.getLowestCostTable()).thenReturn(lct);
+ Mockito.when(child.getPlan()).thenReturn(mockedChild);
+ children = Lists.newArrayList(child);
+
PhysicalProject parentPlan = new
PhysicalProject<>(Lists.newArrayList(), null, mockedGroupPlan);
GroupExpression parent = new GroupExpression(parentPlan);
parentPlan = parentPlan.withGroupExpression(Optional.of(parent));
@@ -157,6 +155,19 @@ public class ChildrenPropertiesRegulatorTest {
Mockito.when(mockedGroupPlan.getGroup()).thenReturn(mockedGroup);
// let AbstractTreeNode's init happy
Mockito.when(mockedGroupPlan.getAllChildrenTypes()).thenReturn(new
BitSet());
+
+ List<GroupExpression> children;
+ Group childGroup = Mockito.mock(Group.class);
+
Mockito.when(childGroup.getLogicalProperties()).thenReturn(Mockito.mock(LogicalProperties.class));
+ GroupExpression child = Mockito.mock(GroupExpression.class);
+
Mockito.when(child.getOutputProperties(Mockito.any())).thenReturn(PhysicalProperties.MUST_SHUFFLE);
+ Mockito.when(child.getOwnerGroup()).thenReturn(childGroup);
+ Map<PhysicalProperties, Pair<Cost, List<PhysicalProperties>>> lct
= Maps.newHashMap();
+ lct.put(PhysicalProperties.MUST_SHUFFLE, Pair.of(Cost.zero(),
Lists.newArrayList()));
+ Mockito.when(child.getLowestCostTable()).thenReturn(lct);
+ Mockito.when(child.getPlan()).thenReturn(mockedChild);
+ children = Lists.newArrayList(child);
+
GroupExpression parent = new GroupExpression(new
PhysicalFilter<>(Sets.newHashSet(), null, mockedGroupPlan));
ChildrenPropertiesRegulator regulator = new
ChildrenPropertiesRegulator(parent, children,
new ArrayList<>(originOutputChildrenProperties), null,
mockedJobContext);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
