Re: [PR] [Bug] window assigns error #11286 [iotdb]

via GitHub Tue, 17 Oct 2023 02:59:02 -0700


cqzhang7 commented on code in PR #11290:
URL: https://github.com/apache/iotdb/pull/11290#discussion_r1361848488



##########
iotdb-core/tsfile/src/main/java/org/apache/iotdb/tsfile/read/filter/GroupByMonthFilter.java:
##########
@@ -255,32 +325,47 @@ private long getTimePointPosition(long time) {
   private void getNthTimeInterval(long n) {
     // get start time of time interval
     if (isSlidingStepByMonth) {
-      calendar.setTimeInMillis(initialStartTime);
-      calendar.add(Calendar.MONTH, (int) (slidingStepsInMo * n));
+      if (fixedSlidingStepOther == 0) {
+        this.startTime = calcIntervalByMonth(initialStartTime, 
slidingStepsInMo * n);
+      } else {
+        this.startTime = timeArray[(int) n];
+      }
     } else {
-      calendar.setTimeInMillis(initialStartTime + slidingStep * n);
+      this.startTime = initialStartTime + slidingStep * n;
     }
-    this.startTime = calendar.getTimeInMillis();
 
     // get interval and sliding step
     if (isIntervalByMonth) {
-      if (isSlidingStepByMonth) {
-        calendar.setTimeInMillis(initialStartTime);
-        calendar.add(Calendar.MONTH, (int) (slidingStepsInMo * n) + 
intervalInMo);
-      } else {
-        calendar.add(Calendar.MONTH, intervalInMo);
-      }
-      this.interval = calendar.getTimeInMillis() - startTime;
+      this.interval =
+          calcIntervalByMonthWithFixedOther(startTime, intervalInMo, 
fixedIntervalOther)
+              - startTime;
     }
     if (isSlidingStepByMonth) {
-      calendar.setTimeInMillis(initialStartTime);
-      calendar.add(Calendar.MONTH, (int) (slidingStepsInMo * (n + 1)));
-      this.slidingStep = calendar.getTimeInMillis() - startTime;
+      this.slidingStep =
+          calcIntervalByMonthWithFixedOther(startTime, slidingStepsInMo, 
fixedSlidingStepOther)
+              - startTime;
     }
   }
 
   @Override
   public FilterSerializeId getSerializeId() {
     return FilterSerializeId.GROUP_BY_MONTH;
   }
+
+  private long calcIntervalByMonth(long startTime, long numMonths) {
+    calendar.setTimeInMillis(startTime);
+    boolean isLastDayOfMonth =
+        calendar.get(Calendar.DAY_OF_MONTH) == 
calendar.getActualMaximum(Calendar.DAY_OF_MONTH);
+    calendar.add(Calendar.MONTH, (int) (numMonths));
+    if (isLastDayOfMonth) {
+      calendar.set(Calendar.DAY_OF_MONTH, 
calendar.getActualMaximum(Calendar.DAY_OF_MONTH));
+    }
+    return calendar.getTimeInMillis();
+  }
+
+  private long calcIntervalByMonthWithFixedOther(
+      long startTime, long fixedMonths, long fixedOther) {
+    long nextStartTime = calcIntervalByMonth(startTime, fixedMonths);
+    return nextStartTime + fixedOther;
+  }

Review Comment:
   I tried doing this，but not sure is this ok? I don't know if it will 
introduce unknown effects... 
   
![image](https://github.com/apache/iotdb/assets/49841963/4d24ee4f-3bf9-4f66-8284-846caeac29b3)
   



##########
iotdb-core/tsfile/src/main/java/org/apache/iotdb/tsfile/read/filter/GroupByMonthFilter.java:
##########
@@ -255,32 +325,47 @@ private long getTimePointPosition(long time) {
   private void getNthTimeInterval(long n) {
     // get start time of time interval
     if (isSlidingStepByMonth) {
-      calendar.setTimeInMillis(initialStartTime);
-      calendar.add(Calendar.MONTH, (int) (slidingStepsInMo * n));
+      if (fixedSlidingStepOther == 0) {
+        this.startTime = calcIntervalByMonth(initialStartTime, 
slidingStepsInMo * n);
+      } else {
+        this.startTime = timeArray[(int) n];
+      }
     } else {
-      calendar.setTimeInMillis(initialStartTime + slidingStep * n);
+      this.startTime = initialStartTime + slidingStep * n;
     }
-    this.startTime = calendar.getTimeInMillis();
 
     // get interval and sliding step
     if (isIntervalByMonth) {
-      if (isSlidingStepByMonth) {
-        calendar.setTimeInMillis(initialStartTime);
-        calendar.add(Calendar.MONTH, (int) (slidingStepsInMo * n) + 
intervalInMo);
-      } else {
-        calendar.add(Calendar.MONTH, intervalInMo);
-      }
-      this.interval = calendar.getTimeInMillis() - startTime;
+      this.interval =
+          calcIntervalByMonthWithFixedOther(startTime, intervalInMo, 
fixedIntervalOther)
+              - startTime;
     }
     if (isSlidingStepByMonth) {
-      calendar.setTimeInMillis(initialStartTime);
-      calendar.add(Calendar.MONTH, (int) (slidingStepsInMo * (n + 1)));
-      this.slidingStep = calendar.getTimeInMillis() - startTime;
+      this.slidingStep =
+          calcIntervalByMonthWithFixedOther(startTime, slidingStepsInMo, 
fixedSlidingStepOther)
+              - startTime;
     }
   }
 
   @Override
   public FilterSerializeId getSerializeId() {
     return FilterSerializeId.GROUP_BY_MONTH;
   }
+
+  private long calcIntervalByMonth(long startTime, long numMonths) {
+    calendar.setTimeInMillis(startTime);
+    boolean isLastDayOfMonth =
+        calendar.get(Calendar.DAY_OF_MONTH) == 
calendar.getActualMaximum(Calendar.DAY_OF_MONTH);
+    calendar.add(Calendar.MONTH, (int) (numMonths));
+    if (isLastDayOfMonth) {
+      calendar.set(Calendar.DAY_OF_MONTH, 
calendar.getActualMaximum(Calendar.DAY_OF_MONTH));
+    }
+    return calendar.getTimeInMillis();
+  }
+
+  private long calcIntervalByMonthWithFixedOther(
+      long startTime, long fixedMonths, long fixedOther) {
+    long nextStartTime = calcIntervalByMonth(startTime, fixedMonths);
+    return nextStartTime + fixedOther;
+  }

Review Comment:
   I tried doing this，but not sure is this ok? I don't know if it will 
introduce unknown effects... 
   
![image](https://github.com/apache/iotdb/assets/49841963/4d24ee4f-3bf9-4f66-8284-846caeac29b3)
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [Bug] window assigns error #11286 [iotdb]

Reply via email to