You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by er...@apache.org on 2022/10/26 01:23:25 UTC
[iotdb] branch master updated: [IOTDB-4751] Fix GROUP BY TAGS being without any error message (#7720)
This is an automated email from the ASF dual-hosted git repository.
ericpai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iotdb.git
The following commit(s) were added to refs/heads/master by this push:
new 8a68f3a8ce [IOTDB-4751] Fix GROUP BY TAGS being without any error message (#7720)
8a68f3a8ce is described below
commit 8a68f3a8ce8d38de75912fc8ec0e8daf6db62bf7
Author: BaiJian <er...@hotmail.com>
AuthorDate: Wed Oct 26 09:23:19 2022 +0800
[IOTDB-4751] Fix GROUP BY TAGS being without any error message (#7720)
---
.../org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4 | 2 +-
.../db/it/aggregation/IoTDBTagAggregationIT.java | 16 +++++++++
.../iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java | 8 ++++-
.../iotdb/db/mpp/plan/parser/ASTVisitor.java | 10 ++++++
.../db/mpp/plan/statement/crud/QueryStatement.java | 19 +++++++---
.../iotdb/db/mpp/plan/plan/LogicalPlannerTest.java | 40 ++++++++++++++++++++++
6 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4 b/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4
index 3bc96c4577..16493e5f87 100644
--- a/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4
+++ b/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4
@@ -428,7 +428,7 @@ specialClause
| groupByTimeClause havingClause? orderByClause? specialLimit? #groupByTimeStatement
| groupByFillClause havingClause? orderByClause? specialLimit? #groupByFillStatement
| groupByLevelClause havingClause? orderByClause? specialLimit? #groupByLevelStatement
- | groupByTagClause orderByClause? #groupByTagStatement
+ | groupByTagClause havingClause? orderByClause? specialLimit? #groupByTagStatement
| fillClause orderByClause? specialLimit? #fillStatement
;
diff --git a/integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBTagAggregationIT.java b/integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBTagAggregationIT.java
index e679657064..695bae88d3 100644
--- a/integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBTagAggregationIT.java
+++ b/integration-test/src/test/java/org/apache/iotdb/db/it/aggregation/IoTDBTagAggregationIT.java
@@ -511,4 +511,20 @@ public class IoTDBTagAggregationIT {
e.getMessage().contains("Only time filters are supported in GROUP BY TAGS query"));
}
}
+
+ @Test
+ public void testWithHaving() {
+ String query =
+ "SELECT COUNT(t) from root.sg.** GROUP BY ([0, 20), 10ms), TAGS(k1) HAVING COUNT(t) > 3";
+ // Having is not supported yet
+ try (Connection connection = EnvFactory.getEnv().getConnection();
+ Statement statement = connection.createStatement()) {
+ try (ResultSet ignored = statement.executeQuery(query)) {
+ Assert.fail();
+ }
+ } catch (SQLException e) {
+ Assert.assertTrue(
+ e.getMessage().contains("Having clause is not supported yet in GROUP BY TAGS query"));
+ }
+ }
}
diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java
index 925d5558ec..144cd3dcb3 100644
--- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java
+++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/AnalyzeVisitor.java
@@ -381,7 +381,10 @@ public class AnalyzeVisitor extends StatementVisitor<Analysis, MPPQueryContext>
boolean hasAlias = resultColumn.hasAlias();
List<Expression> resultExpressions =
ExpressionAnalyzer.removeWildcardInExpression(resultColumn.getExpression(), schemaTree);
- if (hasAlias && !queryStatement.isGroupByLevel() && resultExpressions.size() > 1) {
+ if (hasAlias
+ && !queryStatement.isGroupByLevel()
+ && !queryStatement.isGroupByTag()
+ && resultExpressions.size() > 1) {
throw new SemanticException(
String.format(
"alias '%s' can only be matched with one time series", resultColumn.getAlias()));
@@ -700,6 +703,9 @@ public class AnalyzeVisitor extends StatementVisitor<Analysis, MPPQueryContext>
if (analysis.hasValueFilter()) {
throw new SemanticException("Only time filters are supported in GROUP BY TAGS query");
}
+ if (queryStatement.hasHaving()) {
+ throw new SemanticException("Having clause is not supported yet in GROUP BY TAGS query");
+ }
Map<List<String>, LinkedHashMap<Expression, List<Expression>>>
tagValuesToGroupedTimeseriesOperands = new HashMap<>();
LinkedHashMap<Expression, Set<Expression>> groupByTagOutputExpressions = new LinkedHashMap<>();
diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/plan/parser/ASTVisitor.java b/server/src/main/java/org/apache/iotdb/db/mpp/plan/parser/ASTVisitor.java
index 121817e4f0..3df754f1cc 100644
--- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/parser/ASTVisitor.java
+++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/parser/ASTVisitor.java
@@ -1113,6 +1113,11 @@ public class ASTVisitor extends IoTDBSqlParserBaseVisitor<Statement> {
parseOrderByClause(ctx.orderByClause());
}
+ // parse limit & offset
+ if (ctx.specialLimit() != null) {
+ return visit(ctx.specialLimit());
+ }
+
return queryStatement;
}
@@ -1199,6 +1204,11 @@ public class ASTVisitor extends IoTDBSqlParserBaseVisitor<Statement> {
@Override
public Statement visitLimitStatement(IoTDBSqlParser.LimitStatementContext ctx) {
+ if ((ctx.limitClause() != null || ctx.slimitClause() != null)
+ && queryStatement.isGroupByTag()) {
+ // TODO: I will support limit and slimit in later PRs
+ throw new SemanticException("Limit or slimit are not supported yet in GROUP BY TAGS");
+ }
// parse LIMIT
parseLimitClause(ctx.limitClause(), queryStatement);
diff --git a/server/src/main/java/org/apache/iotdb/db/mpp/plan/statement/crud/QueryStatement.java b/server/src/main/java/org/apache/iotdb/db/mpp/plan/statement/crud/QueryStatement.java
index ae6586de5f..e894c70d06 100644
--- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/statement/crud/QueryStatement.java
+++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/statement/crud/QueryStatement.java
@@ -314,18 +314,27 @@ public class QueryStatement extends Statement {
throw new SemanticException("AGGREGATION doesn't support disable align clause.");
}
if (isGroupByLevel() && isAlignByDevice()) {
- throw new SemanticException("group by level does not support align by device now.");
+ throw new SemanticException("GROUP BY LEVEL does not support align by device now.");
}
if (isGroupByTag() && isAlignByDevice()) {
- throw new SemanticException("group by tag does not support align by device now.");
- }
- if (isGroupByTag() && isGroupByLevel()) {
- throw new SemanticException("group by level cannot be used togather with group by tag");
+ throw new SemanticException("GROUP BY TAGS does not support align by device now.");
}
+ Set<String> outputColumn = new HashSet<>();
for (ResultColumn resultColumn : selectComponent.getResultColumns()) {
if (resultColumn.getColumnType() != ResultColumn.ColumnType.AGGREGATION) {
throw new SemanticException("Raw data and aggregation hybrid query is not supported.");
}
+ outputColumn.add(
+ resultColumn.getAlias() != null
+ ? resultColumn.getAlias()
+ : resultColumn.getExpression().getExpressionString());
+ }
+ if (isGroupByTag()) {
+ for (String s : getGroupByTagComponent().getTagKeys()) {
+ if (outputColumn.contains(s)) {
+ throw new SemanticException("Output column is duplicated with the tag key: " + s);
+ }
+ }
}
} else {
if (isGroupByTime() || isGroupByLevel()) {
diff --git a/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/LogicalPlannerTest.java b/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/LogicalPlannerTest.java
index fe2b9500a0..617f25d252 100644
--- a/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/LogicalPlannerTest.java
+++ b/server/src/test/java/org/apache/iotdb/db/mpp/plan/plan/LogicalPlannerTest.java
@@ -683,6 +683,46 @@ public class LogicalPlannerTest {
}
}
+ @Test
+ public void testGroupByTagWithIllegalSpecialLimitClause() {
+ String[] inputSql =
+ new String[] {
+ "select max_value(s1) from root.** group by tags(key1) disable align",
+ "select max_value(s1) from root.** group by tags(key1) align by device",
+ "select max_value(s1) from root.** group by tags(key1) without null any",
+ "select max_value(s1) from root.** group by tags(key1) limit 1",
+ "select max_value(s1) from root.** group by([0, 10000), 5ms), tags(key1) limit 1 offset 1 slimit 1 soffset 1"
+ };
+ String[] expectedMsg =
+ new String[] {
+ "AGGREGATION doesn't support disable align clause",
+ "GROUP BY TAGS does not support align by device now",
+ "WITHOUT NULL clause is not supported yet",
+ "Limit or slimit are not supported yet in GROUP BY TAGS",
+ "Limit or slimit are not supported yet in GROUP BY TAGS",
+ };
+ for (int i = 0; i < inputSql.length; i++) {
+ try {
+ parseSQLToPlanNode(inputSql[i]);
+ fail();
+ } catch (Exception e) {
+ Assert.assertTrue(inputSql[i], e.getMessage().contains(expectedMsg[i]));
+ }
+ }
+ }
+
+ @Test
+ public void testGroupByTagWithDuplicatedAliasWithTagKey() {
+ String sql = "select max_value(s1) as key1 from root.** group by tags(key1)";
+ try {
+ parseSQLToPlanNode(sql);
+ fail();
+ } catch (Exception e) {
+ Assert.assertTrue(
+ e.getMessage().contains("Output column is duplicated with the tag key: key1"));
+ }
+ }
+
private PlanNode parseSQLToPlanNode(String sql) {
Statement statement = StatementGenerator.createStatement(sql, ZonedDateTime.now().getOffset());
MPPQueryContext context = new MPPQueryContext(new QueryId("test_query"));