haimeiguo commented on a change in pull request #2029:
URL: https://github.com/apache/iotdb/pull/2029#discussion_r528535324
##########
File path:
server/src/main/java/org/apache/iotdb/db/query/dataset/groupby/GroupByEngineDataSet.java
##########
@@ -70,36 +78,72 @@ public GroupByEngineDataSet(QueryContext context,
GroupByTimePlan groupByTimePla
long intervalNum = (long) Math.ceil(queryRange / (double) slidingStep);
curStartTime = slidingStep * (intervalNum - 1) + startTime;
}
- curEndTime = Math.min(curStartTime + interval, endTime);
+
+
+ if (isIntervalByMonth) {
+ //if is group by months interval and sliding step are calculated in ms
by * 30 * 86400_000L
+ //now converting them back to integer months
+ interval = interval / msToMonth;
+ //calculate interval length by natural month based on curStartTime
+ //ie. startTIme = 1/31, interval = 1mo, curEndTime will be set to 2/29
+ curEndTime = Math.min(curStartTime + calcIntervalByMonth(interval,
curStartTime), endTime);
+ } else {
+ curEndTime = Math.min(curStartTime + interval, endTime);
+ }
+ if (isSlidingStepByMonth) {
+ slidingStep = slidingStep / msToMonth;
+ }
this.hasCachedTimeInterval = true;
}
@Override
protected boolean hasNextWithoutConstraint() {
+ long curSlidingStep = slidingStep;
+ long curInterval = interval;
// has cached
if (hasCachedTimeInterval) {
return true;
}
+ intervalTimes++;
+ //group by natural month, given startTime recalculate interval and sliding
step
+ if (isIntervalByMonth) {
+ curInterval = calcIntervalByMonth(intervalTimes * slidingStep +
interval, startTime);
+ }
+ if (isSlidingStepByMonth) {
+ curSlidingStep = calcIntervalByMonth(slidingStep * intervalTimes,
curStartTime);
+ }
+
// check if the next interval out of range
if (ascending) {
- curStartTime += slidingStep;
+ curStartTime += curSlidingStep;
//This is an open interval , [0-100)
if (curStartTime >= endTime) {
return false;
}
} else {
- curStartTime -= slidingStep;
+ curStartTime -= curSlidingStep;
if (curStartTime < startTime) {
return false;
}
}
hasCachedTimeInterval = true;
- curEndTime = Math.min(curStartTime + interval, endTime);
+ if (isIntervalByMonth) {
+ curEndTime = Math.min(startTime + curInterval, endTime);
+ } else {
+ curEndTime = Math.min(curStartTime + curInterval, endTime);
+ }
return true;
}
+ public long calcIntervalByMonth(long numMonths, long curStartTime) {
+ Calendar calendar = Calendar.getInstance();
+ calendar.setTimeInMillis(startTime);
+ calendar.add(Calendar.MONTH, (int) (numMonths));
+ return calendar.getTimeInMillis() - curStartTime;
+ }
+
Review comment:
Hi, the calendar is set to the original startTime and uses intervalTImes
to calendar.add() in case of dates = 29, 30, 31
ie.
when add one month to 1/31, curStartTime increments to 2/29
if then set calendar to curStartTime = 2/29, then increments one month, we
get curStartTime = 3/29, which is not what we intended. What we want is 1/31,
2/29, 3/30, 4/31 ....
so I used intervalTimes each time calculating the interval and set calendar
to original startTime and add intervalTimes months to startTime to avoid edge
cases.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
