Skip to content
Snippets Groups Projects
Unverified Commit 930bbc5a authored by lbrdnk's avatar lbrdnk Committed by GitHub
Browse files

Update server side relative datetime generation honeysql (#36528)

* Use date instead of timestamp

* Update logging and add code for local testing

* Rename datetime to date
parent f5d6697a
No related branches found
No related tags found
No related merge requests found
......@@ -215,7 +215,7 @@
[unit]
(contains? #{:day :week :month :quarter :year} unit))
(defn- server-side-relative-datetime-honeysql-form
(defn- server-side-relative-date-honeysql-form
"Compute `:relative-datetime` clause value server-side. Value is sql formatted (and not passed as date time) to avoid
jdbc driver's timezone adjustments. Use of `qp.timezone/now` ensures correct timezone is used for the calculation.
For details see the [[metabase.driver.redshift-test/server-side-relative-datetime-truncation-test]]."
......@@ -225,12 +225,12 @@
(u.date/truncate unit)
(u.date/add unit amount)
(u.date/format-sql))
:timestamp])
:date])
(defmethod sql.qp/->honeysql [:redshift :relative-datetime]
[driver [_ amount unit]]
(if (use-server-side-relative-datetime? unit)
(server-side-relative-datetime-honeysql-form amount unit)
(server-side-relative-date-honeysql-form amount unit)
(let [now-hsql (sql.qp/current-datetime-honeysql-form driver)]
(sql.qp/date driver unit (if (zero? amount)
now-hsql
......
......@@ -20,6 +20,7 @@
[metabase.public-settings :as public-settings]
[metabase.query-processor :as qp]
[metabase.query-processor.test-util :as qp.test-util]
[metabase.query-processor.timezone :as qp.timezone]
[metabase.sync :as sync]
[metabase.test :as mt]
[metabase.test.data.interface :as tx]
......@@ -472,7 +473,16 @@
sql (sql/format honey)
result (apply run-native-query sql)
[db-generated ss-generated] (-> result mt/rows first)]
(is (= db-generated ss-generated))))))
(is (= db-generated ss-generated)
(format (str "\n---\n"
"results timezone == %s\n"
"report timezone == %s\n"
"database timezone == %s\n"
"system timezone == %s\n")
(qp.timezone/results-timezone-id)
(qp.timezone/report-timezone-id-if-supported)
(qp.timezone/database-timezone-id)
(qp.timezone/system-timezone-id)))))))
(deftest server-side-relative-datetime-test
(mt/test-driver
......@@ -505,6 +515,69 @@
(let [test-thunk (getdate-vs-ss-ts-test-thunk-generator)]
(test-thunk))))))))))
(comment
;; For local testing.
(defn- generate-tz-tests [fs tzs thunks]
(letfn [(combine-acc [acc [f & fs*] tzs]
(if (nil? f)
acc
(recur (into acc (mapcat (fn [thunk*] (for [tz tzs] #(f tz thunk*)))) acc)
fs*
tzs)))]
(combine-acc (vec thunks) fs tzs)))
;; For local testing.
(defn- generate-test-thunks
"Generate test thunks of product units x tests"
[units values]
(for [unit units
value values
:let [test-thunk (getdate-vs-ss-ts-test-thunk-generator unit value)]]
(fn []
(testing (format "unit = %s, value = %s" unit value)
(test-thunk)))))
;; Following is meant for local testing and not to be run in the CI. Feel free to tweak the values.
(deftest tz-settings-combinations-test
(mt/test-driver
:redshift
(mt/with-metadata-provider (mt/id)
(let [thunks-with-various-values-units (generate-test-thunks [:day :week :month] [-1 0 1])
tz-setters [qp.test-util/do-with-report-timezone-id
test.tz/do-with-system-timezone-id]
tzs ["America/Los_Angeles"
"Europe/Prague"]
testcases (generate-tz-tests tz-setters tzs thunks-with-various-values-units)]
#_{:clj-kondo/ignore [:discouraged-var]}
(time (doseq [testcase testcases]
(time (testcase))))))))
)
(comment
(def tz-setters [qp.test-util/do-with-report-timezone-id
test.tz/do-with-system-timezone-id
qp.test-util/do-with-database-timezone-id
qp.test-util/do-with-results-timezone-id])
(def tzs ["America/Los_Angeles"
"Europe/Prague"
"UTC"])
(-> (generate-test-thunks [:day :week :month :year] [-30 -7 -1 0 1 7 30]) count)
;; => 28
(->> (generate-test-thunks [:day :week :month :year] [-30 -7 -1 0 1 7 30])
(generate-tz-tests tz-setters tzs)
count)
;; => 7168
(def some-tz-setters [qp.test-util/do-with-report-timezone-id
test.tz/do-with-system-timezone-id])
(def some-tzs ["America/Los_Angeles"
"Europe/Prague"])
(->> (generate-test-thunks [:day :week :month :year] [-1 0 1])
(generate-tz-tests some-tz-setters some-tzs)
count)
;; => 108
)
(deftest server-side-relative-datetime-various-units-test
(mt/test-driver
:redshift
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment