You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/11/03 03:50:56 UTC
[GitHub] [iotdb] CRZbulabula opened a new pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
CRZbulabula opened a new pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303
Refactor GroupByFillDataSet.java to GroupByFillWithoutValueFilterDataSet.java.
Group by fill will be processed according to time series.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] cornmonster commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r743346455
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/LinearFill.java
##########
@@ -65,6 +68,30 @@ public LinearFill(TSDataType dataType, long queryTime, long beforeRange, long af
this.afterRange = afterRange;
}
+ public LinearFill() {}
+
+ public void convertRange(long startTime, long endTime) {
+ if (beforeRange % GroupByEngineDataSet.MS_TO_MONTH == 0) {
+ Calendar calendar = Calendar.getInstance();
Review comment:
Please use the new date and time API defined in java8. See org.apache.iotdb.db.qp.utils.DatetimeUtils for examples.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/PreviousFill.java
##########
@@ -77,6 +80,18 @@ void constructFilter() {
timeFilter = FilterFactory.and(lowerBound, TimeFilter.ltEq(queryTime));
}
+ public void convertRange(long startTime) {
+ if (beforeRange % GroupByEngineDataSet.MS_TO_MONTH == 0) {
+ Calendar calendar = Calendar.getInstance();
Review comment:
Please use the new date and time API defined in java8. See org.apache.iotdb.db.qp.utils.DatetimeUtils for examples.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByEngineDataSet.java
##########
@@ -66,6 +66,83 @@ public GroupByEngineDataSet(QueryContext context, GroupByTimePlan groupByTimePla
initGroupByEngineDataSetFields(context, groupByTimePlan);
}
+ protected Pair<Long, Long> getFirstTimeRange() {
+ long retEndTime;
+ if (isIntervalByMonth) {
+ // calculate interval length by natural month based on startTime
+ // ie. startTIme = 1/31, interval = 1mo, curEndTime will be set to 2/29
+ retEndTime = Math.min(calcIntervalByMonth(interval), endTime);
+ } else {
+ retEndTime = Math.min(startTime + interval, endTime);
+ }
+ return new Pair<>(startTime, retEndTime);
+ }
+
+ protected Pair<Long, Long> getLastTimeRange() {
+ long retStartTime, retEndTime;
Review comment:
Coding style issue. Please do not define multiple variables in a single line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/44063227)
Coverage increased (+0.1%) to 67.07% when pulling **a209bb3147abe19c406c2f76e6001acb5c9684e0 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/43973370)
Coverage increased (+0.1%) to 67.087% when pulling **2fe3b7da5819f8e0b0216eb2af17606104b8b4fa on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls commented on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/43971858)
Coverage increased (+0.1%) to 67.086% when pulling **b92665fe64128f580ff89ff6ed01d5889d1e0744 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/44063164)
Coverage increased (+0.1%) to 67.061% when pulling **a209bb3147abe19c406c2f76e6001acb5c9684e0 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls commented on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/43971858)
Coverage increased (+0.1%) to 67.086% when pulling **b92665fe64128f580ff89ff6ed01d5889d1e0744 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] cornmonster commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r743346455
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/LinearFill.java
##########
@@ -65,6 +68,30 @@ public LinearFill(TSDataType dataType, long queryTime, long beforeRange, long af
this.afterRange = afterRange;
}
+ public LinearFill() {}
+
+ public void convertRange(long startTime, long endTime) {
+ if (beforeRange % GroupByEngineDataSet.MS_TO_MONTH == 0) {
+ Calendar calendar = Calendar.getInstance();
Review comment:
Please use the new date and time API defined in java8. See org.apache.iotdb.db.qp.utils.DatetimeUtils for examples.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/PreviousFill.java
##########
@@ -77,6 +80,18 @@ void constructFilter() {
timeFilter = FilterFactory.and(lowerBound, TimeFilter.ltEq(queryTime));
}
+ public void convertRange(long startTime) {
+ if (beforeRange % GroupByEngineDataSet.MS_TO_MONTH == 0) {
+ Calendar calendar = Calendar.getInstance();
Review comment:
Please use the new date and time API defined in java8. See org.apache.iotdb.db.qp.utils.DatetimeUtils for examples.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByEngineDataSet.java
##########
@@ -66,6 +66,83 @@ public GroupByEngineDataSet(QueryContext context, GroupByTimePlan groupByTimePla
initGroupByEngineDataSetFields(context, groupByTimePlan);
}
+ protected Pair<Long, Long> getFirstTimeRange() {
+ long retEndTime;
+ if (isIntervalByMonth) {
+ // calculate interval length by natural month based on startTime
+ // ie. startTIme = 1/31, interval = 1mo, curEndTime will be set to 2/29
+ retEndTime = Math.min(calcIntervalByMonth(interval), endTime);
+ } else {
+ retEndTime = Math.min(startTime + interval, endTime);
+ }
+ return new Pair<>(startTime, retEndTime);
+ }
+
+ protected Pair<Long, Long> getLastTimeRange() {
+ long retStartTime, retEndTime;
Review comment:
Coding style issue. Please do not define multiple variables in a single line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] CRZbulabula commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r744060439
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByEngineDataSet.java
##########
@@ -66,6 +66,83 @@ public GroupByEngineDataSet(QueryContext context, GroupByTimePlan groupByTimePla
initGroupByEngineDataSetFields(context, groupByTimePlan);
}
+ protected Pair<Long, Long> getFirstTimeRange() {
+ long retEndTime;
+ if (isIntervalByMonth) {
+ // calculate interval length by natural month based on startTime
+ // ie. startTIme = 1/31, interval = 1mo, curEndTime will be set to 2/29
+ retEndTime = Math.min(calcIntervalByMonth(interval), endTime);
+ } else {
+ retEndTime = Math.min(startTime + interval, endTime);
+ }
+ return new Pair<>(startTime, retEndTime);
+ }
+
+ protected Pair<Long, Long> getLastTimeRange() {
+ long retStartTime, retEndTime;
Review comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r741831872
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java
##########
@@ -51,10 +51,10 @@
public class GroupByWithoutValueFilterDataSet extends GroupByEngineDataSet {
- private static final Logger logger =
+ protected static final Logger logger =
Review comment:
You should construct a new Logger using the Subclass in itself.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/QueryRouter.java
##########
@@ -252,15 +256,28 @@ protected FillQueryExecutor getFillExecutor(FillQueryPlan plan) {
public QueryDataSet groupByFill(GroupByTimeFillPlan groupByFillPlan, QueryContext context)
throws QueryFilterOptimizationException, StorageEngineException, QueryProcessException,
IOException {
- GroupByEngineDataSet groupByEngineDataSet =
- (GroupByEngineDataSet) groupBy(groupByFillPlan, context);
- return new GroupByFillDataSet(
- groupByFillPlan.getDeduplicatedPaths(),
- groupByFillPlan.getDeduplicatedDataTypes(),
- groupByEngineDataSet,
- groupByFillPlan.getFillType(),
- context,
- groupByFillPlan);
+ // if (logger.isDebugEnabled()) {
+ // logger.debug(
+ // "paths:"
+ // + groupByFillPlan.getPaths()
+ // + " level:"
+ // + Arrays.toString(groupByFillPlan.getLevels()));
+ // }
Review comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/44040364)
Coverage increased (+0.1%) to 67.068% when pulling **9735b8af5262db0b5854100f7e015331bc755153 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/44063175)
Coverage increased (+0.08%) to 67.042% when pulling **a209bb3147abe19c406c2f76e6001acb5c9684e0 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r741831872
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java
##########
@@ -51,10 +51,10 @@
public class GroupByWithoutValueFilterDataSet extends GroupByEngineDataSet {
- private static final Logger logger =
+ protected static final Logger logger =
Review comment:
You should construct a new Logger using the Subclass in itself.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/QueryRouter.java
##########
@@ -252,15 +256,28 @@ protected FillQueryExecutor getFillExecutor(FillQueryPlan plan) {
public QueryDataSet groupByFill(GroupByTimeFillPlan groupByFillPlan, QueryContext context)
throws QueryFilterOptimizationException, StorageEngineException, QueryProcessException,
IOException {
- GroupByEngineDataSet groupByEngineDataSet =
- (GroupByEngineDataSet) groupBy(groupByFillPlan, context);
- return new GroupByFillDataSet(
- groupByFillPlan.getDeduplicatedPaths(),
- groupByFillPlan.getDeduplicatedDataTypes(),
- groupByEngineDataSet,
- groupByFillPlan.getFillType(),
- context,
- groupByFillPlan);
+ // if (logger.isDebugEnabled()) {
+ // logger.debug(
+ // "paths:"
+ // + groupByFillPlan.getPaths()
+ // + " level:"
+ // + Arrays.toString(groupByFillPlan.getLevels()));
+ // }
Review comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/44040364)
Coverage increased (+0.1%) to 67.068% when pulling **9735b8af5262db0b5854100f7e015331bc755153 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] cornmonster commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r743346455
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/LinearFill.java
##########
@@ -65,6 +68,30 @@ public LinearFill(TSDataType dataType, long queryTime, long beforeRange, long af
this.afterRange = afterRange;
}
+ public LinearFill() {}
+
+ public void convertRange(long startTime, long endTime) {
+ if (beforeRange % GroupByEngineDataSet.MS_TO_MONTH == 0) {
+ Calendar calendar = Calendar.getInstance();
Review comment:
Please use the new date and time API defined in java8. See org.apache.iotdb.db.qp.utils.DatetimeUtils for examples.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/44063160)
Coverage increased (+0.1%) to 67.098% when pulling **a209bb3147abe19c406c2f76e6001acb5c9684e0 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 merged pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
JackieTien97 merged pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls commented on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/43971858)
Coverage increased (+0.1%) to 67.086% when pulling **b92665fe64128f580ff89ff6ed01d5889d1e0744 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r741831872
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java
##########
@@ -51,10 +51,10 @@
public class GroupByWithoutValueFilterDataSet extends GroupByEngineDataSet {
- private static final Logger logger =
+ protected static final Logger logger =
Review comment:
You should construct a new Logger using the Subclass in itself.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/QueryRouter.java
##########
@@ -252,15 +256,28 @@ protected FillQueryExecutor getFillExecutor(FillQueryPlan plan) {
public QueryDataSet groupByFill(GroupByTimeFillPlan groupByFillPlan, QueryContext context)
throws QueryFilterOptimizationException, StorageEngineException, QueryProcessException,
IOException {
- GroupByEngineDataSet groupByEngineDataSet =
- (GroupByEngineDataSet) groupBy(groupByFillPlan, context);
- return new GroupByFillDataSet(
- groupByFillPlan.getDeduplicatedPaths(),
- groupByFillPlan.getDeduplicatedDataTypes(),
- groupByEngineDataSet,
- groupByFillPlan.getFillType(),
- context,
- groupByFillPlan);
+ // if (logger.isDebugEnabled()) {
+ // logger.debug(
+ // "paths:"
+ // + groupByFillPlan.getPaths()
+ // + " level:"
+ // + Arrays.toString(groupByFillPlan.getLevels()));
+ // }
Review comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls commented on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/43971858)
Coverage increased (+0.1%) to 67.086% when pulling **b92665fe64128f580ff89ff6ed01d5889d1e0744 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] JackieTien97 commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r741831872
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java
##########
@@ -51,10 +51,10 @@
public class GroupByWithoutValueFilterDataSet extends GroupByEngineDataSet {
- private static final Logger logger =
+ protected static final Logger logger =
Review comment:
You should construct a new Logger using the Subclass in itself.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/QueryRouter.java
##########
@@ -252,15 +256,28 @@ protected FillQueryExecutor getFillExecutor(FillQueryPlan plan) {
public QueryDataSet groupByFill(GroupByTimeFillPlan groupByFillPlan, QueryContext context)
throws QueryFilterOptimizationException, StorageEngineException, QueryProcessException,
IOException {
- GroupByEngineDataSet groupByEngineDataSet =
- (GroupByEngineDataSet) groupBy(groupByFillPlan, context);
- return new GroupByFillDataSet(
- groupByFillPlan.getDeduplicatedPaths(),
- groupByFillPlan.getDeduplicatedDataTypes(),
- groupByEngineDataSet,
- groupByFillPlan.getFillType(),
- context,
- groupByFillPlan);
+ // if (logger.isDebugEnabled()) {
+ // logger.debug(
+ // "paths:"
+ // + groupByFillPlan.getPaths()
+ // + " level:"
+ // + Arrays.toString(groupByFillPlan.getLevels()));
+ // }
Review comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/43971884)
Coverage increased (+0.01%) to 66.972% when pulling **b92665fe64128f580ff89ff6ed01d5889d1e0744 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/43975739)
Coverage increased (+0.1%) to 67.085% when pulling **d41504c4525518ea306fb53660f286df40284c54 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] cornmonster commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
cornmonster commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r743346664
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/fill/PreviousFill.java
##########
@@ -77,6 +80,18 @@ void constructFilter() {
timeFilter = FilterFactory.and(lowerBound, TimeFilter.ltEq(queryTime));
}
+ public void convertRange(long startTime) {
+ if (beforeRange % GroupByEngineDataSet.MS_TO_MONTH == 0) {
+ Calendar calendar = Calendar.getInstance();
Review comment:
Please use the new date and time API defined in java8. See org.apache.iotdb.db.qp.utils.DatetimeUtils for examples.
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByEngineDataSet.java
##########
@@ -66,6 +66,83 @@ public GroupByEngineDataSet(QueryContext context, GroupByTimePlan groupByTimePla
initGroupByEngineDataSetFields(context, groupByTimePlan);
}
+ protected Pair<Long, Long> getFirstTimeRange() {
+ long retEndTime;
+ if (isIntervalByMonth) {
+ // calculate interval length by natural month based on startTime
+ // ie. startTIme = 1/31, interval = 1mo, curEndTime will be set to 2/29
+ retEndTime = Math.min(calcIntervalByMonth(interval), endTime);
+ } else {
+ retEndTime = Math.min(startTime + interval, endTime);
+ }
+ return new Pair<>(startTime, retEndTime);
+ }
+
+ protected Pair<Long, Long> getLastTimeRange() {
+ long retStartTime, retEndTime;
Review comment:
Coding style issue. Please do not define multiple variables in a single line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] CRZbulabula commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r744072484
##########
File path: server/src/main/java/org/apache/iotdb/db/query/executor/QueryRouter.java
##########
@@ -252,15 +256,28 @@ protected FillQueryExecutor getFillExecutor(FillQueryPlan plan) {
public QueryDataSet groupByFill(GroupByTimeFillPlan groupByFillPlan, QueryContext context)
throws QueryFilterOptimizationException, StorageEngineException, QueryProcessException,
IOException {
- GroupByEngineDataSet groupByEngineDataSet =
- (GroupByEngineDataSet) groupBy(groupByFillPlan, context);
- return new GroupByFillDataSet(
- groupByFillPlan.getDeduplicatedPaths(),
- groupByFillPlan.getDeduplicatedDataTypes(),
- groupByEngineDataSet,
- groupByFillPlan.getFillType(),
- context,
- groupByFillPlan);
+ // if (logger.isDebugEnabled()) {
+ // logger.debug(
+ // "paths:"
+ // + groupByFillPlan.getPaths()
+ // + " level:"
+ // + Arrays.toString(groupByFillPlan.getLevels()));
+ // }
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#issuecomment-958650788
[](https://coveralls.io/builds/44040364)
Coverage increased (+0.1%) to 67.068% when pulling **9735b8af5262db0b5854100f7e015331bc755153 on CRZbulabula:support_other_aggregations_in_group_by_fill** into **b05e21c078debcbb020d62ecd6d8a00a932863bd on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] CRZbulabula commented on a change in pull request #4303: [IOTDB-1760] Support other aggregations in group by fill
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #4303:
URL: https://github.com/apache/iotdb/pull/4303#discussion_r744060431
##########
File path: server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByWithoutValueFilterDataSet.java
##########
@@ -51,10 +51,10 @@
public class GroupByWithoutValueFilterDataSet extends GroupByEngineDataSet {
- private static final Logger logger =
+ protected static final Logger logger =
Review comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org