This is an automated email from the ASF dual-hosted git repository. xiangweiwei pushed a commit to branch iotdb-1303 in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit d4e3053bde0fc8c5097e35b4d3af57cd5d4c6fc5 Author: Alima777 <[email protected]> AuthorDate: Thu Apr 15 15:41:42 2021 +0800 Fix bug in iotdb-1303 --- .../apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java | 9 ++ .../iotdb/db/qp/strategy/PhysicalGenerator.java | 49 +++++------ .../iotdb/db/integration/IOTDBGroupByIT.java | 19 +++++ .../iotdb/db/integration/IoTDBGroupByFillIT.java | 97 +++++++++++----------- .../aggregation/IoTDBAggregationByLevelIT.java | 21 +++++ 5 files changed, 118 insertions(+), 77 deletions(-) diff --git a/server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java b/server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java index cfd923b..0f4cff6 100644 --- a/server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java +++ b/server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java @@ -1337,6 +1337,9 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> { } public void parseGroupByLevelClause(GroupByLevelClauseContext ctx, QueryOperator queryOp) { + if (!queryOp.getSelectOperator().hasAggregation()) { + throw new SQLParserException("There is no aggregation function with group by query"); + } queryOp.setGroupByLevel(true); queryOp.setLevel(Integer.parseInt(ctx.INT().getText())); } @@ -1428,6 +1431,9 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> { } private void parseGroupByTimeClause(GroupByTimeClauseContext ctx, QueryOperator queryOp) { + if (!queryOp.getSelectOperator().hasAggregation()) { + throw new SQLParserException("There is no aggregation function with group by query"); + } queryOp.setGroupByTime(true); queryOp.setLeftCRightO(ctx.timeInterval().LS_BRACKET() != null); // parse timeUnit @@ -1453,6 +1459,9 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> { } private void parseGroupByFillClause(GroupByFillClauseContext ctx, QueryOperator queryOp) { + if (!queryOp.getSelectOperator().hasAggregation()) { + throw new SQLParserException("There is no aggregation function with group by query"); + } queryOp.setGroupByTime(true); queryOp.setFill(true); queryOp.setLeftCRightO(ctx.timeInterval().LS_BRACKET() != null); diff --git a/server/src/main/java/org/apache/iotdb/db/qp/strategy/PhysicalGenerator.java b/server/src/main/java/org/apache/iotdb/db/qp/strategy/PhysicalGenerator.java index dd514d3..77171cc 100644 --- a/server/src/main/java/org/apache/iotdb/db/qp/strategy/PhysicalGenerator.java +++ b/server/src/main/java/org/apache/iotdb/db/qp/strategy/PhysicalGenerator.java @@ -255,8 +255,7 @@ public class PhysicalGenerator { TracingOperator tracingOperator = (TracingOperator) operator; return new TracingPlan(tracingOperator.isTracingOn()); case QUERY: - QueryOperator query = (QueryOperator) operator; - return transformQuery(query, fetchSize); + return transformQuery((QueryOperator) operator); case TTL: switch (operator.getTokenIntType()) { case SQLConstant.TOK_SET: @@ -435,15 +434,14 @@ public class PhysicalGenerator { } interface Transfrom { - QueryPlan transform(QueryOperator queryOperator, int fetchSize) throws QueryProcessException; + QueryPlan transform(QueryOperator queryOperator) throws QueryProcessException; } /** agg physical plan transform */ public static class AggPhysicalPlanRule implements Transfrom { @Override - public QueryPlan transform(QueryOperator queryOperator, int fetchSize) - throws QueryProcessException { + public QueryPlan transform(QueryOperator queryOperator) throws QueryProcessException { QueryPlan queryPlan; if (queryOperator.hasUdf()) { throw new QueryProcessException( @@ -460,17 +458,18 @@ public class PhysicalGenerator { .setAggregations(queryOperator.getSelectOperator().getAggregations()); if (queryOperator.isGroupByTime()) { - ((GroupByTimePlan) queryPlan).setInterval(queryOperator.getUnit()); - ((GroupByTimePlan) queryPlan).setIntervalByMonth(queryOperator.isIntervalByMonth()); - ((GroupByTimePlan) queryPlan).setSlidingStep(queryOperator.getSlidingStep()); - ((GroupByTimePlan) queryPlan).setSlidingStepByMonth(queryOperator.isSlidingStepByMonth()); - ((GroupByTimePlan) queryPlan).setLeftCRightO(queryOperator.isLeftCRightO()); + GroupByTimePlan groupByTimePlan = (GroupByTimePlan) queryPlan; + groupByTimePlan.setInterval(queryOperator.getUnit()); + groupByTimePlan.setIntervalByMonth(queryOperator.isIntervalByMonth()); + groupByTimePlan.setSlidingStep(queryOperator.getSlidingStep()); + groupByTimePlan.setSlidingStepByMonth(queryOperator.isSlidingStepByMonth()); + groupByTimePlan.setLeftCRightO(queryOperator.isLeftCRightO()); if (!queryOperator.isLeftCRightO()) { - ((GroupByTimePlan) queryPlan).setStartTime(queryOperator.getStartTime() + 1); - ((GroupByTimePlan) queryPlan).setEndTime(queryOperator.getEndTime() + 1); + groupByTimePlan.setStartTime(queryOperator.getStartTime() + 1); + groupByTimePlan.setEndTime(queryOperator.getEndTime() + 1); } else { - ((GroupByTimePlan) queryPlan).setStartTime(queryOperator.getStartTime()); - ((GroupByTimePlan) queryPlan).setEndTime(queryOperator.getEndTime()); + groupByTimePlan.setStartTime(queryOperator.getStartTime()); + groupByTimePlan.setEndTime(queryOperator.getEndTime()); } } if (queryOperator.isFill()) { @@ -498,33 +497,30 @@ public class PhysicalGenerator { public static class FillPhysicalPlanRule implements Transfrom { @Override - public QueryPlan transform(QueryOperator queryOperator, int fetchSize) - throws QueryProcessException { - QueryPlan queryPlan; + public QueryPlan transform(QueryOperator queryOperator) throws QueryProcessException { if (queryOperator.hasUdf()) { throw new QueryProcessException("Fill functions are not supported in UDF queries."); } - queryPlan = new FillQueryPlan(); + FillQueryPlan queryPlan = new FillQueryPlan(); FilterOperator timeFilter = queryOperator.getFilterOperator(); if (!timeFilter.isSingle()) { throw new QueryProcessException("Slice query must select a single time point"); } long time = Long.parseLong(((BasicFunctionOperator) timeFilter).getValue()); - ((FillQueryPlan) queryPlan).setQueryTime(time); - ((FillQueryPlan) queryPlan).setFillType(queryOperator.getFillTypes()); + queryPlan.setQueryTime(time); + queryPlan.setFillType(queryOperator.getFillTypes()); return queryPlan; } } @SuppressWarnings("squid:S3776") // Suppress high Cognitive Complexity warning - private PhysicalPlan transformQuery(QueryOperator queryOperator, int fetchSize) - throws QueryProcessException { + private PhysicalPlan transformQuery(QueryOperator queryOperator) throws QueryProcessException { QueryPlan queryPlan = null; if (queryOperator.hasAggregation()) { - queryPlan = new AggPhysicalPlanRule().transform(queryOperator, fetchSize); + queryPlan = new AggPhysicalPlanRule().transform(queryOperator); } else if (queryOperator.isFill()) { - queryPlan = new FillPhysicalPlanRule().transform(queryOperator, fetchSize); + queryPlan = new FillPhysicalPlanRule().transform(queryOperator); } else if (queryOperator.isLastQuery()) { queryPlan = new LastQueryPlan(); } else if (queryOperator.getIndexType() != null) { @@ -537,7 +533,7 @@ public class PhysicalGenerator { } if (queryOperator.isAlignByDevice()) { - queryPlan = getAlignQueryPlan(queryOperator, fetchSize, queryPlan); + queryPlan = getAlignQueryPlan(queryOperator, queryPlan); } else { queryPlan.setPaths(queryOperator.getSelectedPaths()); // Last query result set will not be affected by alignment @@ -587,8 +583,7 @@ public class PhysicalGenerator { } @SuppressWarnings("squid:S3777") // Suppress high Cognitive Complexity warning - private QueryPlan getAlignQueryPlan( - QueryOperator queryOperator, int fetchSize, QueryPlan queryPlan) + private QueryPlan getAlignQueryPlan(QueryOperator queryOperator, QueryPlan queryPlan) throws QueryProcessException { // below is the core realization of ALIGN_BY_DEVICE sql logic AlignByDevicePlan alignByDevicePlan = new AlignByDevicePlan(); diff --git a/server/src/test/java/org/apache/iotdb/db/integration/IOTDBGroupByIT.java b/server/src/test/java/org/apache/iotdb/db/integration/IOTDBGroupByIT.java index 3d16ddf..b9d0bd7c 100644 --- a/server/src/test/java/org/apache/iotdb/db/integration/IOTDBGroupByIT.java +++ b/server/src/test/java/org/apache/iotdb/db/integration/IOTDBGroupByIT.java @@ -931,6 +931,25 @@ public class IOTDBGroupByIT { } } + /** + * Test group by without aggregation function used in select clause. The expected situation is + * throwing an exception. + */ + @Test + public void TestGroupByWithoutAggregationFunc() { + try (Connection connection = + DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root"); + Statement statement = connection.createStatement()) { + + statement.execute("select temperature from root.ln.wf01.wt01 group by ([0, 100), 5ms)"); + + fail("No expected exception thrown"); + } catch (Exception e) { + Assert.assertTrue( + e.getMessage().contains("There is no aggregation function with group by query")); + } + } + private void prepareData() { try (Connection connection = DriverManager.getConnection( diff --git a/server/src/test/java/org/apache/iotdb/db/integration/IoTDBGroupByFillIT.java b/server/src/test/java/org/apache/iotdb/db/integration/IoTDBGroupByFillIT.java index 0257e0b..3bec6e1 100644 --- a/server/src/test/java/org/apache/iotdb/db/integration/IoTDBGroupByFillIT.java +++ b/server/src/test/java/org/apache/iotdb/db/integration/IoTDBGroupByFillIT.java @@ -24,6 +24,7 @@ import org.apache.iotdb.jdbc.Config; import org.apache.iotdb.jdbc.IoTDBSQLException; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -90,8 +91,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previous])"); assertTrue(hasResultSet); @@ -111,8 +111,7 @@ public class IoTDBGroupByFillIT { hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previous]) order by time desc"); assertTrue(hasResultSet); @@ -141,8 +140,7 @@ public class IoTDBGroupByFillIT { DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root"); Statement statement = connection.createStatement()) { statement.execute( - "select count(temperature) from " - + "root.ln.wf01.wt01 " + "select count(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previous])"); } catch (IoTDBSQLException e) { assertTrue(e.getMessage().contains("Group By Fill only support last_value function")); @@ -155,8 +153,7 @@ public class IoTDBGroupByFillIT { DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root"); Statement statement = connection.createStatement()) { statement.execute( - "select count(temperature) from " - + "root.ln.wf01.wt01 " + "select count(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previous]) order by time desc"); } catch (IoTDBSQLException e) { assertTrue(e.getMessage().contains("Group By Fill only support last_value function")); @@ -178,8 +175,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(int32[previous])"); assertTrue(hasResultSet); @@ -199,8 +195,7 @@ public class IoTDBGroupByFillIT { hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(int32[previous]) order by time desc"); assertTrue(hasResultSet); @@ -244,8 +239,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature), last_value(hardware) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature), last_value(hardware) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(int32[previous], double[previous])"); assertTrue(hasResultSet); @@ -267,8 +261,7 @@ public class IoTDBGroupByFillIT { hasResultSet = statement.execute( - "select last_value(temperature), last_value(hardware) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature), last_value(hardware) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(int32[previous], double[previous]) order by time desc"); assertTrue(hasResultSet); @@ -305,8 +298,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ((5, 40], 5ms) FILL(int32[previous])"); assertTrue(hasResultSet); @@ -326,8 +318,7 @@ public class IoTDBGroupByFillIT { hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ((5, 40], 5ms) FILL(int32[previous]) order by time desc"); assertTrue(hasResultSet); @@ -370,8 +361,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature), last_value(hardware) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature), last_value(hardware) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(ALL[previous])"); assertTrue(hasResultSet); @@ -393,8 +383,7 @@ public class IoTDBGroupByFillIT { hasResultSet = statement.execute( - "select last_value(temperature), last_value(hardware) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature), last_value(hardware) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(ALL[previous]) order by time desc"); assertTrue(hasResultSet); @@ -430,8 +419,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previousUntilLast])"); assertTrue(hasResultSet); @@ -450,8 +438,7 @@ public class IoTDBGroupByFillIT { } hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previousUntilLast]) order by time desc"); assertTrue(hasResultSet); @@ -479,8 +466,7 @@ public class IoTDBGroupByFillIT { DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root"); Statement statement = connection.createStatement()) { statement.execute( - "select count(temperature) from " - + "root.ln.wf01.wt01 " + "select count(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previousUntilLast])"); } catch (IoTDBSQLException e) { System.out.println(e.getMessage()); @@ -494,8 +480,7 @@ public class IoTDBGroupByFillIT { DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root"); Statement statement = connection.createStatement()) { statement.execute( - "select count(temperature) from " - + "root.ln.wf01.wt01 " + "select count(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previousUntilLast]) order by time desc"); } catch (IoTDBSQLException e) { System.out.println(e.getMessage()); @@ -519,8 +504,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(int32[previousUntilLast])"); assertTrue(hasResultSet); @@ -540,8 +524,7 @@ public class IoTDBGroupByFillIT { hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(int32[previousUntilLast])order by time desc"); assertTrue(hasResultSet); @@ -584,8 +567,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature), last_value(hardware) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature), last_value(hardware) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(int32[previousUntilLast], double[previousUntilLast])"); assertTrue(hasResultSet); @@ -607,8 +589,7 @@ public class IoTDBGroupByFillIT { hasResultSet = statement.execute( - "select last_value(temperature), last_value(hardware) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature), last_value(hardware) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(int32[previousUntilLast], double[previousUntilLast]) order by time desc"); assertTrue(hasResultSet); @@ -644,8 +625,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(float[previousUntilLast])"); assertTrue(hasResultSet); @@ -665,8 +645,7 @@ public class IoTDBGroupByFillIT { hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(float[previousUntilLast]) order by time desc"); assertTrue(hasResultSet); @@ -699,8 +678,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ((4, 44], 5ms) FILL(int32[previousUntilLast])"); assertTrue(hasResultSet); @@ -745,8 +723,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature), last_value(hardware) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature), last_value(hardware) from root.ln.wf01.wt01 " + "GROUP BY ([2, 48), 5ms) FILL(ALL[previousUntilLast])"); assertTrue(hasResultSet); @@ -785,8 +762,7 @@ public class IoTDBGroupByFillIT { Statement statement = connection.createStatement()) { boolean hasResultSet = statement.execute( - "select last_value(temperature) from " - + "root.ln.wf01.wt01 " + "select last_value(temperature) from root.ln.wf01.wt01 " + "GROUP BY ([17, 48), 5ms) FILL(int32[previous]) " + "limit 5 offset 2"); @@ -811,6 +787,27 @@ public class IoTDBGroupByFillIT { } } + /** + * Test group by fill without aggregation function used in select clause. The expected situation + * is throwing an exception. + */ + @Test + public void TestGroupByFillWithoutAggregationFunc() { + try (Connection connection = + DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root"); + Statement statement = connection.createStatement()) { + + statement.execute( + "select temperature from root.ln.wf01.wt01 " + + "group by ([0, 100), 5ms) FILL(int32[previous])"); + + fail("No expected exception thrown"); + } catch (Exception e) { + Assert.assertTrue( + e.getMessage().contains("There is no aggregation function with group by query")); + } + } + private void prepareData() { try (Connection connection = DriverManager.getConnection( diff --git a/server/src/test/java/org/apache/iotdb/db/integration/aggregation/IoTDBAggregationByLevelIT.java b/server/src/test/java/org/apache/iotdb/db/integration/aggregation/IoTDBAggregationByLevelIT.java index c1a7e14..802e077 100644 --- a/server/src/test/java/org/apache/iotdb/db/integration/aggregation/IoTDBAggregationByLevelIT.java +++ b/server/src/test/java/org/apache/iotdb/db/integration/aggregation/IoTDBAggregationByLevelIT.java @@ -33,6 +33,8 @@ import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.Statement; +import static org.junit.Assert.fail; + public class IoTDBAggregationByLevelIT { private Planner planner = new Planner(); @@ -322,6 +324,25 @@ public class IoTDBAggregationByLevelIT { } } + /** + * Test group by level without aggregation function used in select clause. The expected situation + * is throwing an exception. + */ + @Test + public void TestGroupByLevelWithoutAggregationFunc() { + try (Connection connection = + DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root"); + Statement statement = connection.createStatement()) { + + statement.execute("select temperature from root.sg1.* group by level = 2"); + + fail("No expected exception thrown"); + } catch (Exception e) { + Assert.assertTrue( + e.getMessage().contains("There is no aggregation function with group by query")); + } + } + private void prepareData() { try (Connection connection = DriverManager.getConnection(
