This is an automated email from the ASF dual-hosted git repository.
jackietien pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iotdb.git
The following commit(s) were added to refs/heads/master by this push:
new 6c10edff06 [IOTDB-3417] Group by month unit bug in MPP (#6185)
6c10edff06 is described below
commit 6c10edff06fc427573799de9ade0ffcf3a119cf5
Author: Xiangwei Wei <[email protected]>
AuthorDate: Tue Jun 7 20:15:30 2022 +0800
[IOTDB-3417] Group by month unit bug in MPP (#6185)
---
.../TimeRangeIteratorFactory.java | 12 ++++++----
.../apache/iotdb/db/mpp/plan/analyze/Analyzer.java | 28 +++++++++++++++++-----
.../plan/parameter/GroupByTimeParameter.java | 13 +++++-----
.../db/mpp/aggregation/TimeRangeIteratorTest.java | 22 ++++++++++++-----
4 files changed, 52 insertions(+), 23 deletions(-)
diff --git
a/server/src/main/java/org/apache/iotdb/db/mpp/aggregation/timerangeiterator/TimeRangeIteratorFactory.java
b/server/src/main/java/org/apache/iotdb/db/mpp/aggregation/timerangeiterator/TimeRangeIteratorFactory.java
index 1dbe7a04f8..5351eb66ad 100644
---
a/server/src/main/java/org/apache/iotdb/db/mpp/aggregation/timerangeiterator/TimeRangeIteratorFactory.java
+++
b/server/src/main/java/org/apache/iotdb/db/mpp/aggregation/timerangeiterator/TimeRangeIteratorFactory.java
@@ -28,8 +28,7 @@ public class TimeRangeIteratorFactory {
/**
* The method returns different implements of ITimeRangeIterator depending
on the parameters.
*
- * <p>Note: interval and slidingStep stand for the milliseconds if not
grouped by month, or the
- * month count if grouped by month.
+ * <p>Note: interval and slidingStep is always stand for the milliseconds in
this method.
*/
public static ITimeRangeIterator getTimeRangeIterator(
long startTime,
@@ -41,9 +40,12 @@ public class TimeRangeIteratorFactory {
boolean isSlidingStepByMonth,
boolean leftCRightO,
boolean outputPartialTimeWindow) {
- long tmpInterval = isIntervalByMonth ? interval * MS_TO_MONTH : interval;
- long tmpSlidingStep = isSlidingStepByMonth ? slidingStep * MS_TO_MONTH :
slidingStep;
- if (outputPartialTimeWindow && tmpInterval > tmpSlidingStep) {
+ long originInterval = interval;
+ long originSlidingStep = slidingStep;
+ interval = isIntervalByMonth ? interval / MS_TO_MONTH : interval;
+ slidingStep = isSlidingStepByMonth ? slidingStep / MS_TO_MONTH :
slidingStep;
+
+ if (outputPartialTimeWindow && originInterval > originSlidingStep) {
if (!isIntervalByMonth && !isSlidingStepByMonth) {
return new PreAggrWindowIterator(
startTime, endTime, interval, slidingStep, isAscending,
leftCRightO);
diff --git
a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/Analyzer.java
b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/Analyzer.java
index 2e8ac3a942..08f88c3ff2 100644
--- a/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/Analyzer.java
+++ b/server/src/main/java/org/apache/iotdb/db/mpp/plan/analyze/Analyzer.java
@@ -80,6 +80,7 @@ import
org.apache.iotdb.db.mpp.plan.statement.metadata.ShowTimeSeriesStatement;
import org.apache.iotdb.db.mpp.plan.statement.sys.ExplainStatement;
import org.apache.iotdb.tsfile.file.metadata.enums.TSDataType;
import org.apache.iotdb.tsfile.read.filter.GroupByFilter;
+import org.apache.iotdb.tsfile.read.filter.GroupByMonthFilter;
import org.apache.iotdb.tsfile.read.filter.basic.Filter;
import org.apache.iotdb.tsfile.read.filter.factory.FilterFactory;
import org.apache.iotdb.tsfile.utils.Pair;
@@ -97,6 +98,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import java.util.TimeZone;
import java.util.stream.Collectors;
/** Analyze the statement and generate Analysis. */
@@ -581,12 +583,7 @@ public class Analyzer {
}
if (queryStatement.isGroupByTime()) {
GroupByTimeComponent groupByTimeComponent =
queryStatement.getGroupByTimeComponent();
- Filter groupByFilter =
- new GroupByFilter(
- groupByTimeComponent.getInterval(),
- groupByTimeComponent.getSlidingStep(),
- groupByTimeComponent.getStartTime(),
- groupByTimeComponent.getEndTime());
+ Filter groupByFilter = initGroupByFilter(groupByTimeComponent);
if (globalTimeFilter == null) {
globalTimeFilter = groupByFilter;
} else {
@@ -1390,4 +1387,23 @@ public class Analyzer {
return analysis;
}
}
+
+ private GroupByFilter initGroupByFilter(GroupByTimeComponent
groupByTimeComponent) {
+ if (groupByTimeComponent.isIntervalByMonth() ||
groupByTimeComponent.isSlidingStepByMonth()) {
+ return new GroupByMonthFilter(
+ groupByTimeComponent.getInterval(),
+ groupByTimeComponent.getSlidingStep(),
+ groupByTimeComponent.getStartTime(),
+ groupByTimeComponent.getEndTime(),
+ groupByTimeComponent.isSlidingStepByMonth(),
+ groupByTimeComponent.isIntervalByMonth(),
+ TimeZone.getTimeZone("+00:00"));
+ } else {
+ return new GroupByFilter(
+ groupByTimeComponent.getInterval(),
+ groupByTimeComponent.getSlidingStep(),
+ groupByTimeComponent.getStartTime(),
+ groupByTimeComponent.getEndTime());
+ }
+ }
}
diff --git
a/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/parameter/GroupByTimeParameter.java
b/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/parameter/GroupByTimeParameter.java
index 1210bc55ce..20d728fa6d 100644
---
a/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/parameter/GroupByTimeParameter.java
+++
b/server/src/main/java/org/apache/iotdb/db/mpp/plan/planner/plan/parameter/GroupByTimeParameter.java
@@ -25,9 +25,12 @@ import org.apache.iotdb.tsfile.utils.ReadWriteIOUtils;
import java.nio.ByteBuffer;
import java.util.Objects;
-import static org.apache.iotdb.db.qp.utils.DatetimeUtils.MS_TO_MONTH;
-
-/** The parameter of `GROUP BY TIME` */
+/**
+ * The parameter of `GROUP BY TIME`.
+ *
+ * <p>Remember: interval and slidingStep is always in timestamp unit before
transforming to
+ * timeRangeIterator even if it's by month unit.
+ */
public class GroupByTimeParameter {
// [startTime, endTime)
@@ -138,9 +141,7 @@ public class GroupByTimeParameter {
}
public boolean hasOverlap() {
- long tmpInterval = isIntervalByMonth ? interval * MS_TO_MONTH : interval;
- long tmpSlidingStep = isSlidingStepByMonth ? slidingStep * MS_TO_MONTH :
slidingStep;
- return tmpInterval > tmpSlidingStep;
+ return interval > slidingStep;
}
public void serialize(ByteBuffer buffer) {
diff --git
a/server/src/test/java/org/apache/iotdb/db/mpp/aggregation/TimeRangeIteratorTest.java
b/server/src/test/java/org/apache/iotdb/db/mpp/aggregation/TimeRangeIteratorTest.java
index 48a3f53553..88e4b6bf16 100644
---
a/server/src/test/java/org/apache/iotdb/db/mpp/aggregation/TimeRangeIteratorTest.java
+++
b/server/src/test/java/org/apache/iotdb/db/mpp/aggregation/TimeRangeIteratorTest.java
@@ -28,6 +28,8 @@ import org.junit.Test;
public class TimeRangeIteratorTest {
+ private static final long MS_TO_MONTH = 30 * 86400_000L;
+
@Test
public void testNotSplitTimeRange() {
String[] res = {
@@ -260,27 +262,35 @@ public class TimeRangeIteratorTest {
};
checkRes(
TimeRangeIteratorFactory.getTimeRangeIterator(
- 1604102400000L, 1617148800000L, 1, 1, true, true, true, true,
false),
+ 1604102400000L,
+ 1617148800000L,
+ MS_TO_MONTH,
+ MS_TO_MONTH,
+ true,
+ true,
+ true,
+ true,
+ false),
res1);
checkRes(
TimeRangeIteratorFactory.getTimeRangeIterator(
- 1604102400000L, 1617148800000L, 1, 1, true, true, true, true,
true),
+ 1604102400000L, 1617148800000L, MS_TO_MONTH, MS_TO_MONTH, true,
true, true, true, true),
res1);
checkRes(
TimeRangeIteratorFactory.getTimeRangeIterator(
- 1604102400000L, 1617148800000L, 864000000, 1, true, false, true,
true, false),
+ 1604102400000L, 1617148800000L, 864000000, MS_TO_MONTH, true,
false, true, true, false),
res2);
checkRes(
TimeRangeIteratorFactory.getTimeRangeIterator(
- 1604102400000L, 1617148800000L, 864000000, 1, true, false, true,
true, true),
+ 1604102400000L, 1617148800000L, 864000000, MS_TO_MONTH, true,
false, true, true, true),
res2);
checkRes(
TimeRangeIteratorFactory.getTimeRangeIterator(
- 1604102400000L, 1617148800000L, 1, 864000000, true, true, false,
true, false),
+ 1604102400000L, 1617148800000L, MS_TO_MONTH, 864000000, true,
true, false, true, false),
res3);
checkRes(
TimeRangeIteratorFactory.getTimeRangeIterator(
- 1604102400000L, 1617148800000L, 1, 864000000, true, true, false,
true, true),
+ 1604102400000L, 1617148800000L, MS_TO_MONTH, 864000000, true,
true, false, true, true),
res4);
}
