From 9771eaa29066b8c3fc9315fb23f474d611616ee7 Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Tue, 21 May 2024 21:58:47 +0300
Subject: [PATCH] Migrate druid metrics-inside-aggregation-clauses-test to
druid-jdbc (#42653)
* Migrate druid metrics-inside-aggregation-clauses-test to druid-jdbc
* Fix test-runner: druid should not exclude druid-jdbc
* Implement set-parameter and unprepare-value for LocalDateTime for druid-jdbc
---
.../src/metabase/driver/druid_jdbc.clj | 16 +++++++++---
.../druid_jdbc/query_processor_test.clj | 26 +++++++++++++++++++
.../test/metabase/driver/druid_jdbc_test.clj | 18 +------------
.../driver/druid/query_processor_test.clj | 22 +---------------
test/metabase/test_runner.clj | 7 +++--
5 files changed, 46 insertions(+), 43 deletions(-)
create mode 100644 modules/drivers/druid-jdbc/test/metabase/driver/druid_jdbc/query_processor_test.clj
diff --git a/modules/drivers/druid-jdbc/src/metabase/driver/druid_jdbc.clj b/modules/drivers/druid-jdbc/src/metabase/driver/druid_jdbc.clj
index 2d651e0c93a..c25d2483dc8 100644
--- a/modules/drivers/druid-jdbc/src/metabase/driver/druid_jdbc.clj
+++ b/modules/drivers/druid-jdbc/src/metabase/driver/druid_jdbc.clj
@@ -21,7 +21,7 @@
[metabase.util.log :as log])
(:import
(java.sql ResultSet Types)
- (java.time ZonedDateTime)))
+ (java.time LocalDateTime ZonedDateTime)))
(set! *warn-on-reflection* true)
@@ -113,13 +113,23 @@
host)
port))
+(defn- format-datetime [t] (t/format "yyyy-MM-dd HH:mm:ss.SSS" t))
+
(defmethod sql-jdbc.execute/set-parameter [:druid-jdbc ZonedDateTime]
[driver ps i t]
- (sql-jdbc.execute/set-parameter driver ps i (t/format "yyyy-MM-dd HH:mm:ss.SSS" t)))
+ (sql-jdbc.execute/set-parameter driver ps i (format-datetime t)))
(defmethod unprepare/unprepare-value [:druid-jdbc ZonedDateTime]
[_driver t]
- (format "'%s'" (t/format "yyyy-MM-dd HH:mm:ss.SSS" t)))
+ (format "'%s'" (format-datetime t)))
+
+(defmethod sql-jdbc.execute/set-parameter [:druid-jdbc LocalDateTime]
+ [driver ps i t]
+ (sql-jdbc.execute/set-parameter driver ps i (format-datetime t)))
+
+(defmethod unprepare/unprepare-value [:druid-jdbc LocalDateTime]
+ [_driver t]
+ (format "'%s'" (format-datetime t)))
(defmethod sql.qp/json-query :druid-jdbc
[_driver unwrapped-identifier nfc-field]
diff --git a/modules/drivers/druid-jdbc/test/metabase/driver/druid_jdbc/query_processor_test.clj b/modules/drivers/druid-jdbc/test/metabase/driver/druid_jdbc/query_processor_test.clj
new file mode 100644
index 00000000000..fdbe63905d8
--- /dev/null
+++ b/modules/drivers/druid-jdbc/test/metabase/driver/druid_jdbc/query_processor_test.clj
@@ -0,0 +1,26 @@
+(ns metabase.driver.druid-jdbc.query-processor-test
+ (:require
+ [clojure.test :refer :all]
+ [metabase.models :refer [Card]]
+ [metabase.test :as mt]
+ [metabase.timeseries-query-processor-test.util :as tqpt]
+ [toucan2.tools.with-temp :as t2.with-temp]))
+
+(deftest metrics-inside-aggregation-clauses-test
+ (mt/test-driver :druid-jdbc
+ (testing "check that we can handle METRICS inside expression aggregation clauses"
+ (tqpt/with-flattened-dbdef
+ (t2.with-temp/with-temp [Card {metric-id :id} {:dataset_query
+ (mt/mbql-query checkins
+ {:aggregation [:sum $venue_price]
+ :filter [:> $venue_price 1]
+ :source-table (mt/id :checkins)})
+ :type :metric}]
+ (is (= [[2 1231]
+ [3 346]
+ [4 197]]
+ (mt/rows
+ (mt/run-mbql-query checkins
+ {:aggregation [:+ [:metric metric-id] 1]
+ :breakout [$venue_price]
+ :source-table (str "card__" metric-id)})))))))))
diff --git a/modules/drivers/druid-jdbc/test/metabase/driver/druid_jdbc_test.clj b/modules/drivers/druid-jdbc/test/metabase/driver/druid_jdbc_test.clj
index bc8ef08e744..ce14d8e66fa 100644
--- a/modules/drivers/druid-jdbc/test/metabase/driver/druid_jdbc_test.clj
+++ b/modules/drivers/druid-jdbc/test/metabase/driver/druid_jdbc_test.clj
@@ -14,8 +14,7 @@
[metabase.test :as mt]
[metabase.timeseries-query-processor-test.util :as tqpt]
[metabase.util :as u]
- [toucan2.core :as t2]
- [toucan2.tools.with-temp :as t2.with-temp]))
+ [toucan2.core :as t2]))
(set! *warn-on-reflection* true)
@@ -366,21 +365,6 @@
(mt/mbql-query checkins
{:aggregation [[:distinct [:+ $id $venue_price]]]})))))))
-(deftest metrics-inside-aggregation-clauses-test
- (tqpt/test-timeseries-drivers
- (testing "check that we can handle METRICS inside expression aggregation clauses"
- (t2.with-temp/with-temp [:model/Metric metric {:definition (mt/$ids checkins
- {:aggregation [:sum $venue_price]
- :filter [:> $venue_price 1]})
- :table_id (mt/id :checkins)}]
- (is (= [[2 1231]
- [3 346]
- [4 197]]
- (-> (mt/run-mbql-query checkins
- {:aggregation [:+ [:metric (u/the-id metric)] 1]
- :breakout [$venue_price]})
- mt/rows)))))))
-
(deftest order-by-aggregation-test
(tqpt/test-timeseries-drivers
(doseq [[direction expected-rows] {:desc [["Bar" "Felipinho Asklepios" 8]
diff --git a/modules/drivers/druid/test/metabase/driver/druid/query_processor_test.clj b/modules/drivers/druid/test/metabase/driver/druid/query_processor_test.clj
index 333e679f800..596a9bf0b95 100644
--- a/modules/drivers/druid/test/metabase/driver/druid/query_processor_test.clj
+++ b/modules/drivers/druid/test/metabase/driver/druid/query_processor_test.clj
@@ -9,7 +9,7 @@
[metabase.db.metadata-queries :as metadata-queries]
[metabase.driver :as driver]
[metabase.driver.druid.query-processor :as druid.qp]
- [metabase.models :refer [#_Card Field Table]]
+ [metabase.models :refer [Field Table]]
[metabase.query-processor :as qp]
[metabase.query-processor.compile :as qp.compile]
[metabase.test :as mt]
@@ -542,26 +542,6 @@
(druid-query
{:aggregation [[:distinct [:+ $id $checkins.venue_price]]]}))))))
-;; FIXME metrics v2
-#_(deftest metrics-inside-aggregation-clauses-test
- (mt/test-driver :druid
- (testing "check that we can handle METRICS inside expression aggregation clauses"
- (tqpt/with-flattened-dbdef
- (t2.with-temp/with-temp [Card {metric-id :id} {:dataset_query
- (mt/mbql-query checkins
- {:aggregation [:sum $venue_price]
- :filter [:> $venue_price 1]
- :source-table (mt/id :checkins)})
- :type :metric}]
- (is (= [["2" 1231.0]
- ["3" 346.0]
- ["4" 197.0]]
- (mt/rows
- (mt/run-mbql-query checkins
- {:aggregation [:+ [:metric metric-id] 1]
- :breakout [$venue_price]
- :source-table (str "card__" metric-id)})))))))))
-
(deftest order-by-aggregation-test
(mt/test-driver :druid
(doseq [[direction expected-rows] {:desc [["Bar" "Felipinho Asklepios" 8]
diff --git a/test/metabase/test_runner.clj b/test/metabase/test_runner.clj
index 95ddee9d916..2c3d2807505 100644
--- a/test/metabase/test_runner.clj
+++ b/test/metabase/test_runner.clj
@@ -58,8 +58,11 @@
(let [excluded-driver-dirs (for [driver (excluded-drivers)]
(format "modules/drivers/%s" (name driver)))
exclude-directory? (fn [dir]
- (some (partial str/includes? (str dir))
- excluded-driver-dirs))
+ (let [dir (str dir)]
+ (some (fn [excluded]
+ (or (str/ends-with? dir excluded)
+ (str/includes? dir (str excluded "/"))))
+ excluded-driver-dirs)))
directories (for [^java.io.File file (classpath/system-classpath)
:when (and (.isDirectory file)
(not (exclude-directory? file)))]
--
GitLab