Skip to content

Commit 93c6f92

Browse files
Cool6689Cool6689
committed
Fix window aggregation validation for GROUP BY ALL
1 parent e679720 commit 93c6f92

3 files changed

Lines changed: 115 additions & 30 deletions

File tree

‎integration-test/src/test/java/org/apache/iotdb/relational/it/query/recent/IoTDBGroupByAllTableIT.java‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,45 @@ public void testWindowFunctionValidation() {
176176
retArray,
177177
DATABASE_NAME);
178178

179+
expectedHeader = new String[] {"device_id", "s1"};
180+
retArray = new String[] {"d1,20.0,", "d2,20.0,", "d1,30.0,", "d2,40.0,"};
181+
tableResultSetEqualTest(
182+
"SELECT device_id, s1 FROM table1 GROUP BY ALL ORDER BY rank() OVER (ORDER BY s1), device_id",
183+
expectedHeader,
184+
retArray,
185+
DATABASE_NAME);
186+
179187
tableAssertTestFail(
180188
"SELECT device_id, rank() OVER (ORDER BY s1), avg(s1) FROM table1 GROUP BY ALL",
181-
"must be an aggregate expression or appear in GROUP BY clause",
189+
"ORDER BY expression 's1' must be an aggregate expression or appear in GROUP BY clause",
190+
DATABASE_NAME);
191+
192+
tableAssertTestFail(
193+
"SELECT device_id, count(*) OVER (PARTITION BY s1), avg(s1) FROM table1 GROUP BY ALL",
194+
"PARTITION BY expression 's1' must be an aggregate expression or appear in GROUP BY clause",
195+
DATABASE_NAME);
196+
197+
tableAssertTestFail(
198+
"SELECT device_id, avg(s1) FROM table1 GROUP BY ALL ORDER BY rank() OVER (ORDER BY s1)",
199+
"ORDER BY expression 's1' must be an aggregate expression or appear in GROUP BY clause",
200+
DATABASE_NAME);
201+
202+
tableAssertTestFail(
203+
"SELECT sum(count(*) OVER (PARTITION BY device_id)) FROM table1 GROUP BY ALL",
204+
"Cannot nest window functions inside aggregation",
205+
DATABASE_NAME);
206+
}
207+
208+
@Test
209+
public void testWindowFrameValidation() {
210+
tableAssertTestFail(
211+
"SELECT device_id, count(*) OVER (PARTITION BY device_id ORDER BY device_id ROWS BETWEEN x PRECEDING AND CURRENT ROW), avg(s1) FROM table1 GROUP BY ALL",
212+
"Window frame start must be an aggregate expression or appear in GROUP BY clause",
213+
DATABASE_NAME);
214+
215+
tableAssertTestFail(
216+
"SELECT device_id, count(*) OVER (PARTITION BY device_id ORDER BY device_id ROWS BETWEEN CURRENT ROW AND x FOLLOWING), avg(s1) FROM table1 GROUP BY ALL",
217+
"Window frame end must be an aggregate expression or appear in GROUP BY clause",
182218
DATABASE_NAME);
183219
}
184220

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

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,13 @@
5151
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Row;
5252
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.SearchedCaseExpression;
5353
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.SimpleCaseExpression;
54+
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.SortItem;
5455
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.SubqueryExpression;
5556
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Trim;
5657
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.WhenClause;
58+
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Window;
59+
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.WindowFrame;
60+
import org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.WindowSpecification;
5761
import org.apache.iotdb.db.i18n.DataNodeQueryMessages;
5862
import org.apache.iotdb.db.queryengine.plan.relational.planner.ScopeAware;
5963
import org.apache.iotdb.db.queryengine.plan.relational.sql.ast.AstVisitor;
@@ -73,11 +77,13 @@
7377
import static java.lang.String.format;
7478
import static java.util.Objects.requireNonNull;
7579
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.ExpressionTreeUtils.extractAggregateFunctions;
80+
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.ExpressionTreeUtils.extractWindowExpressions;
7681
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.ExpressionTreeUtils.isAggregationFunction;
7782
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.ScopeReferenceExtractor.getReferencesToScope;
7883
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.ScopeReferenceExtractor.hasReferencesToScope;
7984
import static org.apache.iotdb.db.queryengine.plan.relational.analyzer.ScopeReferenceExtractor.isFieldFromScope;
8085
import static org.apache.iotdb.db.queryengine.plan.relational.planner.ScopeAware.scopeAwareKey;
86+
import static org.apache.iotdb.db.queryengine.plan.relational.utils.NodeUtils.getSortItemsFromOrderBy;
8187

