From d482da6e9be1afabff436bc4a8e0f48f166ba492 Mon Sep 17 00:00:00 2001
From: Xue Yu <278006819@qq.com>
Date: Tue, 4 Jun 2019 16:16:31 +0800
Subject: [PATCH] fix timestamp ceil lower bound bug (#7823)

---
 .../expression/TimestampCeilExprMacro.java    | 15 ++++++-
 .../druid/query/expression/ExprMacroTest.java |  2 +
 .../druid/sql/calcite/CalciteQueryTest.java   | 39 +++++++++++++++++++
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java
index a775c69b796..37b35d809de 100644
--- a/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java
+++ b/processing/src/main/java/org/apache/druid/query/expression/TimestampCeilExprMacro.java
@@ -26,6 +26,7 @@ import org.apache.druid.java.util.common.granularity.PeriodGranularity;
 import org.apache.druid.math.expr.Expr;
 import org.apache.druid.math.expr.ExprEval;
 import org.apache.druid.math.expr.ExprMacroTable;
+import org.joda.time.DateTime;
 
 import javax.annotation.Nonnull;
 import java.util.List;
@@ -72,7 +73,12 @@ public class TimestampCeilExprMacro implements ExprMacroTable.ExprMacro
         // Return null if the argument if null.
         return ExprEval.of(null);
       }
-      return ExprEval.of(granularity.bucketEnd(DateTimes.utc(arg.eval(bindings).asLong())).getMillis());
+      DateTime argTime = DateTimes.utc(arg.eval(bindings).asLong());
+      DateTime bucketStartTime = granularity.bucketStart(argTime);
+      if (argTime.equals(bucketStartTime)) {
+        return ExprEval.of(bucketStartTime.getMillis());
+      }
+      return ExprEval.of(granularity.increment(bucketStartTime).getMillis());
     }
 
     @Override
@@ -107,7 +113,12 @@ public class TimestampCeilExprMacro implements ExprMacroTable.ExprMacro
     public ExprEval eval(final ObjectBinding bindings)
     {
       final PeriodGranularity granularity = getGranularity(args, bindings);
-      return ExprEval.of(granularity.bucketEnd(DateTimes.utc(args.get(0).eval(bindings).asLong())).getMillis());
+      DateTime argTime = DateTimes.utc(args.get(0).eval(bindings).asLong());
+      DateTime bucketStartTime = granularity.bucketStart(argTime);
+      if (argTime.equals(bucketStartTime)) {
+        return ExprEval.of(bucketStartTime.getMillis());
+      }
+      return ExprEval.of(granularity.increment(bucketStartTime).getMillis());
     }
 
     @Override
diff --git a/server/src/test/java/org/apache/druid/query/expression/ExprMacroTest.java b/server/src/test/java/org/apache/druid/query/expression/ExprMacroTest.java
index bbab646e564..d79cff18b47 100644
--- a/server/src/test/java/org/apache/druid/query/expression/ExprMacroTest.java
+++ b/server/src/test/java/org/apache/druid/query/expression/ExprMacroTest.java
@@ -34,6 +34,7 @@ public class ExprMacroTest
   private static final Expr.ObjectBinding BINDINGS = Parser.withMap(
       ImmutableMap.<String, Object>builder()
           .put("t", DateTimes.of("2000-02-03T04:05:06").getMillis())
+          .put("t1", DateTimes.of("2000-02-03T00:00:00").getMillis())
           .put("tstr", "2000-02-03T04:05:06")
           .put("tstr_sql", "2000-02-03 04:05:06")
           .put("x", "foo")
@@ -88,6 +89,7 @@ public class ExprMacroTest
     assertExpr("timestamp_ceil(t, 'P1D',null,'America/Los_Angeles')", DateTimes.of("2000-02-03T08").getMillis());
     assertExpr("timestamp_ceil(t, 'P1D',null,CityOfAngels)", DateTimes.of("2000-02-03T08").getMillis());
     assertExpr("timestamp_ceil(t, 'P1D','1970-01-01T01','Etc/UTC')", DateTimes.of("2000-02-04T01").getMillis());
+    assertExpr("timestamp_ceil(t1, 'P1D')", DateTimes.of("2000-02-03").getMillis());
   }
 
   @Test
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 9c8aac0499f..b130511268c 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
@@ -7906,4 +7906,43 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
         )
     );
   }
+
+  @Test
+  public void testTimestampCeil() throws Exception
+  {
+    testQuery(
+        "SELECT CEIL(TIMESTAMP '2000-01-01 00:00:00' TO DAY), \n"
+        + "CEIL(TIMESTAMP '2000-01-01 01:00:00' TO DAY) \n"
+        + "FROM druid.foo\n"
+        + "LIMIT 1",
+        ImmutableList.of(
+            newScanQueryBuilder()
+                .dataSource(CalciteTests.DATASOURCE1)
+                .intervals(querySegmentSpec(Filtration.eternity()))
+                .virtualColumns(
+                    expressionVirtualColumn("v0", "946684800000", ValueType.LONG),
+                    expressionVirtualColumn("v1", "946771200000", ValueType.LONG)
+                )
+                .columns("v0", "v1")
+                .limit(1)
+                .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+                .context(QUERY_CONTEXT_DEFAULT)
+                .build()
+
+        ),
+        ImmutableList.of(
+            new Object[]{
+                Calcites.jodaToCalciteTimestamp(
+                    DateTimes.of("2000-01-01"),
+                    DateTimeZone.UTC
+                ),
+                Calcites.jodaToCalciteTimestamp(
+                    DateTimes.of("2000-01-02"),
+                    DateTimeZone.UTC
+                )
+            }
+        )
+    );
+  }
+
 }