From 76c4b6446eac8da688f6e7c621779a8b98ad41d8 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Fri, 24 Mar 2017 18:45:23 -0700 Subject: [PATCH] SQL: Fix handling of CURRENT_TIMESTAMP and friends in non-UTC timezones. (#4114) --- .../druid/sql/calcite/planner/Calcites.java | 17 +++++++++++++++ .../calcite/planner/DruidConvertletTable.java | 10 +++++---- .../druid/sql/calcite/CalciteQueryTest.java | 21 +++++++++++++++---- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/sql/src/main/java/io/druid/sql/calcite/planner/Calcites.java b/sql/src/main/java/io/druid/sql/calcite/planner/Calcites.java index 6f33cb6c6e5..3cdbf05d478 100644 --- a/sql/src/main/java/io/druid/sql/calcite/planner/Calcites.java +++ b/sql/src/main/java/io/druid/sql/calcite/planner/Calcites.java @@ -34,6 +34,7 @@ import org.joda.time.DateTimeZone; import org.joda.time.Days; import java.nio.charset.Charset; +import java.util.Calendar; /** * Utility functions for Calcite. @@ -139,6 +140,22 @@ public class Calcites return Days.daysBetween(new DateTime(0L, DateTimeZone.UTC), date.withZoneRetainFields(DateTimeZone.UTC)).getDays(); } + /** + * Calcite expects TIMESTAMP and DATE literals to be represented by Calendars that would have the expected + * local time fields if printed as UTC. + * + * @param dateTime joda timestamp + * @param timeZone session time zone + * + * @return Calcite style Calendar, appropriate for literals + */ + public static Calendar jodaToCalciteCalendarLiteral(final DateTime dateTime, final DateTimeZone timeZone) + { + final Calendar calendar = Calendar.getInstance(); + calendar.setTimeInMillis(Calcites.jodaToCalciteTimestamp(dateTime, timeZone)); + return calendar; + } + /** * The inverse of {@link #jodaToCalciteTimestamp(DateTime, DateTimeZone)}. * diff --git a/sql/src/main/java/io/druid/sql/calcite/planner/DruidConvertletTable.java b/sql/src/main/java/io/druid/sql/calcite/planner/DruidConvertletTable.java index e83264498a3..4f370c99cde 100644 --- a/sql/src/main/java/io/druid/sql/calcite/planner/DruidConvertletTable.java +++ b/sql/src/main/java/io/druid/sql/calcite/planner/DruidConvertletTable.java @@ -32,7 +32,6 @@ import org.apache.calcite.sql2rel.SqlRexConvertlet; import org.apache.calcite.sql2rel.SqlRexConvertletTable; import org.apache.calcite.sql2rel.StandardConvertletTable; -import java.util.Locale; import java.util.Map; public class DruidConvertletTable implements SqlRexConvertletTable @@ -84,17 +83,20 @@ public class DruidConvertletTable implements SqlRexConvertletTable final SqlOperator operator = call.getOperator(); if (operator == SqlStdOperatorTable.CURRENT_TIMESTAMP || operator == SqlStdOperatorTable.LOCALTIMESTAMP) { return cx.getRexBuilder().makeTimestampLiteral( - plannerContext.getLocalNow().toCalendar(Locale.getDefault()), + Calcites.jodaToCalciteCalendarLiteral(plannerContext.getLocalNow(), plannerContext.getTimeZone()), RelDataType.PRECISION_NOT_SPECIFIED ); } else if (operator == SqlStdOperatorTable.CURRENT_TIME || operator == SqlStdOperatorTable.LOCALTIME) { return cx.getRexBuilder().makeTimeLiteral( - plannerContext.getLocalNow().toCalendar(Locale.getDefault()), + Calcites.jodaToCalciteCalendarLiteral(plannerContext.getLocalNow(), plannerContext.getTimeZone()), RelDataType.PRECISION_NOT_SPECIFIED ); } else if (operator == SqlStdOperatorTable.CURRENT_DATE) { return cx.getRexBuilder().makeDateLiteral( - plannerContext.getLocalNow().hourOfDay().roundFloorCopy().toCalendar(Locale.getDefault()) + Calcites.jodaToCalciteCalendarLiteral( + plannerContext.getLocalNow().hourOfDay().roundFloorCopy(), + plannerContext.getTimeZone() + ) ); } else { throw new ISE("WTF?! Should not have got here, operator was: %s", operator); diff --git a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java index 51e1fa5c628..c088c93bbd2 100644 --- a/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/io/druid/sql/calcite/CalciteQueryTest.java @@ -2962,6 +2962,19 @@ public class CalciteQueryTest ); } + @Test + public void testSelectCurrentTimeAndDateLosAngeles() throws Exception + { + testQuery( + PlannerContext.create(PLANNER_CONFIG_DEFAULT, QUERY_CONTEXT_LOS_ANGELES), + "SELECT CURRENT_TIMESTAMP, CURRENT_DATE, CURRENT_DATE + INTERVAL '1' DAY", + ImmutableList.of(), + ImmutableList.of( + new Object[]{T("2000-01-01T00Z", LOS_ANGELES), D("1999-12-31"), D("2000-01-01")} + ) + ); + } + @Test public void testFilterOnCurrentTimestampLosAngeles() throws Exception { @@ -2972,14 +2985,14 @@ public class CalciteQueryTest ImmutableList.of( Druids.newTimeseriesQueryBuilder() .dataSource(CalciteTests.DATASOURCE1) - .intervals(QSS(new Interval("2000-01-02T08Z/2002-01-01T08Z"))) + .intervals(QSS(new Interval("2000-01-02T00Z/2002-01-01T08Z"))) .granularity(Granularities.ALL) .aggregators(AGGS(new CountAggregatorFactory("a0"))) .context(TIMESERIES_CONTEXT_LOS_ANGELES) .build() ), ImmutableList.of( - new Object[]{4L} + new Object[]{5L} ) ); } @@ -3016,14 +3029,14 @@ public class CalciteQueryTest ImmutableList.of( Druids.newTimeseriesQueryBuilder() .dataSource(CalciteTests.DATASOURCE1) - .intervals(QSS(new Interval("2000-01-02T08Z/2002-01-01T08Z"))) + .intervals(QSS(new Interval("2000-01-02T00Z/2002-01-01T08Z"))) .granularity(Granularities.ALL) .aggregators(AGGS(new CountAggregatorFactory("a0"))) .context(TIMESERIES_CONTEXT_LOS_ANGELES) .build() ), ImmutableList.of( - new Object[]{4L} + new Object[]{5L} ) ); }