Skip to content

Commit 3a9dcf4

Browse files
branch-4.0: [fix](nereids) when Expression has no more than 2 children, the attribute hasUnbound is not set correctly #60152 (#60305)
Cherry-picked from #60152 Co-authored-by: minghong <zhouminghong@selectdb.com>
1 parent 5c46a39 commit 3a9dcf4

2 files changed

Lines changed: 237 additions & 47 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Expression.java

Lines changed: 4 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import com.google.common.collect.Lists;
5252
import org.apache.commons.lang3.StringUtils;
5353

54+
import java.util.Arrays;
5455
import java.util.List;
5556
import java.util.Optional;
5657
import java.util.Set;
@@ -76,53 +77,7 @@ public abstract class Expression extends AbstractTreeNode<Expression> implements
7677
private final Supplier<Integer> hashCodeCache = LazyCompute.of(this::computeHashCode);
7778

7879
protected Expression(Expression... children) {
79-
super(children);
80-
81-
boolean hasUnbound = false;
82-
switch (children.length) {
83-
case 0:
84-
this.depth = 1;
85-
this.width = 1;
86-
this.compareWidthAndDepth = supportCompareWidthAndDepth();
87-
this.fastChildrenHashCode = 0;
88-
break;
89-
case 1:
90-
Expression child = children[0];
91-
this.depth = child.depth + 1;
92-
this.width = child.width;
93-
this.compareWidthAndDepth = child.compareWidthAndDepth && supportCompareWidthAndDepth();
94-
this.fastChildrenHashCode = child.fastChildrenHashCode() + 1;
95-
break;
96-
case 2:
97-
Expression left = children[0];
98-
Expression right = children[1];
99-
this.depth = Math.max(left.depth, right.depth) + 1;
100-
this.width = left.width + right.width;
101-
this.compareWidthAndDepth =
102-
left.compareWidthAndDepth && right.compareWidthAndDepth && supportCompareWidthAndDepth();
103-
this.fastChildrenHashCode = left.fastChildrenHashCode() + right.fastChildrenHashCode() + 2;
104-
break;
105-
default:
106-
int maxChildDepth = 0;
107-
int sumChildWidth = 0;
108-
boolean compareWidthAndDepth = true;
109-
int fastChildrenHashCode = 0;
110-
for (Expression expression : children) {
111-
child = expression;
112-
maxChildDepth = Math.max(child.depth, maxChildDepth);
113-
sumChildWidth += child.width;
114-
hasUnbound |= child.hasUnbound;
115-
compareWidthAndDepth &= child.compareWidthAndDepth;
116-
fastChildrenHashCode = fastChildrenHashCode + expression.fastChildrenHashCode() + 1;
117-
}
118-
this.depth = maxChildDepth + 1;
119-
this.width = sumChildWidth;
120-
this.compareWidthAndDepth = compareWidthAndDepth;
121-
this.fastChildrenHashCode = fastChildrenHashCode;
122-
}
123-
checkLimit();
124-
this.inferred = false;
125-
this.hasUnbound = hasUnbound || this instanceof Unbound;
80+
this(Arrays.asList(children));
12681
}
12782

