You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by fj...@apache.org on 2019/05/21 18:40:50 UTC
[incubator-druid] branch master updated: SQL: Respect default
timezone for TIME_PARSE and TIME_SHIFT. (#7704)
This is an automated email from the ASF dual-hosted git repository.
fjy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git
The following commit(s) were added to refs/heads/master by this push:
new 43c5438 SQL: Respect default timezone for TIME_PARSE and TIME_SHIFT. (#7704)
43c5438 is described below
commit 43c54385f6f7705507570c50e048b08cf5902b10
Author: Gian Merlino <gi...@imply.io>
AuthorDate: Tue May 21 11:40:44 2019 -0700
SQL: Respect default timezone for TIME_PARSE and TIME_SHIFT. (#7704)
* SQL: Respect default timezone for TIME_PARSE and TIME_SHIFT.
They were inadvertently using UTC rather than the default timezone.
Also, harmonize how time functions handle their parameters.
* Fix tests
* Add another TIME_SHIFT test.
---
.../calcite/expression/OperatorConversions.java | 19 +++++++++
.../builtin/TimeArithmeticOperatorConversion.java | 3 +-
.../builtin/TimeExtractOperatorConversion.java | 9 ++--
.../builtin/TimeFloorOperatorConversion.java | 23 +++++++----
.../builtin/TimeFormatOperatorConversion.java | 21 +++++++---
.../builtin/TimeParseOperatorConversion.java | 38 ++++++++++++++++-
.../builtin/TimeShiftOperatorConversion.java | 48 +++++++++++++++++++++-
.../apache/druid/sql/calcite/CalciteQueryTest.java | 13 +++---
.../sql/calcite/expression/ExpressionsTest.java | 20 +++++++--
9 files changed, 165 insertions(+), 29 deletions(-)
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
index f982d80..a4dbfd2 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
@@ -21,6 +21,7 @@ package org.apache.druid.sql.calcite.expression;
import com.google.common.base.Preconditions;
import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.sql.SqlFunction;
import org.apache.calcite.sql.SqlFunctionCategory;
@@ -105,6 +106,24 @@ public class OperatorConversions
return expressionFunction.apply(druidExpressions);
}
+ /**
+ * Gets operand "i" from "operands", or returns a default value if it doesn't exist (operands is too short)
+ * or is null.
+ */
+ public static <T> T getOperandWithDefault(
+ final List<RexNode> operands,
+ final int i,
+ final Function<RexNode, T> f,
+ final T defaultReturnValue
+ )
+ {
+ if (operands.size() > i && !RexLiteral.isNullLiteral(operands.get(i))) {
+ return f.apply(operands.get(i));
+ } else {
+ return defaultReturnValue;
+ }
+ }
+
public static OperatorBuilder operatorBuilder(final String name)
{
return new OperatorBuilder(name);
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeArithmeticOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeArithmeticOperatorConversion.java
index 815cb7e..66f69ff 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeArithmeticOperatorConversion.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeArithmeticOperatorConversion.java
@@ -91,7 +91,8 @@ public abstract class TimeArithmeticOperatorConversion implements SqlOperatorCon
simpleExtraction -> null,
expression -> StringUtils.format("concat('P', %s, 'M')", expression)
),
- DruidExpression.fromExpression(DruidExpression.numberLiteral(direction > 0 ? 1 : -1))
+ DruidExpression.fromExpression(DruidExpression.numberLiteral(direction > 0 ? 1 : -1)),
+ DruidExpression.fromExpression(DruidExpression.stringLiteral(plannerContext.getTimeZone().getID()))
)
);
} else if (rightRexNode.getType().getFamily() == SqlTypeFamily.INTERVAL_DAY_TIME) {
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeExtractOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeExtractOperatorConversion.java
index 5f1c77d..c8d7f3e 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeExtractOperatorConversion.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeExtractOperatorConversion.java
@@ -88,9 +88,12 @@ public class TimeExtractOperatorConversion implements SqlOperatorConversion
StringUtils.toUpperCase(RexLiteral.stringValue(call.getOperands().get(1)))
);
- final DateTimeZone timeZone = call.getOperands().size() > 2 && !RexLiteral.isNullLiteral(call.getOperands().get(2))
- ? DateTimes.inferTzFromString(RexLiteral.stringValue(call.getOperands().get(2)))
- : plannerContext.getTimeZone();
+ final DateTimeZone timeZone = OperatorConversions.getOperandWithDefault(
+ call.getOperands(),
+ 2,
+ operand -> DateTimes.inferTzFromString(RexLiteral.stringValue(operand)),
+ plannerContext.getTimeZone()
+ );
return applyTimeExtract(timeExpression, unit, timeZone);
}
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java
index 097cfe2..24dbd95 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java
@@ -135,14 +135,21 @@ public class TimeFloorOperatorConversion implements SqlOperatorConversion
&& (operands.size() <= 3 || operands.get(3).isA(SqlKind.LITERAL))) {
// Granularity is a literal. Special case since we can use an extractionFn here.
final Period period = new Period(RexLiteral.stringValue(operands.get(1)));
- final DateTime origin =
- operands.size() > 2 && !RexLiteral.isNullLiteral(operands.get(2))
- ? Calcites.calciteDateTimeLiteralToJoda(operands.get(2), plannerContext.getTimeZone())
- : null;
- final DateTimeZone timeZone =
- operands.size() > 3 && !RexLiteral.isNullLiteral(operands.get(3))
- ? DateTimes.inferTzFromString(RexLiteral.stringValue(operands.get(3)))
- : plannerContext.getTimeZone();
+
+ final DateTime origin = OperatorConversions.getOperandWithDefault(
+ call.getOperands(),
+ 2,
+ operand -> Calcites.calciteDateTimeLiteralToJoda(operands.get(2), plannerContext.getTimeZone()),
+ null
+ );
+
+ final DateTimeZone timeZone = OperatorConversions.getOperandWithDefault(
+ call.getOperands(),
+ 3,
+ operand -> DateTimes.inferTzFromString(RexLiteral.stringValue(operand)),
+ plannerContext.getTimeZone()
+ );
+
final PeriodGranularity granularity = new PeriodGranularity(period, origin, timeZone);
return applyTimestampFloor(druidExpressions.get(0), granularity, plannerContext.getExprMacroTable());
} else {
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFormatOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFormatOperatorConversion.java
index b84d314..e91afda 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFormatOperatorConversion.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFormatOperatorConversion.java
@@ -41,6 +41,8 @@ import java.util.stream.Collectors;
public class TimeFormatOperatorConversion implements SqlOperatorConversion
{
+ private static final String DEFAULT_PATTERN = "yyyy-MM-dd'T'HH:mm:ss.SSSZZ";
+
private static final SqlFunction SQL_FUNCTION = OperatorConversions
.operatorBuilder("TIME_FORMAT")
.operandTypes(SqlTypeFamily.TIMESTAMP, SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER)
@@ -69,12 +71,19 @@ public class TimeFormatOperatorConversion implements SqlOperatorConversion
return null;
}
- final String pattern = call.getOperands().size() > 1 && !RexLiteral.isNullLiteral(call.getOperands().get(1))
- ? RexLiteral.stringValue(call.getOperands().get(1))
- : "yyyy-MM-dd'T'HH:mm:ss.SSSZZ";
- final DateTimeZone timeZone = call.getOperands().size() > 2 && !RexLiteral.isNullLiteral(call.getOperands().get(2))
- ? DateTimes.inferTzFromString(RexLiteral.stringValue(call.getOperands().get(2)))
- : plannerContext.getTimeZone();
+ final String pattern = OperatorConversions.getOperandWithDefault(
+ call.getOperands(),
+ 1,
+ RexLiteral::stringValue,
+ DEFAULT_PATTERN
+ );
+
+ final DateTimeZone timeZone = OperatorConversions.getOperandWithDefault(
+ call.getOperands(),
+ 2,
+ operand -> DateTimes.inferTzFromString(RexLiteral.stringValue(operand)),
+ plannerContext.getTimeZone()
+ );
return DruidExpression.fromFunctionCall(
"timestamp_format",
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeParseOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeParseOperatorConversion.java
index 0a8ca2b..de0068d 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeParseOperatorConversion.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeParseOperatorConversion.java
@@ -19,17 +19,25 @@
package org.apache.druid.sql.calcite.expression.builtin;
+import com.google.common.collect.ImmutableList;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.sql.SqlFunction;
import org.apache.calcite.sql.SqlFunctionCategory;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.type.SqlTypeFamily;
import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.Expressions;
import org.apache.druid.sql.calcite.expression.OperatorConversions;
import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
import org.apache.druid.sql.calcite.planner.PlannerContext;
import org.apache.druid.sql.calcite.table.RowSignature;
+import org.joda.time.DateTimeZone;
+
+import java.util.stream.Collectors;
public class TimeParseOperatorConversion implements SqlOperatorConversion
{
@@ -54,6 +62,34 @@ public class TimeParseOperatorConversion implements SqlOperatorConversion
final RexNode rexNode
)
{
- return OperatorConversions.convertCall(plannerContext, rowSignature, rexNode, "timestamp_parse");
+ final RexCall call = (RexCall) rexNode;
+ final RexNode timeArg = call.getOperands().get(0);
+ final DruidExpression timeExpression = Expressions.toDruidExpression(plannerContext, rowSignature, timeArg);
+ if (timeExpression == null) {
+ return null;
+ }
+
+ final String pattern = OperatorConversions.getOperandWithDefault(
+ call.getOperands(),
+ 1,
+ RexLiteral::stringValue,
+ null
+ );
+
+ final DateTimeZone timeZone = OperatorConversions.getOperandWithDefault(
+ call.getOperands(),
+ 2,
+ operand -> DateTimes.inferTzFromString(RexLiteral.stringValue(operand)),
+ plannerContext.getTimeZone()
+ );
+
+ return DruidExpression.fromFunctionCall(
+ "timestamp_parse",
+ ImmutableList.of(
+ timeExpression.getExpression(),
+ DruidExpression.stringLiteral(pattern),
+ DruidExpression.stringLiteral(timeZone.getID())
+ ).stream().map(DruidExpression::fromExpression).collect(Collectors.toList())
+ );
}
}
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeShiftOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeShiftOperatorConversion.java
index 69539c4..0eebbeb 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeShiftOperatorConversion.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeShiftOperatorConversion.java
@@ -19,17 +19,25 @@
package org.apache.druid.sql.calcite.expression.builtin;
+import com.google.common.collect.ImmutableList;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.sql.SqlFunction;
import org.apache.calcite.sql.SqlFunctionCategory;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.type.SqlTypeFamily;
import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.Expressions;
import org.apache.druid.sql.calcite.expression.OperatorConversions;
import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
import org.apache.druid.sql.calcite.planner.PlannerContext;
import org.apache.druid.sql.calcite.table.RowSignature;
+import org.joda.time.DateTimeZone;
+
+import java.util.stream.Collectors;
public class TimeShiftOperatorConversion implements SqlOperatorConversion
{
@@ -54,6 +62,44 @@ public class TimeShiftOperatorConversion implements SqlOperatorConversion
final RexNode rexNode
)
{
- return OperatorConversions.convertCall(plannerContext, rowSignature, rexNode, "timestamp_shift");
+ final RexCall call = (RexCall) rexNode;
+ final DruidExpression timeExpression = Expressions.toDruidExpression(
+ plannerContext,
+ rowSignature,
+ call.getOperands().get(0)
+ );
+
+ final DruidExpression periodExpression = Expressions.toDruidExpression(
+ plannerContext,
+ rowSignature,
+ call.getOperands().get(1)
+ );
+
+ final DruidExpression stepExpression = Expressions.toDruidExpression(
+ plannerContext,
+ rowSignature,
+ call.getOperands().get(2)
+ );
+
+ if (timeExpression == null || periodExpression == null || stepExpression == null) {
+ return null;
+ }
+
+ final DateTimeZone timeZone = OperatorConversions.getOperandWithDefault(
+ call.getOperands(),
+ 3,
+ operand -> DateTimes.inferTzFromString(RexLiteral.stringValue(operand)),
+ plannerContext.getTimeZone()
+ );
+
+ return DruidExpression.fromFunctionCall(
+ "timestamp_shift",
+ ImmutableList.of(
+ timeExpression.getExpression(),
+ periodExpression.getExpression(),
+ stepExpression.getExpression(),
+ DruidExpression.stringLiteral(timeZone.getID())
+ ).stream().map(DruidExpression::fromExpression).collect(Collectors.toList())
+ );
}
}
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index e67cde8..79e51d5 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -5951,8 +5951,11 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
PLANNER_CONFIG_LOS_ANGELES,
QUERY_CONTEXT_DEFAULT,
"SELECT SUM(cnt), gran FROM (\n"
- + " SELECT FLOOR(__time TO MONTH) AS gran,\n"
- + " cnt FROM druid.foo\n"
+ + " SELECT\n"
+ + " FLOOR(__time TO MONTH) AS gran,\n"
+ + " cnt\n"
+ + " FROM druid.foo\n"
+ + " WHERE __time >= TIME_PARSE('1999-12-01 00:00:00') AND __time < TIME_PARSE('2002-01-01 00:00:00')\n"
+ ") AS x\n"
+ "GROUP BY gran\n"
+ "ORDER BY gran",
@@ -5960,7 +5963,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
ImmutableList.of(
Druids.newTimeseriesQueryBuilder()
.dataSource(CalciteTests.DATASOURCE1)
- .intervals(querySegmentSpec(Filtration.eternity()))
+ .intervals(querySegmentSpec(Intervals.of("1999-12-01T00-08:00/2002-01-01T00-08:00")))
.granularity(new PeriodGranularity(Period.months(1), null, DateTimes.inferTzFromString(LOS_ANGELES)))
.aggregators(aggregators(new LongSumAggregatorFactory("a0", "cnt")))
.context(TIMESERIES_CONTEXT_DEFAULT)
@@ -6006,7 +6009,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
{
testQuery(
"SELECT SUM(cnt), gran FROM (\n"
- + " SELECT TIME_FLOOR(TIME_SHIFt(__time, 'P1D', -1), 'P1M') AS gran,\n"
+ + " SELECT TIME_FLOOR(TIME_SHIFT(__time, 'P1D', -1), 'P1M') AS gran,\n"
+ " cnt FROM druid.foo\n"
+ ") AS x\n"
+ "GROUP BY gran\n"
@@ -6019,7 +6022,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
.setVirtualColumns(
expressionVirtualColumn(
"v0",
- "timestamp_floor(timestamp_shift(\"__time\",'P1D',-1),'P1M',null,'UTC')",
+ "timestamp_floor(timestamp_shift(\"__time\",'P1D',-1,'UTC'),'P1M',null,'UTC')",
ValueType.LONG
)
)
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
index 5bf4923..fc46a0a 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
@@ -698,7 +698,19 @@ public class ExpressionsTest extends CalciteTestBase
rexBuilder.makeLiteral("PT2H"),
rexBuilder.makeLiteral(-3, typeFactory.createSqlType(SqlTypeName.INTEGER), true)
),
- DruidExpression.fromExpression("timestamp_shift(\"t\",'PT2H',-3)"),
+ DruidExpression.fromExpression("timestamp_shift(\"t\",'PT2H',-3,'UTC')"),
+ DateTimes.of("2000-02-02T22:05:06").getMillis()
+ );
+
+ testExpression(
+ rexBuilder.makeCall(
+ new TimeShiftOperatorConversion().calciteOperator(),
+ inputRef("t"),
+ rexBuilder.makeLiteral("PT2H"),
+ rexBuilder.makeLiteral(-3, typeFactory.createSqlType(SqlTypeName.INTEGER), true),
+ rexBuilder.makeLiteral("America/Los_Angeles")
+ ),
+ DruidExpression.fromExpression("timestamp_shift(\"t\",'PT2H',-3,'America/Los_Angeles')"),
DateTimes.of("2000-02-02T22:05:06").getMillis()
);
}
@@ -766,7 +778,7 @@ public class ExpressionsTest extends CalciteTestBase
),
DruidExpression.of(
null,
- "timestamp_shift(\"t\",concat('P', 13, 'M'),1)"
+ "timestamp_shift(\"t\",concat('P', 13, 'M'),1,'UTC')"
),
DateTimes.of("2000-02-03T04:05:06").plus(period).getMillis()
);
@@ -816,7 +828,7 @@ public class ExpressionsTest extends CalciteTestBase
),
DruidExpression.of(
null,
- "timestamp_shift(\"t\",concat('P', 13, 'M'),-1)"
+ "timestamp_shift(\"t\",concat('P', 13, 'M'),-1,'UTC')"
),
DateTimes.of("2000-02-03T04:05:06").minus(period).getMillis()
);
@@ -831,7 +843,7 @@ public class ExpressionsTest extends CalciteTestBase
inputRef("tstr"),
rexBuilder.makeLiteral("yyyy-MM-dd HH:mm:ss")
),
- DruidExpression.fromExpression("timestamp_parse(\"tstr\",'yyyy-MM-dd HH:mm:ss')"),
+ DruidExpression.fromExpression("timestamp_parse(\"tstr\",'yyyy-MM-dd HH:mm:ss','UTC')"),
DateTimes.of("2000-02-03T04:05:06").getMillis()
);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org