From 239847ef13cfdb7060c965185ea63baf6b94417b Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Mon, 1 May 2023 21:28:50 +0300
Subject: [PATCH] Use keyword as datetime-diff units (#30439)
* Use keyword as datetime-diff units
Fixes #29897.
* Fix MLv2 round trip test
Fields with :base-type :type/Text are not considered temporal expressions
therefore :datetime-diff with such arguments is not a valid expression.
Such expressions removed by the clean step of the conversion to pMBQL.
---
src/metabase/lib/schema/expression/temporal.cljc | 2 +-
.../date_time_zone_functions_test.clj | 8 ++++----
test/metabase/query_processor_test/test_mlv2.clj | 4 ----
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/metabase/lib/schema/expression/temporal.cljc b/src/metabase/lib/schema/expression/temporal.cljc
index f7b3eb4b5db..f7a881c804a 100644
--- a/src/metabase/lib/schema/expression/temporal.cljc
+++ b/src/metabase/lib/schema/expression/temporal.cljc
@@ -33,7 +33,7 @@
(mbql-clause/define-tuple-mbql-clause :datetime-diff :- :type/Integer
#_:datetime1 [:schema [:ref ::expression/temporal]]
#_:datetime2 [:schema [:ref ::expression/temporal]]
- #_:unit [:enum "year" "month" "day" "hour" "second" "millisecond" "quarter"])
+ #_:unit [:ref ::temporal-bucketing/unit.date-time.truncate])
(mbql-clause/define-tuple-mbql-clause :get-week :- :type/Integer
#_:datetime [:schema [:ref ::expression/temporal]]
diff --git a/test/metabase/query_processor_test/date_time_zone_functions_test.clj b/test/metabase/query_processor_test/date_time_zone_functions_test.clj
index e0088092544..63e3aa1f320 100644
--- a/test/metabase/query_processor_test/date_time_zone_functions_test.clj
+++ b/test/metabase/query_processor_test/date_time_zone_functions_test.clj
@@ -1043,13 +1043,13 @@
{:database (mt/id)
:type :query
:query {:filter [:and
- [:= a-str [:field "a_dt_tz_text" {:base-type :type/Text}]]
- [:= b-str [:field "b_dt_tz_text" {:base-type :type/Text}]]]
+ [:= a-str [:field "a_dt_tz_text" {:base-type :type/DateTime}]]
+ [:= b-str [:field "b_dt_tz_text" {:base-type :type/DateTime}]]]
:expressions (into {}
(for [unit units]
[(name unit) [:datetime-diff
- [:field "a_dt_tz" {:base-type :type/Text}]
- [:field "b_dt_tz" {:base-type :type/Text}]
+ [:field "a_dt_tz" {:base-type :type/DateTime}]
+ [:field "b_dt_tz" {:base-type :type/DateTime}]
unit]]))
:fields (into [] (for [unit units]
[:expression (name unit)]))
diff --git a/test/metabase/query_processor_test/test_mlv2.clj b/test/metabase/query_processor_test/test_mlv2.clj
index a2cad88d8bc..b91197c9237 100644
--- a/test/metabase/query_processor_test/test_mlv2.clj
+++ b/test/metabase/query_processor_test/test_mlv2.clj
@@ -35,10 +35,6 @@
[legacy-query]
(or
*skip-conversion-tests*
- ;; #29897: `:datetime-diff` is not handled correctly.
- (mbql.u/match-one legacy-query
- :datetime-diff
- "#29897")
;; #29904: `:fields` in `:joins` are supposed to be returned even if `:fields` is specified.
(mbql.u/match-one legacy-query
{:fields fields, :joins joins}
--
GitLab