Skip to content

Commit db22016

Browse files
Wei-hao-LiWei-hao-Li
authored
Fix order in subquery when meets diff function (#17629)
1 parent cfbcc24 commit db22016

5 files changed

Lines changed: 210 additions & 8 deletions

File tree

‎integration-test/src/test/java/org/apache/iotdb/relational/it/query/old/builtinfunction/scalar/IoTDBDiffFunctionTableIT.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,4 +146,84 @@ public void testCaseInSensitive() {
146146
retArray,
147147
DATABASE_NAME);
148148
}
149+
150+
@Test
151+
public void testDiffWithOrderBySubquery() {
152+
String[] expectedHeader = new String[] {"time", "device_id", "s1", "_col3"};
153+
String[] retArray =
154+
new String[] {
155+
"1970-02-27T20:53:20.001Z,d1,8,3.0,",
156+
"1970-02-27T20:53:20.000Z,d1,null,null,",
157+
"1970-01-01T00:00:00.006Z,d1,null,null,",
158+
"1970-01-01T00:00:00.005Z,d1,5,1.0,",
159+
"1970-01-01T00:00:00.004Z,d1,4,2.0,",
160+
"1970-01-01T00:00:00.003Z,d1,null,null,",
161+
"1970-01-01T00:00:00.002Z,d1,2,1.0,",
162+
"1970-01-01T00:00:00.001Z,d1,1,null,"
163+
};
164+
tableResultSetEqualTest(
165+
"SELECT time, device_id, s1, diff(s1) FROM ("
166+
+ "select * "
167+
+ "from table1 where device_id='d1' ORDER by time"
168+
+ ") "
169+
+ "ORDER by time DESC",
170+
expectedHeader,
171+
retArray,
172+
DATABASE_NAME);
173+
}
174+
175+
@Test
176+
public void testDiffInOuterWhereHavingOrderBy() {
177+
String[] expectedHeader = new String[] {"time", "device_id", "s1"};
178+
String[] expectedRowsFilteredByDiff =
179+
new String[] {
180+
"1970-02-27T20:53:20.001Z,d1,8,",
181+
"1970-01-01T00:00:00.005Z,d1,5,",
182+
"1970-01-01T00:00:00.004Z,d1,4,",
183+
"1970-01-01T00:00:00.002Z,d1,2,"
184+
};
185+
tableResultSetEqualTest(
186+
"SELECT time, device_id, s1 FROM ("
187+
+ "select * "
188+
+ "from table1 where device_id='d1' ORDER by time"
189+
+ ") "
190+
+ "WHERE diff(s1) IS NOT NULL "
191+
+ "ORDER by time DESC",
192+
expectedHeader,
193+
expectedRowsFilteredByDiff,
194+
DATABASE_NAME);
195+
196+
tableResultSetEqualTest(
197+
"SELECT time, device_id, s1 FROM ("
198+
+ "select * "
199+
+ "from table1 where device_id='d1' ORDER by time"
200+
+ ") "
201+
+ "GROUP BY time, device_id, s1 "
202+
+ "HAVING diff(s1) IS NOT NULL "
203+
+ "ORDER by time DESC",
204+
expectedHeader,
205+
expectedRowsFilteredByDiff,
206+
DATABASE_NAME);
207+
208+
String[] expectedRowsOrderedByDiff =
209+
new String[] {
210+
"1970-02-27T20:53:20.001Z,d1,8,",
211+
"1970-01-01T00:00:00.004Z,d1,4,",
212+
"1970-01-01T00:00:00.005Z,d1,5,",
213+
"1970-01-01T00:00:00.002Z,d1,2,",
214+
"1970-02-27T20:53:20.000Z,d1,null,",
215+
"1970-01-01T00:00:00.006Z,d1,null,",
216+
"1970-01-01T00:00:00.003Z,d1,null,",
217+
"1970-01-01T00:00:00.001Z,d1,1,"
218+
};
219+
tableResultSetEqualTest(
220+
"SELECT time, device_id, s1 FROM ("
221+
+ "select * "
222+
+ "from table1 where device_id='d1' ORDER by time"
223+
+ ") "
224+
+ "ORDER BY coalesce(diff(s1), -1000.0) DESC, time DESC",
225+
expectedHeader,
226+
expectedRowsOrderedByDiff,
227+
DATABASE_NAME);
228+
}
149229
}

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/StatementAnalyzer.java‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@
292292
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.Scope.BasisType.TABLE;
293293
import static org.apache.iotdb.db.queryengine.plan.relational.metadata.MetadataUtil.createQualifiedObjectName;
294294
import static org.apache.iotdb.db.queryengine.plan.relational.planner.IrExpressionInterpreter.evaluateConstantExpression;
295+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.optimizations.PushPredicateIntoTableScan.containsDiffFunction;
295296
import static org.apache.iotdb.db.queryengine.plan.relational.sql.util.AstUtil.preOrder;
296297
import static org.apache.iotdb.db.queryengine.plan.relational.utils.NodeUtils.getSortItemsFromOrderBy;
297298
import static org.apache.tsfile.read.common.type.BooleanType.BOOLEAN;
@@ -303,6 +304,7 @@ public class StatementAnalyzer {
303304
private final Analysis analysis;
304305

305306
private boolean hasFillInParentScope = false;
307+
private boolean hasDiffFunctionInParentScope = false;
306308
private final MPPQueryContext queryContext;
307309

308310
private final AccessControl accessControl;
@@ -870,9 +872,13 @@ public Scope visitExplainAnalyze(ExplainAnalyze node, Optional<Scope> context) {
870872

871873
@Override
872874
public Scope visitQuery(Query node, Optional<Scope> context) {
875+
boolean originalHasFillInParentScope = hasFillInParentScope;
876+
boolean originalHasDiffFunctionInParentScope = hasDiffFunctionInParentScope;
873877
analysis.setQuery(true);
874878
Scope withScope = analyzeWith(node, context);
875879
hasFillInParentScope = node.getFill().isPresent() || hasFillInParentScope;
880+
hasDiffFunctionInParentScope =
881+
containsDiffFunctionInQuery(node) || hasDiffFunctionInParentScope;
876882
Scope queryBodyScope = process(node.getQueryBody(), withScope);
877883

878884
if (node.getFill().isPresent()) {
@@ -887,7 +893,8 @@ public Scope visitQuery(Query node, Optional<Scope> context) {
887893
if ((queryBodyScope.getOuterQueryParent().isPresent() || !isTopLevel)
888894
&& !node.getLimit().isPresent()
889895
&& !node.getOffset().isPresent()
890-
&& !hasFillInParentScope) {
896+
&& !hasFillInParentScope
897+
&& !hasDiffFunctionInParentScope) {
891898
// not the root scope and ORDER BY is ineffective
892899
analysis.markRedundantOrderBy(node.getOrderBy().get());
893900
warningCollector.add(
@@ -922,6 +929,8 @@ public Scope visitQuery(Query node, Optional<Scope> context) {
922929
.build();
923930

924931
analysis.setScope(node, queryScope);
932+
hasFillInParentScope = originalHasFillInParentScope;
933+
hasDiffFunctionInParentScope = originalHasDiffFunctionInParentScope;
925934
return queryScope;
926935
}
927936

@@ -1144,6 +1153,7 @@ public Scope visitTableSubquery(TableSubquery node, Optional<Scope> scope) {
11441153
statementAnalyzerFactory.createStatementAnalyzer(
11451154
analysis, queryContext, sessionContext, warningCollector, CorrelationSupport.ALLOWED);
11461155
analyzer.hasFillInParentScope = hasFillInParentScope;
1156+
analyzer.hasDiffFunctionInParentScope = hasDiffFunctionInParentScope;
11471157
Scope queryScope =
11481158
analyzer.analyze(
11491159
node.getQuery(),
@@ -1153,9 +1163,13 @@ public Scope visitTableSubquery(TableSubquery node, Optional<Scope> scope) {
11531163

11541164
@Override
11551165
public Scope visitQuerySpecification(QuerySpecification node, Optional<Scope> scope) {
1166+
boolean originalHasFillInParentScope = hasFillInParentScope;
1167+
boolean originalHasDiffFunctionInParentScope = hasDiffFunctionInParentScope;
11561168
// TODO: extract candidate names from SELECT, WHERE, HAVING, GROUP BY and ORDER BY expressions
11571169
// to pass down to analyzeFrom
11581170
hasFillInParentScope = node.getFill().isPresent() || hasFillInParentScope;
1171+
hasDiffFunctionInParentScope =
1172+
containsDiffFunctionInQuerySpecification(node) || hasDiffFunctionInParentScope;
11591173

11601174
Scope sourceScope = analyzeFrom(node, scope);
11611175
analyzeWindowDefinitions(node, sourceScope);
@@ -1188,7 +1202,8 @@ public Scope visitQuerySpecification(QuerySpecification node, Optional<Scope> sc
11881202
if ((sourceScope.getOuterQueryParent().isPresent() || !isTopLevel)
11891203
&& !node.getLimit().isPresent()
11901204
&& !node.getOffset().isPresent()
1191-
&& !hasFillInParentScope) {
1205+
&& !hasFillInParentScope
1206+
&& !hasDiffFunctionInParentScope) {
11921207
// not the root scope and ORDER BY is ineffective
11931208
analysis.markRedundantOrderBy(orderBy);
11941209
warningCollector.add(
@@ -1249,6 +1264,8 @@ public Scope visitQuerySpecification(QuerySpecification node, Optional<Scope> sc
12491264
orderByScope.orElseThrow(() -> new NoSuchElementException("No value present")));
12501265
}
12511266

1267+
hasFillInParentScope = originalHasFillInParentScope;
1268+
hasDiffFunctionInParentScope = originalHasDiffFunctionInParentScope;
12521269
return outputScope;
12531270
}
12541271

@@ -1301,6 +1318,37 @@ private List<FunctionCall> analyzeWindowFunctions(
13011318
return windowFunctions;
13021319
}
13031320

1321+
// cover case: (query1) UNION (query2) order by ...
1322+
private boolean containsDiffFunctionInQuery(Query node) {
1323+
return getSortItemsFromOrderBy(node.getOrderBy()).stream()
1324+
.map(SortItem::getSortKey)
1325+
.anyMatch(expression -> containsDiffFunction(expression));
1326+
}
1327+
1328+
private boolean containsDiffFunctionInQuerySpecification(QuerySpecification node) {
1329+
for (SelectItem selectItem : node.getSelect().getSelectItems()) {
1330+
if (selectItem instanceof AllColumns) {
1331+
Optional<Expression> target = ((AllColumns) selectItem).getTarget();
1332+
if (target.isPresent() && containsDiffFunction(target.get())) {
1333+
return true;
1334+
}
1335+
} else if (selectItem instanceof SingleColumn
1336+
&& containsDiffFunction(((SingleColumn) selectItem).getExpression())) {
1337+
return true;
1338+
}
1339+
}
1340+
1341+
if (node.getWhere().isPresent() && containsDiffFunction(node.getWhere().get())) {
1342+
return true;
1343+
}
1344+
if (node.getHaving().isPresent() && containsDiffFunction(node.getHaving().get())) {
1345+
return true;
1346+
}
1347+
return getSortItemsFromOrderBy(node.getOrderBy()).stream()
1348+
.map(SortItem::getSortKey)
1349+
.anyMatch(expression -> containsDiffFunction(expression));
1350+
}
1351+
13041352
private void resolveFunctionCallAndMeasureWindows(QuerySpecification querySpecification) {
13051353
ImmutableList.Builder<Expression> expressions = ImmutableList.builder();
13061354

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/distribute/TableDistributedPlanGenerator.java‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@
107107
import org.apache.iotdb.db.queryengine.plan.relational.planner.node.schema.TableDeviceQueryCountNode;
108108
import org.apache.iotdb.db.queryengine.plan.relational.planner.node.schema.TableDeviceQueryScanNode;
109109
import org.apache.iotdb.db.queryengine.plan.relational.planner.optimizations.DataNodeLocationSupplierFactory;
110-
import org.apache.iotdb.db.queryengine.plan.relational.planner.optimizations.PushPredicateIntoTableScan;
111110
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.Insert;
112111
import org.apache.iotdb.db.queryengine.plan.statement.component.Ordering;
113112
import org.apache.iotdb.db.schemaengine.table.DataNodeTableCache;
@@ -321,7 +320,22 @@ public List<PlanNode> visitOffset(OffsetNode node, PlanContext context) {
321320

322321
@Override
323322
public List<PlanNode> visitProject(ProjectNode node, PlanContext context) {
323+
boolean containsDiff =
324+
node.getAssignments().getMap().values().stream()
325+
.anyMatch(expression -> containsDiffFunction(expression));
326+
OrderingScheme originalExpectedOrdering = context.expectedOrderingScheme;
327+
boolean originalHasSortProperty = context.hasSortProperty;
328+
if (containsDiff) {
329+
context.clearExpectedOrderingScheme();
330+
}
324331
List<PlanNode> childrenNodes = node.getChild().accept(this, context);
332+
if (containsDiff) {
333+
if (originalHasSortProperty) {
334+
context.setExpectedOrderingScheme(originalExpectedOrdering);
335+
} else {
336+
context.clearExpectedOrderingScheme();
337+
}
338+
}
325339
OrderingScheme childOrdering = nodeOrderingMap.get(childrenNodes.get(0).getPlanNodeId());
326340
boolean containAllSortItem = false;
327341
if (childOrdering != null) {
@@ -378,9 +392,6 @@ public List<PlanNode> visitProject(ProjectNode node, PlanContext context) {
378392
return Collections.singletonList(node);
379393
}
380394

381-
boolean containsDiff =
382-
node.getAssignments().getMap().values().stream()
383-
.anyMatch(PushPredicateIntoTableScan::containsDiffFunction);
384395
if (containsDiff) {
385396
if (containAllSortItem) {
386397
nodeOrderingMap.put(node.getPlanNodeId(), childOrdering);
@@ -583,7 +594,20 @@ public List<PlanNode> visitStreamSort(StreamSortNode node, PlanContext context)
583594

584595
@Override
585596
public List<PlanNode> visitFilter(FilterNode node, PlanContext context) {
597+
boolean containsDiff = containsDiffFunction(node.getPredicate());
598+
OrderingScheme originalExpectedOrdering = context.expectedOrderingScheme;
599+
boolean originalHasSortProperty = context.hasSortProperty;
600+
if (containsDiff) {
601+
context.clearExpectedOrderingScheme();
602+
}
586603
List<PlanNode> childrenNodes = node.getChild().accept(this, context);
604+
if (containsDiff) {
605+
if (originalHasSortProperty) {
606+
context.setExpectedOrderingScheme(originalExpectedOrdering);
607+
} else {
608+
context.clearExpectedOrderingScheme();
609+
}
610+
}
587611
OrderingScheme childOrdering = nodeOrderingMap.get(childrenNodes.get(0).getPlanNodeId());
588612
if (childOrdering != null) {
589613
nodeOrderingMap.put(node.getPlanNodeId(), childOrdering);
@@ -594,7 +618,7 @@ public List<PlanNode> visitFilter(FilterNode node, PlanContext context) {
594618
return Collections.singletonList(node);
595619
}
596620

597-
if (containsDiffFunction(node.getPredicate())) {
621+
if (containsDiff) {
598622
node.setChild(mergeChildrenViaCollectOrMergeSort(childOrdering, childrenNodes));
599623
return Collections.singletonList(node);
600624
}

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/planner/optimizations/PushPredicateIntoTableScan.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1453,7 +1453,7 @@ public static boolean containsDiffFunction(Expression expression) {
14531453

14541454
if (!expression.getChildren().isEmpty()) {
14551455
for (Node node : expression.getChildren()) {
1456-
if (containsDiffFunction((Expression) node)) {
1456+
if (node instanceof Expression && containsDiffFunction((Expression) node)) {
14571457
return true;
14581458
}
14591459
}

‎iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/analyzer/AnalyzerTest.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import org.apache.iotdb.db.queryengine.plan.relational.metadata.OperatorNotFoundException;
6666
import org.apache.iotdb.db.queryengine.plan.relational.metadata.QualifiedObjectName;
6767
import org.apache.iotdb.db.queryengine.plan.relational.metadata.fetcher.TableHeaderSchemaValidator;
68+
import org.apache.iotdb.db.queryengine.plan.relational.planner.PlanTester;
6869
import org.apache.iotdb.db.queryengine.plan.relational.planner.SymbolAllocator;
6970
import org.apache.iotdb.db.queryengine.plan.relational.planner.TableLogicalPlanner;
7071
import org.apache.iotdb.db.queryengine.plan.relational.planner.distribute.TableDistributedPlanner;
@@ -108,6 +109,15 @@
108109
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.TestUtils.TEST_MATADATA;
109110
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.TestUtils.assertTableScanWithoutEntryOrder;
110111
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.TestUtils.getChildrenNode;
112+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanAssert.assertPlan;
113+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.aggregation;
114+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.exchange;
115+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.filter;
116+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.mergeSort;
117+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.output;
118+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.project;
119+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.sort;
120+
import static org.apache.iotdb.db.queryengine.plan.relational.planner.assertions.PlanMatchPattern.tableScan;
111121
import static org.apache.iotdb.db.queryengine.plan.statement.component.Ordering.ASC;
112122
import static org.apache.tsfile.read.common.type.BooleanType.BOOLEAN;
113123
import static org.apache.tsfile.read.common.type.DoubleType.DOUBLE;
@@ -854,6 +864,46 @@ public void diffTest() {
854864
filterNode.getPredicate().toString());
855865
}
856866

867+
@Test
868+
public void diffWithSubqueryOrderByTest() {
869+
PlanTester planTester = new PlanTester();
870+
String sqlWithOuterWhere =
871+
"SELECT time, tag1, s1 FROM ("
872+
+ "select * from table1 ORDER by time"
873+
+ ") WHERE diff(s1) IS NOT NULL ORDER by time DESC";
874+
planTester.createPlan(sqlWithOuterWhere);
875+
assertPlan(
876+
planTester.getFragmentPlan(0),
877+
output(sort(filter(mergeSort(exchange(), exchange(), exchange())))));
878+
assertPlan(planTester.getFragmentPlan(1), sort(tableScan("testdb.table1")));
879+
assertPlan(planTester.getFragmentPlan(2), sort(tableScan("testdb.table1")));
880+
assertPlan(planTester.getFragmentPlan(3), sort(tableScan("testdb.table1")));
881+
882+
String sqlWithOuterHaving =
883+
"SELECT time, tag1, s1 FROM ("
884+
+ "select * from table1 ORDER by time"
885+
+ ") GROUP BY time, tag1, s1 HAVING diff(s1) IS NOT NULL ORDER by time DESC";
886+
planTester.createPlan(sqlWithOuterHaving);
887+
assertPlan(
888+
planTester.getFragmentPlan(0),
889+
output(sort(filter(aggregation(mergeSort(exchange(), exchange(), exchange()))))));
890+
assertPlan(planTester.getFragmentPlan(1), sort(tableScan("testdb.table1")));
891+
assertPlan(planTester.getFragmentPlan(2), sort(tableScan("testdb.table1")));
892+
assertPlan(planTester.getFragmentPlan(3), sort(tableScan("testdb.table1")));
893+
894+
String sqlWithOuterOrderBy =
895+
"SELECT time, tag1, s1 FROM ("
896+
+ "select * from table1 ORDER by time"
897+
+ ") ORDER by coalesce(diff(s1), -1000.0) DESC, time DESC";
898+
planTester.createPlan(sqlWithOuterOrderBy);
899+
assertPlan(
900+
planTester.getFragmentPlan(0),
901+
output(project(sort(project(mergeSort(exchange(), exchange(), exchange()))))));
902+
assertPlan(planTester.getFragmentPlan(1), sort(tableScan("testdb.table1")));
903+
assertPlan(planTester.getFragmentPlan(2), sort(tableScan("testdb.table1")));
904+
assertPlan(planTester.getFragmentPlan(3), sort(tableScan("testdb.table1")));
905+
}
906+
857907
@Test
858908
public void predicatePushDownTest() {
859909
sql =

0 commit comments

Comments
 (0)