From 9fdb17986b91c91ed998a4d472747442295554e6 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Thu, 24 Mar 2022 16:12:22 -0700
Subject: [PATCH] Fix `:postgres` filtering by current quarter; remove a few
unneeded casts in PG SQL output (#21204)
* Fix Postgres filter by quarter
* Fix test for MongoDB to work around #5419
* Spark SQL doesn't allow quarters in intervals either
---
.../20683-postgres-current-quarter.cy.spec.js | 2 +-
.../src/metabase/driver/hive_like.clj | 6 ++-
src/metabase/driver/postgres.clj | 14 +++++--
test/metabase/driver/postgres_test.clj | 42 +++++++++----------
.../date_bucketing_test.clj | 18 +++++++-
5 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/frontend/test/metabase/scenarios/question/reproductions/20683-postgres-current-quarter.cy.spec.js b/frontend/test/metabase/scenarios/question/reproductions/20683-postgres-current-quarter.cy.spec.js
index 4c0717a1a44..4bb803ac947 100644
--- a/frontend/test/metabase/scenarios/question/reproductions/20683-postgres-current-quarter.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/reproductions/20683-postgres-current-quarter.cy.spec.js
@@ -1,6 +1,6 @@
import { restore, visualize } from "__support__/e2e/cypress";
-describe.skip("issue 20683", () => {
+describe("issue 20683", () => {
beforeEach(() => {
restore("postgres-12");
cy.signInAsAdmin();
diff --git a/modules/drivers/sparksql/src/metabase/driver/hive_like.clj b/modules/drivers/sparksql/src/metabase/driver/hive_like.clj
index ba999831def..359c68739ed 100644
--- a/modules/drivers/sparksql/src/metabase/driver/hive_like.clj
+++ b/modules/drivers/sparksql/src/metabase/driver/hive_like.clj
@@ -146,8 +146,10 @@
(hsql/call :percentile (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver p)))
(defmethod sql.qp/add-interval-honeysql-form :hive-like
- [_ hsql-form amount unit]
- (hx/+ (hx/->timestamp hsql-form) (hsql/raw (format "(INTERVAL '%d' %s)" (int amount) (name unit)))))
+ [driver hsql-form amount unit]
+ (if (= unit :quarter)
+ (recur driver hsql-form (* amount 3) :month)
+ (hx/+ (hx/->timestamp hsql-form) (hsql/raw (format "(INTERVAL '%d' %s)" (int amount) (name unit))))))
(def ^:dynamic *param-splice-style*
"How we should splice params into SQL (i.e. 'unprepare' the SQL). Either `:friendly` (the default) or `:paranoid`.
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 8b3c0d68342..690f7bbdd08 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -44,9 +44,13 @@
(hx/cast-unless-type-in "timestamp" #{"timestamp" "timestamptz" "date"} honeysql-form))
(defmethod sql.qp/add-interval-honeysql-form :postgres
- [_ hsql-form amount unit]
- (hx/+ (->timestamp hsql-form)
- (hsql/raw (format "(INTERVAL '%s %s')" amount (name unit)))))
+ [driver hsql-form amount unit]
+ ;; Postgres doesn't support quarter in intervals (#20683)
+ (if (= unit :quarter)
+ (recur driver hsql-form (* 3 amount) :month)
+ (let [hsql-form (->timestamp hsql-form)]
+ (-> (hx/+ hsql-form (hsql/raw (format "(INTERVAL '%s %s')" amount (name unit))))
+ (hx/with-type-info (hx/type-info hsql-form))))))
(defmethod driver/humanize-connection-error-message :postgres
[_ message]
@@ -263,6 +267,10 @@
;;; | metabase.driver.sql impls |
;;; +----------------------------------------------------------------------------------------------------------------+
+(defmethod sql.qp/current-datetime-honeysql-form :postgres
+ [_driver]
+ (hx/with-database-type-info :%now "timestamptz"))
+
(defmethod sql.qp/unix-timestamp->honeysql [:postgres :seconds]
[_ _ expr]
(hsql/call :to_timestamp expr))
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index 9226cbaaaba..b8dbf43fd8d 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -47,7 +47,7 @@
;;; ----------------------------------------------- Connection Details -----------------------------------------------
-(deftest connection-details->spec-test
+(deftest ^:parallel connection-details->spec-test
(testing (str "Check that SSL params get added the connection details in the way we'd like # no SSL -- this should "
"*not* include the key :ssl (regardless of its value) since that will cause the PG driver to use SSL "
"anyway")
@@ -285,7 +285,7 @@
(is (= :type/SerializedJSON
(db/select-one-field :semantic_type Field, :id (mt/id :venues :address))))))))
-(deftest json-query-test
+(deftest ^:parallel json-query-test
(testing "Transforming MBQL query with JSON in it to postgres query works"
(is (= ["json_extract_path_text(CAST(bleh AS json), (CAST(? AS text)))" "meh"]
(hsql/format (#'postgres/json-query 'mlep [:bleh :meh])))))
@@ -313,28 +313,28 @@
(let [details (mt/dbdef->connection-details :postgres :db {:database-name "describe-json-test"})
spec (sql-jdbc.conn/connection-details->spec :postgres details)]
(jdbc/execute! spec [(str "CREATE TABLE describe_json_table (coherent_json_val JSON NOT NULL, incoherent_json_val JSON NOT NULL);"
- "INSERT INTO describe_json_table (coherent_json_val, incoherent_json_val) VALUES ('{\"a\": 1, \"b\": 2}', '{\"a\": 1, \"b\": 2}');"
- "INSERT INTO describe_json_table (coherent_json_val, incoherent_json_val) VALUES ('{\"a\": 2, \"b\": 3}', '{\"a\": [1, 2], \"b\": 2}');")])
+ "INSERT INTO describe_json_table (coherent_json_val, incoherent_json_val) VALUES ('{\"a\": 1, \"b\": 2}', '{\"a\": 1, \"b\": 2}');"
+ "INSERT INTO describe_json_table (coherent_json_val, incoherent_json_val) VALUES ('{\"a\": 2, \"b\": 3}', '{\"a\": [1, 2], \"b\": 2}');")])
(mt/with-temp Database [database {:engine :postgres, :details details}]
(is (= '#{{:name "incoherent_json_val → b",
- :database-type nil,
- :base-type :type/Integer,
- :database-position 0,
- :nfc-path [:incoherent_json_val "b"]}
- {:name "coherent_json_val → a",
- :database-type nil,
- :base-type :type/Integer,
- :database-position 0,
- :nfc-path [:coherent_json_val "a"]}
- {:name "coherent_json_val → b",
- :database-type nil,
- :base-type :type/Integer,
- :database-position 0,
- :nfc-path [:coherent_json_val "b"]}}
+ :database-type nil,
+ :base-type :type/Integer,
+ :database-position 0,
+ :nfc-path [:incoherent_json_val "b"]}
+ {:name "coherent_json_val → a",
+ :database-type nil,
+ :base-type :type/Integer,
+ :database-position 0,
+ :nfc-path [:coherent_json_val "a"]}
+ {:name "coherent_json_val → b",
+ :database-type nil,
+ :base-type :type/Integer,
+ :database-position 0,
+ :nfc-path [:coherent_json_val "b"]}}
(sql-jdbc.sync/describe-nested-field-columns
- :postgres
- database
- {:name "describe_json_table"}))))))))
+ :postgres
+ database
+ {:name "describe_json_table"}))))))))
(mt/defdataset with-uuid
[["users"
diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
index 963fc20d7e5..ad799c40712 100644
--- a/test/metabase/query_processor_test/date_bucketing_test.clj
+++ b/test/metabase/query_processor_test/date_bucketing_test.clj
@@ -1034,7 +1034,8 @@
(mt/test-drivers (mt/normal-drivers-except #{:snowflake})
(testing "if datetime string is not yyyy-MM-dd no date bucketing should take place, and thus we should get no (exact) matches"
(mt/dataset checkins:1-per-day
- (is (= ;; Mongo returns empty row for count = 0. We should fix that (#5419)
+ (is (=
+ ;; Mongo returns empty row for count = 0. We should fix that (#5419)
(case driver/*driver*
:mongo []
[[0]])
@@ -1160,3 +1161,18 @@
{:aggregation [[:count]]
:breakout [!day-of-week.date]
:filter [:between $date "2013-01-03" "2013-01-20"]}))))))))))
+
+(deftest filter-by-current-quarter-test
+ (mt/test-drivers (mt/normal-drivers)
+ (testing "Should be able to filter by current quarter (#20683)"
+ (let [query (mt/mbql-query checkins
+ {:aggregation [[:count]]
+ :filter [:= !quarter.date [:relative-datetime :now]]})]
+ (mt/with-native-query-testing-context query
+ ;; this isn't expected to return anything; for now it's enough just to make sure that the query doesn't fail.
+ (is (=
+ ;; Mongo returns empty row for count = 0. We should fix that (#5419)
+ (case driver/*driver*
+ :mongo []
+ [[0]])
+ (mt/formatted-rows [int] (qp/process-query query)))))))))
--
GitLab