8288
/** Checks whether an expression is constant with respect to the group */
8389
class AggregationAnalyzer {
@@ -288,19 +294,85 @@ public Boolean visitTrim(Trim node, Void context) {
288294
@Override
289295
public Boolean visitFunctionCall(FunctionCall node, Void context) {
290296
if (isAggregationFunction(node.getName().toString())) {
291-
List<FunctionCall> aggregateFunctions = extractAggregateFunctions(node.getArguments());
297+
if (node.getWindow().isEmpty()) {
298+
List<FunctionCall> aggregateFunctions = extractAggregateFunctions(node.getArguments());
299+
List<Expression> windowExpressions = extractWindowExpressions(node.getArguments());
300+
301+
if (!aggregateFunctions.isEmpty()) {
302+
throw new SemanticException(
303+
String.format(
304+
"Cannot nest aggregations inside aggregation '%s': %s",
305+
node.getName(), aggregateFunctions));
306+
}
307+
308+
if (!windowExpressions.isEmpty()) {
309+
throw new SemanticException(
310+
String.format(
311+
"Cannot nest window functions inside aggregation '%s': %s",
312+
node.getName(), windowExpressions));
313+
}
314+
315+
return true;
316+
}
317+
} else {
318+
// Reject FILTER for non-aggregation functions when FunctionCall supports FILTER.
319+
// Reject ORDER BY for non-aggregation functions when FunctionCall supports function-level
320+
// ORDER BY.
321+
}
322+
323+
if (node.getWindow().isPresent()) {
324+
Window window = node.getWindow().get();
325+
if (window instanceof WindowSpecification windowSpecification
326+
&& !process(windowSpecification, context)) {
327+
return false;
328+
}
329+
}
330+
331+
return node.getArguments().stream().allMatch(expression -> process(expression, context));
332+
}
292333

293-
if (!aggregateFunctions.isEmpty()) {
334+
@Override
335+
public Boolean visitWindowSpecification(WindowSpecification node, Void context) {
336+
for (Expression expression : node.getPartitionBy()) {
337+
if (!process(expression, context)) {
294338
throw new SemanticException(
295339
String.format(
296-
"Cannot nest aggregations inside aggregation '%s': %s",
297-
node.getName(), aggregateFunctions));
340+
"PARTITION BY expression '%s' must be an aggregate expression or appear in GROUP BY clause",
341+
expression));
298342
}
343+
}
299344

300-
return true;
345+
for (SortItem sortItem : getSortItemsFromOrderBy(node.getOrderBy())) {
346+
Expression expression = sortItem.getSortKey();
347+
if (!process(expression, context)) {
348+
throw new SemanticException(
349+
String.format(
350+
"ORDER BY expression '%s' must be an aggregate expression or appear in GROUP BY clause",
351+
expression));
352+
}
301353
}
302354

303-
return node.getArguments().stream().allMatch(expression -> process(expression, context));
355+
if (node.getFrame().isPresent()) {
356+
process(node.getFrame().get(), context);
357+
}
358+
359+
return true;
360+
}
361+
362+
@Override
363+
public Boolean visitWindowFrame(WindowFrame node, Void context) {
364+
if (node.getStart().getValue().isPresent()
365+
&& !process(node.getStart().getValue().get(), context)) {
366+
throw new SemanticException(
367+
"Window frame start must be an aggregate expression or appear in GROUP BY clause");
368+
}
369+
if (node.getEnd().isPresent()
370+
&& node.getEnd().get().getValue().isPresent()
371+
&& !process(node.getEnd().get().getValue().get(), context)) {
372+
throw new SemanticException(
373+
"Window frame end must be an aggregate expression or appear in GROUP BY clause");
374+
}
375+
return true;
304376
}
305377

306378
@Override

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

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,9 +2880,6 @@ private Analysis.GroupingSetAnalysis analyzeGroupBy(
28802880
"%s is not comparable, and therefore cannot be used in GROUP BY", type));
28812881
}
28822882
}
2883-
if (groupBy.isAll()) {
2884-
validateAllGroupByWindowFunctions(allGroupByExpressions, scope, outputExpressions);
2885-
}
28862883

28872884
Analysis.GroupingSetAnalysis groupingSets =
28882885
new Analysis.GroupingSetAnalysis(
@@ -2944,26 +2941,6 @@ && extractWindowFunctions(ImmutableList.of(expression)).isEmpty()) {
29442941
return groupingExpressions.build();
29452942
}
29462943

2947-
private void validateAllGroupByWindowFunctions(
2948-
List<Expression> groupingExpressions, Scope scope, List<Expression> outputExpressions) {
2949-
List<FunctionCall> windowFunctions = extractWindowFunctions(outputExpressions);
2950-
for (FunctionCall windowFunction : windowFunctions) {
2951-
Analysis.ResolvedWindow window = analysis.getWindow(windowFunction);
2952-
ImmutableList.Builder<Expression> expressions = ImmutableList.builder();
2953-
expressions.addAll(windowFunction.getArguments());
2954-
expressions.addAll(window.getPartitionBy());
2955-
getSortItemsFromOrderBy(window.getOrderBy()).stream()
2956-
.map(SortItem::getSortKey)
2957-
.forEach(expressions::add);
2958-
if (window.getFrame().isPresent()) {
2959-
WindowFrame frame = window.getFrame().get();
2960-
frame.getStart().getValue().ifPresent(expressions::add);
2961-
frame.getEnd().flatMap(FrameBound::getValue).ifPresent(expressions::add);
2962-
}
2963-
verifySourceAggregations(groupingExpressions, scope, expressions.build(), analysis);
2964-
}
2965-
}
2966-
29672944
private boolean isDateBinGapFill(Expression column) {
29682945
return column instanceof FunctionCall
29692946
&& DATE_BIN

0 commit comments

Comments
 (0)