Skip to content
Snippets Groups Projects
Unverified Commit 73e7a053 authored by Bryan Maass's avatar Bryan Maass Committed by GitHub
Browse files

Fix date filter + relative date + offset bug (#25713) 


* untangled date time ranges (and filters)

* more tests for relative dates

* Update test/metabase/driver/common/parameters/dates_test.clj

Co-authored-by: default avatarCam Saul <1455846+camsaul@users.noreply.github.com>

* respond to review + remove an extra tap>

* remove java time

* with-clock is not allowed inside parallel tests.

Co-authored-by: default avatarCam Saul <1455846+camsaul@users.noreply.github.com>
parent 0b469bca
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/driver/common/parameters/dates.clj
+ 19
− 12
View file @ 73e7a053
......@@ -145,6 +145,12 @@
(def ^:private temporal-units-regex #"(millisecond|second|minute|hour|day|week|month|quarter|year)")
(def ^:private relative-suffix-regex (re-pattern (format "(|~|-from-([0-9]+)%ss)" temporal-units-regex)))
(defn- include-current?
"Adding a tilde (~) at the end of a past<n><unit>s filter means we should include the current time-unit (e.g. year, day,
week, or month)."
[relative-suffix]
(= "~" relative-suffix))
(def ^:private relative-date-string-decoders
[{:parser #(= % "today")
:range (fn [_ dt]
......@@ -172,34 +178,35 @@
{:parser (regex->parser (re-pattern (str #"past([0-9]+)" temporal-units-regex #"s" relative-suffix-regex))
[:int-value :unit :relative-suffix :int-value-1 :unit-1])
:range (fn [{:keys [unit int-value unit-range to-period relative-suffix unit-1 int-value-1]} dt]
(let [dt-off (cond-> dt
unit-1 (t/minus ((get-in operations-by-date-unit [unit-1 :to-period]) int-value-1)))
dt-res (maybe-reduce-resolution unit dt-off)]
(unit-range (t/minus dt-res (to-period int-value))
(t/minus dt-res (to-period (if (seq relative-suffix) 0 1))))))
(let [dt-offset (cond-> dt
unit-1 (t/minus ((get-in operations-by-date-unit [unit-1 :to-period]) int-value-1)))
dt-resolution (maybe-reduce-resolution unit dt-offset)]
(unit-range (t/minus dt-resolution (to-period int-value))
(t/minus dt-resolution (to-period (if (include-current? relative-suffix) 0 1))))))
:filter (fn [{:keys [unit int-value relative-suffix unit-1 int-value-1]} field-clause]
(if unit-1
[:between
[:+ field-clause [:interval int-value-1 (keyword unit-1)]]
[:relative-datetime (- int-value) (keyword unit)]
[:relative-datetime 0 (keyword unit)]]
[:time-interval field-clause (- int-value) (keyword unit) {:include-current (boolean (seq relative-suffix))}]))}
[:time-interval field-clause (- int-value) (keyword unit) {:include-current (include-current? relative-suffix)}]))}
{:parser (regex->parser (re-pattern (str #"next([0-9]+)" temporal-units-regex #"s" relative-suffix-regex))
[:int-value :unit :relative-suffix :int-value-1 :unit-1])
:range (fn [{:keys [unit int-value unit-range to-period relative-suffix unit-1 int-value-1]} dt]
(let [dt-off (cond-> dt
unit-1 (t/plus ((get-in operations-by-date-unit [unit-1 :to-period]) int-value-1)))
dt-res (maybe-reduce-resolution unit dt-off)]
(unit-range (t/plus dt-res (to-period (if (seq relative-suffix) 0 1)))
(t/plus dt-res (to-period int-value)))))
(let [dt-offset (cond-> dt
unit-1 (t/plus ((get-in operations-by-date-unit [unit-1 :to-period]) int-value-1)))
dt-resolution (maybe-reduce-resolution unit dt-offset)]
(unit-range (t/plus dt-resolution (to-period (if (include-current? relative-suffix) 0 1)))
(t/plus dt-resolution (to-period int-value)))))
:filter (fn [{:keys [unit int-value relative-suffix unit-1 int-value-1]} field-clause]
(if unit-1
[:between
[:+ field-clause [:interval (- int-value-1) (keyword unit-1)]]
[:relative-datetime 0 (keyword unit)]
[:relative-datetime int-value (keyword unit)]]
[:time-interval field-clause int-value (keyword unit) {:include-current (boolean (seq relative-suffix))}]))}
[:time-interval field-clause int-value (keyword unit) {:include-current (include-current? relative-suffix)}]))}
{:parser (regex->parser (re-pattern (str #"last" temporal-units-regex))
[:unit])
......
test/metabase/driver/common/parameters/dates_test.clj
+ 97
− 57
View file @ 73e7a053
(ns metabase.driver.common.parameters.dates-test
(:require [clojure.test :refer :all]
[java-time :as t]
[clojure.test.check :as tc]
[clojure.test.check.generators :as gen]
[clojure.test.check.properties :as prop]
[metabase.driver.common.parameters.dates :as params.dates]
[metabase.test :as mt]
[metabase.util.date-2 :as u.date]))
(deftest ^:parallel date-string->filter-test
(deftest date-string->filter-test
(testing "year and month"
(is (= [:between
[:field "field" {:base-type :type/DateTime, :temporal-unit :day}]
......@@ -75,7 +77,7 @@
[:relative-datetime 7 :hour]]
(params.dates/date-string->filter "next7hours-from-13months" [:field "field" {:base-type :type/DateTime}]))))
(testing "exclusions"
(t/with-clock (t/mock-clock #t "2016-06-07T12:13:55Z")
(mt/with-clock #t "2016-06-07T12:13:55Z"
(testing "hours"
(is (= [:!=
[:field "field" {:base-type :type/DateTime, :temporal-unit :hour-of-day}]
......@@ -125,63 +127,72 @@
(params.dates/date-string->filter "exclude-minutes-15-30" [:field "field" {:base-type :type/DateTime}])))))))
(deftest date-string->range-test
(t/with-clock (t/mock-clock #t "2016-06-07T12:13:55Z")
(mt/with-clock #t "2016-06-07T12:13:55Z"
(doseq [[group s->expected]
{"absolute datetimes" {"Q1-2016" {:end "2016-03-31", :start "2016-01-01"}
"2016-02" {:end "2016-02-29", :start "2016-02-01"}
"2016-04-18" {:end "2016-04-18", :start "2016-04-18"}
"2016-04-18~2016-04-23" {:end "2016-04-23", :start "2016-04-18"}
{"absolute datetimes" {"Q1-2016" {:start "2016-01-01", :end "2016-03-31"}
"2016-02" {:start "2016-02-01", :end "2016-02-29"}
"2016-04-18" {:start "2016-04-18", :end "2016-04-18"}
"2016-04-18~2016-04-23" {:start "2016-04-18", :end "2016-04-23"}
"2016-04-18~" {:start "2016-04-18"}
"~2016-04-18" {:end "2016-04-18"}}
"relative (past)" {"past30seconds" {:end "2016-06-07T12:13:54", :start "2016-06-07T12:13:25"}
"past5minutes~" {:end "2016-06-07T12:13:00", :start "2016-06-07T12:08:00"}
"past3hours" {:end "2016-06-07T11:00:00", :start "2016-06-07T09:00:00"}
"past3days" {:end "2016-06-06", :start "2016-06-04"}
"past3days~" {:end "2016-06-07", :start "2016-06-04"}
"past7days" {:end "2016-06-06", :start "2016-05-31"}
"past30days" {:end "2016-06-06", :start "2016-05-08"}
"past2months" {:end "2016-05-31", :start "2016-04-01"}
"past2months~" {:end "2016-06-30", :start "2016-04-01"}
"past13months" {:end "2016-05-31", :start "2015-05-01"}
"past2quarters" {:end "2016-03-31", :start "2015-10-01"}
"past2quarters~" {:end "2016-06-30", :start "2015-10-01"}
"past1years" {:end "2015-12-31", :start "2015-01-01"}
"past1years~" {:end "2016-12-31", :start "2015-01-01"}
"past16years" {:end "2015-12-31", :start "2000-01-01"}}
"relative (next)" {"next45seconds" {:end "2016-06-07T12:14:40", :start "2016-06-07T12:13:56"}
"next20minutes" {:end "2016-06-07T12:33:00", :start "2016-06-07T12:14:00"}
"next6hours" {:end "2016-06-07T18:00:00", :start "2016-06-07T13:00:00"}
"next3days" {:end "2016-06-10", :start "2016-06-08"}
"next3days~" {:end "2016-06-10", :start "2016-06-07"}
"next7days" {:end "2016-06-14", :start "2016-06-08"}
"next30days" {:end "2016-07-07", :start "2016-06-08"}
"next2months" {:end "2016-08-31", :start "2016-07-01"}
"next2months~" {:end "2016-08-31", :start "2016-06-01"}
"next2quarters" {:end "2016-12-31", :start "2016-07-01"}
"next2quarters~" {:end "2016-12-31", :start "2016-04-01"}
"next13months" {:end "2017-07-31", :start "2016-07-01"}
"next1years" {:end "2017-12-31", :start "2017-01-01"}
"next1years~" {:end "2017-12-31", :start "2016-01-01"}
"next16years" {:end "2032-12-31", :start "2017-01-01"}}
"relative (this)" {"thissecond" {:end "2016-06-07T12:13:55", :start "2016-06-07T12:13:55"}
"thisminute" {:end "2016-06-07T12:13:00", :start "2016-06-07T12:13:00"}
"thishour" {:end "2016-06-07T12:00:00", :start "2016-06-07T12:00:00"}
"thisday" {:end "2016-06-07", :start "2016-06-07"}
"thisweek" {:end "2016-06-11", :start "2016-06-05"}
"thismonth" {:end "2016-06-30", :start "2016-06-01"}
"thisquarter" {:end "2016-06-30", :start "2016-04-01"}
"thisyear" {:end "2016-12-31", :start "2016-01-01"}}
"relative (last)" {"lastsecond" {:end "2016-06-07T12:13:54", :start "2016-06-07T12:13:54"}
"lastminute" {:end "2016-06-07T12:12:00", :start "2016-06-07T12:12:00"}
"lasthour" {:end "2016-06-07T11:00:00", :start "2016-06-07T11:00:00"}
"lastweek" {:end "2016-06-04", :start "2016-05-29"}
"lastmonth" {:end "2016-05-31", :start "2016-05-01"}
"lastquarter" {:end "2016-03-31", :start "2016-01-01"}
"lastyear" {:end "2015-12-31", :start "2015-01-01"}}
"relative (today/yesterday)" {"yesterday" {:end "2016-06-06", :start "2016-06-06"}
"today" {:end "2016-06-07", :start "2016-06-07"}}
"relative (past) with starting from" {"past3days-from-3years" {:end "2013-06-07", :start "2013-06-04"}}
"relative (next) with starting from" {"next7hours-from-13months" {:end "2017-07-07T19:00:00", :start "2017-07-07T12:00:00"}}}]
"relative (past)" {"past30seconds" {:start "2016-06-07T12:13:25", :end "2016-06-07T12:13:54"}
"past5minutes~" {:start "2016-06-07T12:08:00", :end "2016-06-07T12:13:00"}
"past3hours" {:start "2016-06-07T09:00:00", :end "2016-06-07T11:00:00"}
"past3days" {:start "2016-06-04", :end "2016-06-06"}
"past3days~" {:start "2016-06-04", :end "2016-06-07"}
"past7days" {:start "2016-05-31", :end "2016-06-06"}
"past30days" {:start "2016-05-08", :end "2016-06-06"}
"past2months" {:start "2016-04-01", :end "2016-05-31"}
"past2months~" {:start "2016-04-01", :end "2016-06-30"}
"past13months" {:start "2015-05-01", :end "2016-05-31"}
"past2quarters" {:start "2015-10-01", :end "2016-03-31"}
"past2quarters~" {:start "2015-10-01", :end "2016-06-30"}
"past1years" {:start "2015-01-01", :end "2015-12-31"}
"past1years~" {:start "2015-01-01", :end "2016-12-31"}
"past16years" {:start "2000-01-01", :end "2015-12-31"}}
"relative (next)" {"next45seconds" {:start "2016-06-07T12:13:56", :end "2016-06-07T12:14:40"}
"next20minutes" {:start "2016-06-07T12:14:00", :end "2016-06-07T12:33:00"}
"next6hours" {:start "2016-06-07T13:00:00", :end "2016-06-07T18:00:00"}
"next3days" {:start "2016-06-08", :end "2016-06-10"}
"next3days~" {:start "2016-06-07", :end "2016-06-10"}
"next7days" {:start "2016-06-08", :end "2016-06-14"}
"next30days" {:start "2016-06-08", :end "2016-07-07"}
"next2months" {:start "2016-07-01", :end "2016-08-31"}
"next2months~" {:start "2016-06-01", :end "2016-08-31"}
"next2quarters" {:start "2016-07-01", :end "2016-12-31"}
"next2quarters~" {:start "2016-04-01", :end "2016-12-31"}
"next13months" {:start "2016-07-01", :end "2017-07-31"}
"next1years" {:start "2017-01-01", :end "2017-12-31"}
"next1years~" {:start "2016-01-01", :end "2017-12-31"}
"next16years" {:start "2017-01-01", :end "2032-12-31"}}
"relative (this)" {"thissecond" {:start "2016-06-07T12:13:55", :end "2016-06-07T12:13:55"}
"thisminute" {:start "2016-06-07T12:13:00", :end "2016-06-07T12:13:00"}
"thishour" {:start "2016-06-07T12:00:00", :end "2016-06-07T12:00:00"}
"thisday" {:start "2016-06-07", :end "2016-06-07"}
"thisweek" {:start "2016-06-05", :end "2016-06-11"}
"thismonth" {:start "2016-06-01", :end "2016-06-30"}
"thisquarter" {:start "2016-04-01", :end "2016-06-30"}
"thisyear" {:start "2016-01-01", :end "2016-12-31"}}
"relative (last)" {"lastsecond" {:start "2016-06-07T12:13:54", :end "2016-06-07T12:13:54"}
"lastminute" {:start "2016-06-07T12:12:00", :end "2016-06-07T12:12:00"}
"lasthour" {:start "2016-06-07T11:00:00", :end "2016-06-07T11:00:00"}
"lastweek" {:start "2016-05-29", :end "2016-06-04"}
"lastmonth" {:start "2016-05-01", :end "2016-05-31"}
"lastquarter" {:start "2016-01-01", :end "2016-03-31"}
"lastyear" {:start "2015-01-01", :end "2015-12-31"}}
"relative (today/yesterday)" {"yesterday" {:start "2016-06-06", :end "2016-06-06"}
"today" {:start "2016-06-07", :end "2016-06-07"}}
"relative (past) with starting from" {"past1days-from-0days" {:start "2016-06-06", :end "2016-06-06"}
"past1months-from-0months" {:start "2016-05-01", :end "2016-05-31"}
"past1months-from-36months" {:start "2013-05-01", :end "2013-05-31"}
"past1years-from-36months" {:start "2012-01-01", :end "2012-12-31"}
"past3days-from-3years" {:start "2013-06-04", :end "2013-06-06"}}
"relative (next) with starting from" {"next2days-from-1months" {:start "2016-07-08", :end "2016-07-09"}
"next1months-from-0months" {:start "2016-07-01", :end "2016-07-31"}
"next1months-from-36months" {:start "2019-07-01", :end "2019-07-31"}
"next1years-from-36months" {:start "2020-01-01", :end "2020-12-31"}
"next3days-from-3years" {:start "2019-06-08", :end "2019-06-10"}
"next7hours-from-13months" {:start "2017-07-07T13:00:00", :end "2017-07-07T19:00:00"}}}]
(testing group
(doseq [[s inclusive-range] s->expected
[options range-xform] (letfn [(adjust [m k amount]
......@@ -201,6 +212,35 @@
(params.dates/date-string->range s options))
(format "%s with options %s should parse to %s" (pr-str s) (pr-str options) (pr-str expected))))))))
(deftest relative-dates-with-starting-from-zero-must-match
(testing "relative dates need to behave the same way, offset or not."
(mt/with-clock #t "2016-06-07T12:13:55Z"
(testing "'past1months-from-0months' should be the same as: 'past1months'"
(is (= {:start "2016-05-01", :end "2016-05-31"}
(params.dates/date-string->range "past1months")
(params.dates/date-string->range "past1months-from-0months"))))
(testing "'next1months-from-0months' should be the same as: 'next1months'"
(is (= {:start "2016-07-01", :end "2016-07-31"}
(params.dates/date-string->range "next1months")
(params.dates/date-string->range "next1months-from-0months")))))))
(def time-range-generator
(let [time-units (mapv #(str % "s") (keys @#'params.dates/operations-by-date-unit))]
(gen/fmap
(fn [[frame n unit unit2]]
[(str frame n unit)
(str frame n unit "-from-0" unit2)])
(gen/tuple
(gen/elements #{"next" "past"})
(gen/such-that #(not= % 0) gen/pos-int)
(gen/elements time-units)
(gen/elements time-units)))))
(tc/quick-check 1000
(prop/for-all [[tr tr+from-zero] time-range-generator]
(= (params.dates/date-string->range tr)
(params.dates/date-string->range tr+from-zero))))
(deftest custom-start-of-week-test
(testing "Relative filters should respect the custom `start-of-week` Setting (#14294)"
(mt/with-clock #t "2021-03-01T14:15:00-08:00[US/Pacific]"
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment