This is an automated email from the ASF dual-hosted git repository. caogaofei pushed a commit to branch 1.2_fix_having_bug in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit 4af0b324e9f7570168bbe9025dbf444e92e4059c Author: Beyyes <[email protected]> AuthorDate: Thu Jun 29 14:54:58 2023 +0800 fix sql error in having and align by device --- .../db/mpp/plan/statement/crud/QueryStatement.java | 19 ++- .../db/mpp/plan/statement/QueryStatementTest.java | 132 +++++++++++++++++++++ 2 files changed, 150 insertions(+), 1 deletion(-) 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 3b7fe31c517..c375c24eee3 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 @@ -557,8 +557,13 @@ public class QueryStatement extends Statement { != ResultColumn.ColumnType.AGGREGATION) { throw new SemanticException("Expression of HAVING clause must to be an Aggregation"); } + if (!isAggregationQuery()) { + throw new SemanticException( + "Expression of HAVING clause can not be used in NonAggregationQuery"); + } try { - if (isGroupByLevel()) { // check path in SELECT and HAVING only have one node + if (isGroupByLevel()) { + // check path in SELECT and HAVING only have one node for (ResultColumn resultColumn : getSelectComponent().getResultColumns()) { ExpressionAnalyzer.checkIsAllMeasurement(resultColumn.getExpression()); } @@ -575,9 +580,21 @@ public class QueryStatement extends Statement { for (ResultColumn resultColumn : selectComponent.getResultColumns()) { ExpressionAnalyzer.checkIsAllMeasurement(resultColumn.getExpression()); } + if (hasGroupByExpression()) { + ExpressionAnalyzer.checkIsAllMeasurement( + getGroupByComponent().getControlColumnExpression()); + } + if (hasOrderByExpression()) { + for (Expression expression : getExpressionSortItemList()) { + ExpressionAnalyzer.checkIsAllMeasurement(expression); + } + } if (getWhereCondition() != null) { ExpressionAnalyzer.checkIsAllMeasurement(getWhereCondition().getPredicate()); } + if (hasHaving()) { + ExpressionAnalyzer.checkIsAllMeasurement(getHavingCondition().getPredicate()); + } } catch (SemanticException e) { throw new SemanticException("ALIGN BY DEVICE: " + e.getMessage()); } diff --git a/server/src/test/java/org/apache/iotdb/db/mpp/plan/statement/QueryStatementTest.java b/server/src/test/java/org/apache/iotdb/db/mpp/plan/statement/QueryStatementTest.java new file mode 100644 index 00000000000..6d358224561 --- /dev/null +++ b/server/src/test/java/org/apache/iotdb/db/mpp/plan/statement/QueryStatementTest.java @@ -0,0 +1,132 @@ +package org.apache.iotdb.db.mpp.plan.statement; + +import org.apache.iotdb.db.exception.sql.SemanticException; +import org.apache.iotdb.db.mpp.plan.parser.StatementGenerator; +import org.apache.iotdb.db.mpp.plan.statement.crud.QueryStatement; +import org.apache.iotdb.tsfile.utils.Pair; + +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.time.ZonedDateTime; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +public class QueryStatementTest { + + private static final Logger logger = LoggerFactory.getLogger(QueryStatementTest.class); + + private static final String ALIGN_BY_DEVICE_ONE_LEVEL_ERROR = + "ALIGN BY DEVICE: the suffix paths can only be measurement or one-level wildcard"; + + @Test + public void semanticCheckTest() { + List<Pair<String, String>> errorSqlWithMessages = + Arrays.asList( + new Pair<>( + "SELECT s1 FROM root.sg.d1 " + + "GROUP BY ([2017-11-01T00:00:00, 2017-11-07T23:00:00),1d)", + "Common queries and aggregated queries are not allowed to appear at the same time"), + new Pair<>( + "SELECT count(s1),s2 FROM root.sg.d1", + "Raw data and aggregation hybrid query is not supported."), + + // test for where clause + new Pair<>( + "SELECT s1 FROM root.sg.d1 WHERE count(s1) > 0", + "aggregate functions are not supported in WHERE clause"), + + // test for having clause + new Pair<>( + "SELECT s1 FROM root.sg.d1 HAVING(s1 > 0)", + "Expression of HAVING clause must to be an Aggregation"), + new Pair<>( + "SELECT s1 FROM root.sg.d1 HAVING(count(s1) > 0)", + "Expression of HAVING clause can not be used in NonAggregationQuery"), + new Pair<>( + "SELECT count(d1.s1) FROM root.sg.d1 GROUP BY level=1 HAVING (count(s1) > 0)", + "When Having used with GroupByLevel: " + + "the suffix paths can only be measurement or one-level wildcard"), + new Pair<>( + "SELECT count(s1) FROM root.sg.d1 GROUP BY level=1 HAVING (count(sg.d1.s1) > 0)", + "When Having used with GroupByLevel: " + + "the suffix paths can only be measurement or one-level wildcard"), + + // test for align by device clause + new Pair<>( + "SELECT d1.s1 FROM root.sg.d1 align by device", ALIGN_BY_DEVICE_ONE_LEVEL_ERROR), + new Pair<>( + "SELECT count(s1) FROM root.sg.d1 group by variation(sg.s1) align by device", + ALIGN_BY_DEVICE_ONE_LEVEL_ERROR), + new Pair<>( + "SELECT s1 FROM root.sg.d1 order by root.sg.d1.s1 align by device", + ALIGN_BY_DEVICE_ONE_LEVEL_ERROR), + new Pair<>( + "SELECT s1 FROM root.sg.d1 where root.sg.d1.s1 > 0 align by device", + ALIGN_BY_DEVICE_ONE_LEVEL_ERROR), + new Pair<>( + "SELECT count(s1) FROM root.sg.d1 having(count(root.sg.d1.s1) > 0) align by device", + ALIGN_BY_DEVICE_ONE_LEVEL_ERROR), + new Pair<>( + "SELECT s1 FROM root.sg.d1 order by timeseries align by device", + "Sorting by timeseries is only supported in last queries."), + + // test for last query + new Pair<>( + "SELECT last s1 FROM root.sg.d1 align by device", + "Last query doesn't support align by device."), + new Pair<>( + "SELECT last s1+s2 FROM root.sg.d1", + "Last queries can only be applied on raw time series."), + new Pair<>( + "SELECT last s1 FROM root.sg.d1 order by device", + "Sorting by device is only supported in ALIGN BY DEVICE queries."), + new Pair<>( + "SELECT last s1 FROM root.sg.d1 SLIMIT 1 SOFFSET 2", + "SLIMIT and SOFFSET can not be used in LastQuery."), + + // test for select into clause + new Pair<>( + "SELECT s1 INTO root.sg.d2(t1) FROM root.sg.d1 SLIMIT 5", + "select into: slimit clauses are not supported."), + new Pair<>( + "SELECT s1 INTO root.sg.d2(t1) FROM root.sg.d1 SOFFSET 6", + "select into: soffset clauses are not supported."), + new Pair<>( + "SELECT last s1 INTO root.sg.d2(t1) FROM root.sg.d1", + "select into: last clauses are not supported."), + new Pair<>( + "SELECT count(s1) INTO root.sg.d2(t1) FROM root.sg.d1 GROUP BY TAGS(a)", + "select into: GROUP BY TAGS clause are not supported."), + new Pair<>( + "SELECT s1 FROM root.sg.d1 order by timeseries", + "Sorting by timeseries is only supported in last queries."), + new Pair<>( + "SELECT s1 FROM root.sg.d1 order by device", + "Sorting by device is only supported in ALIGN BY DEVICE queries.")); + + for (Pair<String, String> pair : errorSqlWithMessages) { + String errorSql = pair.left; + String errorMsg = pair.right; + try { + checkErrorQuerySql(errorSql); + } catch (SemanticException e) { + assertEquals(errorMsg, e.getMessage()); + continue; + } catch (Exception ex) { + fail(String.format("Meets exception %s in test sql: `%s`", errorMsg, errorSql)); + } + fail(String.format("Sql: `%s` must throw exception: %s", errorSql, errorMsg)); + } + } + + private void checkErrorQuerySql(String sql) { + QueryStatement statement = + (QueryStatement) StatementGenerator.createStatement(sql, ZonedDateTime.now().getOffset()); + statement.semanticCheck(); + } +}
