Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Gian Merlino 2019-05-21 11:40:44 -07:00 committed by Fangjin Yang
parent 69b2ea3ddc
commit 43c54385f6
9 changed files with 165 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
View File

@ -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);

3
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeArithmeticOperatorConversion.java
View File

@ -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) {

9
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeExtractOperatorConversion.java
View File

@ -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);
}

23
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFloorOperatorConversion.java
View File

@ -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 {

21
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeFormatOperatorConversion.java
View File

@ -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",

38
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeParseOperatorConversion.java
View File

@ -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())
);
}
}

48
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/TimeShiftOperatorConversion.java
View File

@ -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())
);
}
}

13
sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
View File

@ -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
)
)

20
sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java
View File

@ -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()
);