12883
protected Expression(List<Expression> children) {
@@ -146,6 +101,7 @@ protected Expression(List<Expression> children, boolean inferred) {
146101
this.width = child.width;
147102
this.compareWidthAndDepth = child.compareWidthAndDepth && supportCompareWidthAndDepth();
148103
this.fastChildrenHashCode = child.fastChildrenHashCode() + 1;
104+
hasUnbound = child.hasUnbound();
149105
break;
150106
case 2:
151107
Expression left = children.get(0);
@@ -155,6 +111,7 @@ protected Expression(List<Expression> children, boolean inferred) {
155111
this.compareWidthAndDepth =
156112
left.compareWidthAndDepth && right.compareWidthAndDepth && supportCompareWidthAndDepth();
157113
this.fastChildrenHashCode = left.fastChildrenHashCode() + right.fastChildrenHashCode() + 2;
114+
hasUnbound = left.hasUnbound() || right.hasUnbound();
158115
break;
159116
default:
160117
int maxChildDepth = 0;
Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.doris.nereids.trees.expressions;
19+
20+
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
21+
import org.apache.doris.nereids.types.DataType;
22+
import org.apache.doris.nereids.types.IntegerType;
23+
24+
import org.junit.jupiter.api.Assertions;
25+
import org.junit.jupiter.api.Test;
26+
27+
import java.util.List;
28+
29+
/**
30+
* Test for Expression hasUnbound assignment logic.
31+
*/
32+
public class ExpressionUnboundTest {
33+
34+
/**
35+
* Mock bound expression for testing.
36+
*/
37+
private static class MockBoundExpression extends Expression {
38+
public MockBoundExpression() {
39+
super();
40+
}
41+
42+
public MockBoundExpression(Expression... children) {
43+
super(children);
44+
}
45+
46+
public MockBoundExpression(List<Expression> children) {
47+
super(children);
48+
}
49+
50+
@Override
51+
public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
52+
return null;
53+
}
54+
55+
@Override
56+
public DataType getDataType() {
57+
return IntegerType.INSTANCE;
58+
}
59+
60+
@Override
61+
public boolean nullable() {
62+
return false;
63+
}
64+
65+
@Override
66+
public Expression withChildren(List<Expression> children) {
67+
return new MockBoundExpression(children);
68+
}
69+
70+
@Override
71+
protected String computeToSql() {
72+
return toSql();
73+
}
74+
}
75+
76+
/**
77+
* Mock unbound expression for testing.
78+
*/
79+
private static class MockUnboundExpression extends Expression {
80+
public MockUnboundExpression() {
81+
super();
82+
}
83+
84+
public MockUnboundExpression(Expression... children) {
85+
super(children);
86+
}
87+
88+
public MockUnboundExpression(List<Expression> children) {
89+
super(children);
90+
}
91+
92+
@Override
93+
public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
94+
return null;
95+
}
96+
97+
@Override
98+
public DataType getDataType() {
99+
return IntegerType.INSTANCE;
100+
}
101+
102+
@Override
103+
public boolean nullable() {
104+
return false;
105+
}
106+
107+
@Override
108+
public Expression withChildren(List<Expression> children) {
109+
return new MockUnboundExpression(children);
110+
}
111+
112+
@Override
113+
protected String computeToSql() {
114+
return toSql();
115+
}
116+
117+
// Override hasUnbound to return true for testing
118+
@Override
119+
public boolean hasUnbound() {
120+
return true;
121+
}
122+
}
123+
124+
/**
125+
* Test case 1: Single bound child (line 115 logic).
126+
* When there's one bound child, parent should be bound.
127+
*/
128+
@Test
129+
public void testSingleBoundChild() {
130+
// Create a bound child expression
131+
MockBoundExpression boundChild = new MockBoundExpression();
132+
Assertions.assertFalse(boundChild.hasUnbound(), "Bound child should not have unbound");
133+
134+
// Create parent with single bound child
135+
MockBoundExpression parent = new MockBoundExpression(boundChild);
136+
137+
// Verify line 115: hasUnbound = child.hasUnbound();
138+
Assertions.assertFalse(parent.hasUnbound(), "Parent with bound child should not have unbound");
139+
}
140+
141+
/**
142+
* Test case 2: Single unbound child (line 115 logic).
143+
* When there's one unbound child, parent should be unbound.
144+
*/
145+
@Test
146+
public void testSingleUnboundChild() {
147+
// Create an unbound child expression
148+
MockUnboundExpression unboundChild = new MockUnboundExpression();
149+
Assertions.assertTrue(unboundChild.hasUnbound(), "Unbound child should have unbound");
150+
151+
// Create parent with single unbound child
152+
MockBoundExpression parent = new MockBoundExpression(unboundChild);
153+
154+
// Verify line 115: hasUnbound = child.hasUnbound();
155+
Assertions.assertTrue(parent.hasUnbound(), "Parent with unbound child should have unbound");
156+
}
157+
158+
/**
159+
* Test case 3: Two bound children (line 125 logic).
160+
* When both children are bound, parent should be bound.
161+
*/
162+
@Test
163+
public void testTwoBoundChildren() {
164+
// Create two bound child expressions
165+
MockBoundExpression leftChild = new MockBoundExpression();
166+
MockBoundExpression rightChild = new MockBoundExpression();
167+
Assertions.assertFalse(leftChild.hasUnbound(), "Left bound child should not have unbound");
168+
Assertions.assertFalse(rightChild.hasUnbound(), "Right bound child should not have unbound");
169+
170+
// Create parent with two bound children
171+
MockBoundExpression parent = new MockBoundExpression(leftChild, rightChild);
172+
173+
// Verify line 125: hasUnbound = left.hasUnbound() || right.hasUnbound();
174+
Assertions.assertFalse(parent.hasUnbound(), "Parent with two bound children should not have unbound");
175+
}
176+
177+
/**
178+
* Test case 4: Left unbound, right bound (line 125 logic).
179+
* When left child is unbound, parent should be unbound.
180+
*/
181+
@Test
182+
public void testLeftUnboundRightBound() {
183+
// Create unbound left and bound right child
184+
MockUnboundExpression leftChild = new MockUnboundExpression();
185+
MockBoundExpression rightChild = new MockBoundExpression();
186+
Assertions.assertTrue(leftChild.hasUnbound(), "Left unbound child should have unbound");
187+
Assertions.assertFalse(rightChild.hasUnbound(), "Right bound child should not have unbound");
188+
189+
// Create parent with left unbound, right bound
190+
MockBoundExpression parent = new MockBoundExpression(leftChild, rightChild);
191+
192+
// Verify line 125: hasUnbound = left.hasUnbound() || right.hasUnbound();
193+
Assertions.assertTrue(parent.hasUnbound(), "Parent with left unbound child should have unbound");
194+
}
195+
196+
/**
197+
* Test case 5: Left bound, right unbound (line 125 logic).
198+
* When right child is unbound, parent should be unbound.
199+
*/
200+
@Test
201+
public void testLeftBoundRightUnbound() {
202+
// Create bound left and unbound right child
203+
MockBoundExpression leftChild = new MockBoundExpression();
204+
MockUnboundExpression rightChild = new MockUnboundExpression();
205+
Assertions.assertFalse(leftChild.hasUnbound(), "Left bound child should not have unbound");
206+
Assertions.assertTrue(rightChild.hasUnbound(), "Right unbound child should have unbound");
207+
208+
// Create parent with left bound, right unbound
209+
MockBoundExpression parent = new MockBoundExpression(leftChild, rightChild);
210+
211+
// Verify line 125: hasUnbound = left.hasUnbound() || right.hasUnbound();
212+
Assertions.assertTrue(parent.hasUnbound(), "Parent with right unbound child should have unbound");
213+
}
214+
215+
/**
216+
* Test case 6: Both children unbound (line 125 logic).
217+
* When both children are unbound, parent should be unbound.
218+
*/
219+
@Test
220+
public void testTwoUnboundChildren() {
221+
// Create two unbound child expressions
222+
MockUnboundExpression leftChild = new MockUnboundExpression();
223+
MockUnboundExpression rightChild = new MockUnboundExpression();
224+
Assertions.assertTrue(leftChild.hasUnbound(), "Left unbound child should have unbound");
225+
Assertions.assertTrue(rightChild.hasUnbound(), "Right unbound child should have unbound");
226+
227+
// Create parent with two unbound children
228+
MockBoundExpression parent = new MockBoundExpression(leftChild, rightChild);
229+
230+
// Verify line 125: hasUnbound = left.hasUnbound() || right.hasUnbound();
231+
Assertions.assertTrue(parent.hasUnbound(), "Parent with two unbound children should have unbound");
232+
}
233+
}

0 commit comments

Comments
 (0)