Merge pull request #4101 from metabase/fix-past-relative-date-ranges

Fix past relative date ranges

src/metabase/query_processor/expand.clj

@@ -301,20 +301,19 @@
     (filter {} (time-interval (field-id 100) :current :day)) "
   [f n unit]
   (if-not (integer? n)
-    (let [n (normalize-token n)]
-      (case n
-        :current (recur f  0 unit)
-        :last    (recur f -1 unit)
-        :next    (recur f  1 unit)))
+    (case (normalize-token n)
+      :current (recur f  0 unit)
+      :last    (recur f -1 unit)
+      :next    (recur f  1 unit))
     (let [f (datetime-field f unit)]
       (cond
         (core/= n  0) (= f (value f (relative-datetime :current)))
         (core/= n -1) (= f (value f (relative-datetime -1 unit)))
         (core/= n  1) (= f (value f (relative-datetime  1 unit)))
-        (core/< n -1) (between f (value f (relative-datetime (dec n) unit))
-                                 (value f (relative-datetime      -1 unit)))
-        (core/> n  1) (between f (value f (relative-datetime       1 unit))
-                               (value f (relative-datetime (inc n) unit)))))))
+        (core/< n -1) (between f (value f (relative-datetime  n unit))
+                                 (value f (relative-datetime -1 unit)))
+        (core/> n  1) (between f (value f (relative-datetime  1 unit))
+                                 (value f (relative-datetime  n unit)))))))
 
 (s/defn ^:ql ^:always-validate filter
   "Filter the results returned by the query.

test/metabase/query_processor_test/date_bucketing_test.clj

@@ -337,12 +337,13 @@
 ;; that actually reflects the units the results are in.
 ;; eg when we breakout by one unit and filter by another, make sure the results and the col info
 ;; use the unit used by breakout
-(defn- date-bucketing-unit-when-you [& {:keys [breakout-by filter-by]}]
+(defn- date-bucketing-unit-when-you [& {:keys [breakout-by filter-by with-interval]
+                                        :or   {with-interval :current}}]
   (let [results (data/with-temp-db [_ (checkins:1-per-day)]
                   (data/run-query checkins
                     (ql/aggregation (ql/count))
                     (ql/breakout (ql/datetime-field $timestamp breakout-by))
-                    (ql/filter (ql/time-interval $timestamp :current filter-by))))]
+                    (ql/filter (ql/time-interval $timestamp with-interval filter-by))))]
     {:rows (or (-> results :row_count)
                (throw (ex-info "Query failed!" results)))
      :unit (-> results :data :cols first :unit)}))
@@ -366,3 +367,12 @@
 (expect-with-non-timeseries-dbs-except #{:bigquery}
   {:rows 1, :unit :hour}
   (date-bucketing-unit-when-you :breakout-by "hour", :filter-by "day"))
+
+;; make sure if you use a relative date bucket in the past (e.g. "past 2 months") you get the correct amount of rows (#3910)
+(expect-with-non-timeseries-dbs-except #{:bigquery}
+  {:rows 2, :unit :day}
+  (date-bucketing-unit-when-you :breakout-by "day", :filter-by "day", :with-interval -2))
+
+(expect-with-non-timeseries-dbs-except #{:bigquery}
+  {:rows 2, :unit :day}
+  (date-bucketing-unit-when-you :breakout-by "day", :filter-by "day", :with-interval 2))