You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by ha...@apache.org on 2021/05/14 06:07:23 UTC
[iotdb] branch master updated: [ISSUE-3116] Bug when using natural
month unit in time interval in group by query (#3139)
This is an automated email from the ASF dual-hosted git repository.
haonan 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 517e496 [ISSUE-3116] Bug when using natural month unit in time interval in group by query (#3139)
517e496 is described below
commit 517e496bd4e514929f6d061bb4a9ddee0ba3c7fc
Author: Xiangwei Wei <34...@users.noreply.github.com>
AuthorDate: Fri May 14 14:06:40 2021 +0800
[ISSUE-3116] Bug when using natural month unit in time interval in group by query (#3139)
---
.../apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java | 82 +++++++++-------------
.../apache/iotdb/db/qp/utils/DatetimeUtils.java | 33 +++++++--
.../iotdb/db/integration/IoTDBGroupByMonthIT.java | 29 ++++++++
.../db/qp/utils/DatetimeQueryDataSetUtilsTest.java | 28 ++++++++
4 files changed, 115 insertions(+), 57 deletions(-)
diff --git a/server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java b/server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java
index 3888c74..c279780 100644
--- a/server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java
+++ b/server/src/main/java/org/apache/iotdb/db/qp/sql/IoTDBSqlVisitor.java
@@ -256,7 +256,6 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> {
+ "time > XXX, time <= XXX, or two atomic expressions connected by 'AND'";
private ZoneId zoneId;
QueryOperator queryOp;
- private boolean isParsingSlidingStep;
public void setZoneId(ZoneId zoneId) {
this.zoneId = zoneId;
@@ -1436,17 +1435,17 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> {
queryOp.setGroupByTime(true);
queryOp.setLeftCRightO(ctx.timeInterval().LS_BRACKET() != null);
// parse timeUnit
- queryOp.setUnit(parseDuration(ctx.DURATION(0).getText()));
- queryOp.setSlidingStep(queryOp.getUnit());
+ queryOp.setUnit(parseTimeUnitOrSlidingStep(queryOp, ctx.DURATION(0).getText(), true));
// parse sliding step
if (ctx.DURATION().size() == 2) {
- isParsingSlidingStep = true;
- queryOp.setSlidingStep(parseDuration(ctx.DURATION(1).getText()));
- isParsingSlidingStep = false;
+ queryOp.setSlidingStep(parseTimeUnitOrSlidingStep(queryOp, ctx.DURATION(1).getText(), false));
if (queryOp.getSlidingStep() < queryOp.getUnit()) {
throw new SQLParserException(
"The third parameter sliding step shouldn't be smaller than the second parameter time interval.");
}
+ } else {
+ queryOp.setSlidingStep(queryOp.getUnit());
+ queryOp.setSlidingStepByMonth(queryOp.isIntervalByMonth());
}
parseTimeInterval(ctx.timeInterval(), queryOp);
@@ -1466,7 +1465,7 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> {
queryOp.setLeftCRightO(ctx.timeInterval().LS_BRACKET() != null);
// parse timeUnit
- queryOp.setUnit(parseDuration(ctx.DURATION().getText()));
+ queryOp.setUnit(DatetimeUtils.convertDurationStrToLong(ctx.DURATION().getText()));
queryOp.setSlidingStep(queryOp.getUnit());
parseTimeInterval(ctx.timeInterval(), queryOp);
@@ -1484,7 +1483,9 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> {
if (typeClause.previousUntilLastClause() != null) {
long preRange;
if (typeClause.previousUntilLastClause().DURATION() != null) {
- preRange = parseDuration(typeClause.previousUntilLastClause().DURATION().getText());
+ preRange =
+ DatetimeUtils.convertDurationStrToLong(
+ typeClause.previousUntilLastClause().DURATION().getText());
} else {
preRange = IoTDBDescriptor.getInstance().getConfig().getDefaultFillInterval();
}
@@ -1492,7 +1493,9 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> {
} else {
long preRange;
if (typeClause.previousClause().DURATION() != null) {
- preRange = parseDuration(typeClause.previousClause().DURATION().getText());
+ preRange =
+ DatetimeUtils.convertDurationStrToLong(
+ typeClause.previousClause().DURATION().getText());
} else {
preRange = IoTDBDescriptor.getInstance().getConfig().getDefaultFillInterval();
}
@@ -1523,22 +1526,27 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> {
if (ctx.linearClause() != null) { // linear
if (ctx.linearClause().DURATION(0) != null) {
- long beforeRange = parseDuration(ctx.linearClause().DURATION(0).getText());
- long afterRange = parseDuration(ctx.linearClause().DURATION(1).getText());
+ long beforeRange =
+ DatetimeUtils.convertDurationStrToLong(ctx.linearClause().DURATION(0).getText());
+ long afterRange =
+ DatetimeUtils.convertDurationStrToLong(ctx.linearClause().DURATION(1).getText());
fillTypes.put(dataType, new LinearFill(beforeRange, afterRange));
} else {
fillTypes.put(dataType, new LinearFill(defaultFillInterval, defaultFillInterval));
}
} else if (ctx.previousClause() != null) { // previous
if (ctx.previousClause().DURATION() != null) {
- long preRange = parseDuration(ctx.previousClause().DURATION().getText());
+ long preRange =
+ DatetimeUtils.convertDurationStrToLong(ctx.previousClause().DURATION().getText());
fillTypes.put(dataType, new PreviousFill(preRange));
} else {
fillTypes.put(dataType, new PreviousFill(defaultFillInterval));
}
} else { // previous until last
if (ctx.previousUntilLastClause().DURATION() != null) {
- long preRange = parseDuration(ctx.previousUntilLastClause().DURATION().getText());
+ long preRange =
+ DatetimeUtils.convertDurationStrToLong(
+ ctx.previousUntilLastClause().DURATION().getText());
fillTypes.put(dataType, new PreviousFill(preRange, true));
} else {
fillTypes.put(dataType, new PreviousFill(defaultFillInterval, true));
@@ -1821,56 +1829,30 @@ public class IoTDBSqlVisitor extends SqlBaseBaseVisitor<Operator> {
time = parseTimeFormat(ctx.getChild(0).getText());
for (int i = 1; i < ctx.getChildCount(); i = i + 2) {
if (ctx.getChild(i).getText().equals("+")) {
- time += parseDuration(ctx.getChild(i + 1).getText());
+ time += DatetimeUtils.convertDurationStrToLong(time, ctx.getChild(i + 1).getText());
} else {
- time -= parseDuration(ctx.getChild(i + 1).getText());
+ time -= DatetimeUtils.convertDurationStrToLong(time, ctx.getChild(i + 1).getText());
}
}
return time;
}
/**
- * parse duration to time value.
+ * parse time unit or sliding step in group by query.
*
- * @param durationStr represent duration string like: 12d8m9ns, 1y1mo, etc.
+ * @param durationStr represent duration string like: 12d8m9ns, 1y1d, etc.
* @return time in milliseconds, microseconds, or nanoseconds depending on the profile
*/
- private Long parseDuration(String durationStr) {
- String timestampPrecision = IoTDBDescriptor.getInstance().getConfig().getTimestampPrecision();
-
- long total = 0;
- long tmp = 0;
- for (int i = 0; i < durationStr.length(); i++) {
- char ch = durationStr.charAt(i);
- if (Character.isDigit(ch)) {
- tmp *= 10;
- tmp += (ch - '0');
+ private long parseTimeUnitOrSlidingStep(
+ QueryOperator queryOp, String durationStr, boolean isParsingTimeUnit) {
+ if (durationStr.toLowerCase().contains("mo")) {
+ if (isParsingTimeUnit) {
+ queryOp.setIntervalByMonth(true);
} else {
- String unit = durationStr.charAt(i) + "";
- // This is to identify units with two letters.
- if (i + 1 < durationStr.length() && !Character.isDigit(durationStr.charAt(i + 1))) {
- i++;
- unit += durationStr.charAt(i);
- }
- if (unit.equalsIgnoreCase("mo")) {
- // interval is by month, sliding step by default equals to interval
- if (!isParsingSlidingStep) {
- queryOp.setIntervalByMonth(true);
- }
- queryOp.setSlidingStepByMonth(true);
- } else if (isParsingSlidingStep) {
- // parsing sliding step value, and unit is not by month
- queryOp.setSlidingStepByMonth(false);
- }
- total +=
- DatetimeUtils.convertDurationStrToLong(tmp, unit.toLowerCase(), timestampPrecision);
- tmp = 0;
+ queryOp.setSlidingStepByMonth(true);
}
}
- if (total <= 0) {
- throw new SQLParserException("Interval must more than 0.");
- }
- return total;
+ return DatetimeUtils.convertDurationStrToLong(durationStr);
}
private PartialPath parseSuffixPath(SuffixPathContext ctx) {
diff --git a/server/src/main/java/org/apache/iotdb/db/qp/utils/DatetimeUtils.java b/server/src/main/java/org/apache/iotdb/db/qp/utils/DatetimeUtils.java
index 80035d36..563f7d4 100644
--- a/server/src/main/java/org/apache/iotdb/db/qp/utils/DatetimeUtils.java
+++ b/server/src/main/java/org/apache/iotdb/db/qp/utils/DatetimeUtils.java
@@ -31,6 +31,7 @@ import java.time.format.DateTimeFormatterBuilder;
import java.time.format.DateTimeParseException;
import java.time.format.SignStyle;
import java.time.temporal.ChronoField;
+import java.util.Calendar;
public class DatetimeUtils {
@@ -506,14 +507,15 @@ public class DatetimeUtils {
}
/**
- * convert duration string to time value.
+ * Convert duration string to time value. CurrentTime is used to calculate the days of natural
+ * month. If it's set as -1, which means a context free situation, then '1mo' will be thought as
+ * 30 days.
*
* @param duration represent duration string like: 12d8m9ns, 1y1mo, etc.
* @return time in milliseconds, microseconds, or nanoseconds depending on the profile
*/
public static long convertDurationStrToLong(String duration) {
- String timestampPrecision = IoTDBDescriptor.getInstance().getConfig().getTimestampPrecision();
- return convertDurationStrToLong(duration, timestampPrecision);
+ return convertDurationStrToLong(-1, duration);
}
/**
@@ -522,7 +524,8 @@ public class DatetimeUtils {
* @param duration represent duration string like: 12d8m9ns, 1y1mo, etc.
* @return time in milliseconds, microseconds, or nanoseconds depending on the profile
*/
- public static long convertDurationStrToLong(String duration, String timestampPrecision) {
+ public static long convertDurationStrToLong(long currentTime, String duration) {
+ String timestampPrecision = IoTDBDescriptor.getInstance().getConfig().getTimestampPrecision();
long total = 0;
long temp = 0;
for (int i = 0; i < duration.length(); i++) {
@@ -538,15 +541,24 @@ public class DatetimeUtils {
unit += duration.charAt(i);
}
total +=
- DatetimeUtils.convertDurationStrToLong(temp, unit.toLowerCase(), timestampPrecision);
+ DatetimeUtils.convertDurationStrToLong(
+ currentTime == -1 ? -1 : currentTime + total,
+ temp,
+ unit.toLowerCase(),
+ timestampPrecision);
temp = 0;
}
}
return total;
}
- /** convert duration string to millisecond, microsecond or nanosecond. */
public static long convertDurationStrToLong(long value, String unit, String timestampPrecision) {
+ return convertDurationStrToLong(-1, value, unit, timestampPrecision);
+ }
+
+ /** convert duration string to millisecond, microsecond or nanosecond. */
+ public static long convertDurationStrToLong(
+ long currentTime, long value, String unit, String timestampPrecision) {
DurationUnit durationUnit = DurationUnit.valueOf(unit);
long res = value;
switch (durationUnit) {
@@ -554,7 +566,14 @@ public class DatetimeUtils {
res *= 365 * 86_400_000L;
break;
case mo:
- res *= 30 * 86_400_000L;
+ if (currentTime == -1) {
+ res *= 30 * 86_400_000L;
+ } else {
+ Calendar calendar = Calendar.getInstance();
+ calendar.setTimeInMillis(currentTime);
+ calendar.add(Calendar.MONTH, (int) (value));
+ res = calendar.getTimeInMillis() - currentTime;
+ }
break;
case w:
res *= 7 * 86_400_000L;
diff --git a/server/src/test/java/org/apache/iotdb/db/integration/IoTDBGroupByMonthIT.java b/server/src/test/java/org/apache/iotdb/db/integration/IoTDBGroupByMonthIT.java
index 5ff2236..76e0dba 100644
--- a/server/src/test/java/org/apache/iotdb/db/integration/IoTDBGroupByMonthIT.java
+++ b/server/src/test/java/org/apache/iotdb/db/integration/IoTDBGroupByMonthIT.java
@@ -224,6 +224,35 @@ public class IoTDBGroupByMonthIT {
}
}
+ /** StartTime: now() - 1mo, EndTime: now(). */
+ @Test
+ public void groupByNaturalMonth6() {
+ try (Connection connection =
+ DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root");
+ Statement statement = connection.createStatement()) {
+
+ boolean hasResultSet =
+ statement.execute(
+ "select sum(temperature) from root.sg1.d1 GROUP BY ([now() - 1mo, now()), 1d)");
+
+ Assert.assertTrue(hasResultSet);
+ int cnt = 0;
+ try (ResultSet resultSet = statement.getResultSet()) {
+ while (resultSet.next()) {
+ String ans = resultSet.getString(sum("root.sg1.d1.temperature"));
+ if (ans.equals("0.0")) {
+ cnt++;
+ }
+ }
+ Assert.assertTrue(cnt >= 28);
+ Assert.assertTrue(cnt <= 31);
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail(e.getMessage());
+ }
+ }
+
private void prepareData() {
try (Connection connection =
DriverManager.getConnection("jdbc:iotdb://127.0.0.1:6667/", "root", "root");
diff --git a/server/src/test/java/org/apache/iotdb/db/qp/utils/DatetimeQueryDataSetUtilsTest.java b/server/src/test/java/org/apache/iotdb/db/qp/utils/DatetimeQueryDataSetUtilsTest.java
index 8987830..999a590 100644
--- a/server/src/test/java/org/apache/iotdb/db/qp/utils/DatetimeQueryDataSetUtilsTest.java
+++ b/server/src/test/java/org/apache/iotdb/db/qp/utils/DatetimeQueryDataSetUtilsTest.java
@@ -130,6 +130,34 @@ public class DatetimeQueryDataSetUtilsTest {
Assert.assertEquals(7L, DatetimeUtils.convertDurationStrToLong(7, "ns", "ns"));
}
+ /** Test convert duration including natural month unit. Time includes: 1970-01-01 ~ 1970-12-01 */
+ @Test
+ public void getConvertDurationIncludingMonthUnit() {
+ Assert.assertEquals(31 * 86400000L, DatetimeUtils.convertDurationStrToLong(0, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 28 * 86400000L, DatetimeUtils.convertDurationStrToLong(2678400000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 31 * 86400000L, DatetimeUtils.convertDurationStrToLong(5097600000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 30 * 86400000L, DatetimeUtils.convertDurationStrToLong(7776000000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 31 * 86400000L, DatetimeUtils.convertDurationStrToLong(10368000000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 30 * 86400000L, DatetimeUtils.convertDurationStrToLong(13046400000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 31 * 86400000L, DatetimeUtils.convertDurationStrToLong(15638400000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 31 * 86400000L, DatetimeUtils.convertDurationStrToLong(18316800000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 30 * 86400000L, DatetimeUtils.convertDurationStrToLong(20995200000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 31 * 86400000L, DatetimeUtils.convertDurationStrToLong(23587200000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 30 * 86400000L, DatetimeUtils.convertDurationStrToLong(26265600000L, 1, "mo", "ms"));
+ Assert.assertEquals(
+ 31 * 86400000L, DatetimeUtils.convertDurationStrToLong(28857600000L, 1, "mo", "ms"));
+ }
+
public void testConvertDatetimeStrToLongWithoutMS(ZoneOffset zoneOffset, ZoneId zoneId, long res)
throws LogicalOperatorException {
String[] timeFormatWithoutMs =