[IOTDB-1024] Support multiple aggregated measurements for group by level statement#2714
Merged
HTHou merged 9 commits intoapache:masterfrom Feb 23, 2021
Merged
[IOTDB-1024] Support multiple aggregated measurements for group by level statement#2714HTHou merged 9 commits intoapache:masterfrom
HTHou merged 9 commits intoapache:masterfrom
Conversation
Contributor
|
Don't forget to run Run |
HTHou
approved these changes
Feb 22, 2021
Contributor
There was a problem hiding this comment.
I have tried this feature and the result looks good.
IoTDB> insert into root.yuerj."30001"."client-521".d1.p1(time, errorQps, QPS) values(1,2,3)
Msg: The statement is executed successfully.
IoTDB> insert into root.yuerj."30001"."client-521".d1.p2(time, errorQps, QPS) values(1,2,3)
Msg: The statement is executed successfully.
IoTDB> insert into root.yuerj."30001"."client-521".d1.p3(time, errorQps, QPS) values(1,2,3)
Msg: The statement is executed successfully.
IoTDB> insert into root.yuerj."30001"."client-521".d2.p4(time, errorQps, QPS) values(1,2,3)
Msg: The statement is executed successfully.
IoTDB> insert into root.yuerj."30001"."client-521".d2.p2(time, errorQps, QPS) values(1,2,3)
Msg: The statement is executed successfully.
IoTDB> select sum(errorQps) , sum(QPS) from root.yuerj."30001"."client-521".*.* GROUP BY ([1970-01-01T08:00:00.000+08:00, 1970-01-01T08:00:00.900+08:00), 100ms) ,level=3
+-----------------------------+--------------------------------------------+-------------------------------------------------+
| Time|sum(root.yuerj."30001"."client-521".*.*.QPS)|sum(root.yuerj."30001"."client-521".*.*.errorQps)|
+-----------------------------+--------------------------------------------+-------------------------------------------------+
|1970-01-01T08:00:00.000+08:00| 15.0| 10.0|
|1970-01-01T08:00:00.100+08:00| 0.0| 0.0|
|1970-01-01T08:00:00.200+08:00| 0.0| 0.0|
|1970-01-01T08:00:00.300+08:00| 0.0| 0.0|
|1970-01-01T08:00:00.400+08:00| 0.0| 0.0|
|1970-01-01T08:00:00.500+08:00| 0.0| 0.0|
|1970-01-01T08:00:00.600+08:00| 0.0| 0.0|
|1970-01-01T08:00:00.700+08:00| 0.0| 0.0|
|1970-01-01T08:00:00.800+08:00| 0.0| 0.0|
+-----------------------------+--------------------------------------------+-------------------------------------------------+
Total line number = 9
It costs 0.030s
HTHou
requested changes
Feb 22, 2021
Contributor
HTHou
left a comment
HTHou
left a comment
There was a problem hiding this comment.
By the way, the order of the results is not same with the sql statement. Is it normal?
Contributor
Author
No, the printed column order is wrong. I will correct it soon. |
Contributor
|
The order is correct now, but the result of avg seems not correct, which shouldn't be null. |
HTHou
approved these changes
Feb 22, 2021
Contributor
HTHou
left a comment
HTHou
left a comment
There was a problem hiding this comment.
LGTM now.
IoTDB> select sum(QPS),avg(QPS), sum(errorQps) from root.yuerj."30001"."client-521".*.* GROUP BY ([1970-01-01T08:00:00.000+08:00, 1970-01-01T08:00:00.900+08:00), 100ms) ,level=3
+-----------------------------+--------------------------------------------+--------------------------------------------+-------------------------------------------------+
| Time|sum(root.yuerj."30001"."client-521".*.*.QPS)|avg(root.yuerj."30001"."client-521".*.*.QPS)|sum(root.yuerj."30001"."client-521".*.*.errorQps)|
+-----------------------------+--------------------------------------------+--------------------------------------------+-------------------------------------------------+
|1970-01-01T08:00:00.000+08:00| 15.0| 3.0000000000000004| 10.0|
|1970-01-01T08:00:00.100+08:00| 0.0| null| 0.0|
|1970-01-01T08:00:00.200+08:00| 0.0| null| 0.0|
|1970-01-01T08:00:00.300+08:00| 0.0| null| 0.0|
|1970-01-01T08:00:00.400+08:00| 0.0| null| 0.0|
|1970-01-01T08:00:00.500+08:00| 0.0| null| 0.0|
|1970-01-01T08:00:00.600+08:00| 0.0| null| 0.0|
|1970-01-01T08:00:00.700+08:00| 0.0| null| 0.0|
|1970-01-01T08:00:00.800+08:00| 0.0| null| 0.0|
+-----------------------------+--------------------------------------------+--------------------------------------------+-------------------------------------------------+
Total line number = 9
It costs 0.661s
Alima777
requested changes
Feb 22, 2021
server/src/test/java/org/apache/iotdb/db/integration/aggregation/IoTDBAggregationByLevelIT.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/apache/iotdb/db/integration/aggregation/IoTDBAggregationByLevelIT.java
Show resolved
Hide resolved
server/src/test/java/org/apache/iotdb/db/integration/aggregation/IoTDBAggregationByLevelIT.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/apache/iotdb/db/integration/aggregation/IoTDBAggregationByLevelIT.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/iotdb/db/qp/physical/crud/AggregationPlan.java
Outdated
Show resolved
Hide resolved
Comment on lines
+881
to
+886
| String aggregatePath = | ||
| originalPath.isMeasurementAliasExists() | ||
| ? FilePathUtils.generatePartialPathByLevel( | ||
| originalPath.getFullPathWithAlias(), ((AggregationPlan) queryPlan).getLevel()) | ||
| : FilePathUtils.generatePartialPathByLevel( | ||
| originalPath.toString(), ((AggregationPlan) queryPlan).getLevel()); |
Contributor
There was a problem hiding this comment.
In line 850, check the level and don't change the value of columnForReader. In this way, you can use the columnForReader directly here.
Contributor
Author
There was a problem hiding this comment.
The modified value of columnForReader is used between line 858-871. Considering the complexity of deduplicate(), I tried not to make much changes here to keep correctness.
Perhaps refactor deduplicate() and moving it into QueryPlan would be a choice. What do you think?
Contributor
There was a problem hiding this comment.
Okay...but maybe we have to rewrite this method later...it's so long
Alima777
approved these changes
Feb 23, 2021
Comment on lines
+881
to
+886
| String aggregatePath = | ||
| originalPath.isMeasurementAliasExists() | ||
| ? FilePathUtils.generatePartialPathByLevel( | ||
| originalPath.getFullPathWithAlias(), ((AggregationPlan) queryPlan).getLevel()) | ||
| : FilePathUtils.generatePartialPathByLevel( | ||
| originalPath.toString(), ((AggregationPlan) queryPlan).getLevel()); |
Contributor
There was a problem hiding this comment.
Okay...but maybe we have to rewrite this method later...it's so long
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Currently, only one aggregated measurement is supported in this statement to avoid result ambiguity.
This PR is to support multiple aggregated measurements in group by level statement.
JIRA issue: https://issues.apache.org/jira/browse/IOTDB-1024