Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

Commit

Permalink
Use Java 8 java.time classes in MBQL (#23)
Browse files Browse the repository at this point in the history 
* future-timezone-support [ci skip] [WIP]

* Bump version -> 1.4.0

* Update relative-date and add-datetime-units to use java.time

* Test fixes 🔧
  • Loading branch information
@camsaul
camsaul committed Nov 4, 2019
1 parent 1304ed7 commit a4ec087
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 71 deletions.
5 changes: 3 additions & 2 deletions project.clj
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
(defproject metabase/mbql "1.3.6"
(defproject metabase/mbql "1.4.0"
:description "Shared things used across several Metabase projects, such as i18n and config."
:url "https://github.com/metabase/mbql"
:min-lein-version "2.5.0"
Expand All @@ -18,6 +18,7 @@

:dependencies
[[org.clojure/core.match "0.3.0"]
[clojure.java-time "0.3.2"]
[medley "1.2.0"]
[metabase/common "1.0.4"]
[metabase/schema-util "1.0.2"]
Expand All @@ -43,7 +44,7 @@

:eastwood
{:plugins
[[jonase/eastwood "0.3.5" :exclusions [org.clojure/clojure]]]
[[jonase/eastwood "0.3.6" :exclusions [org.clojure/clojure]]]

:add-linters
[:unused-private-vars
Expand Down
8 changes: 6 additions & 2 deletions src/metabase/mbql/schema.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,11 @@
;; becomes:
;; [:= [:datetime-field [:field-id 10] :day] [:absolute-datetime #inst "2018-10-02" :day]]
(defclause ^:internal absolute-datetime
timestamp java.sql.Timestamp
timestamp (s/cond-pre java.time.Instant
java.time.LocalDate
java.time.LocalDateTime
java.time.OffsetDateTime
java.time.ZonedDateTime)
unit DatetimeFieldUnit)

;; it could make sense to say hour-of-day(field) = hour-of-day("2018-10-10T12:00")
Expand All @@ -114,7 +118,7 @@
;; clearly a time (e.g. "08:00:00.000") and/or the Field derived from `:type/Time` and/or the unit was a
;; time-bucketing unit
(defclause ^:internval time
time java.sql.Time
time (s/cond-pre java.time.LocalTime java.time.OffsetTime)
unit TimeUnit)

(def ^:private DatetimeLiteral
Expand Down
49 changes: 22 additions & 27 deletions src/metabase/mbql/util.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
"Utilitiy functions for working with MBQL queries."
(:refer-clojure :exclude [replace])
(:require [clojure.string :as str]
[java-time
[amount :as t.amount]
[core :as t.core]]
[metabase.common.i18n :refer [tru]]
[metabase.mbql.schema :as mbql.s]
[metabase.mbql.util.match :as mbql.match]
[metabase.util.schema :as su]
[schema.core :as s])
(:import java.sql.Timestamp
[java.util Calendar TimeZone]))
[schema.core :as s]))

(defn qualified-name
"Like `name`, but if `x` is a namespace-qualified keyword, returns that a string including the namespace."
Expand Down Expand Up @@ -461,28 +462,25 @@
;; otherwise add new clause at the end
(update inner-query :order-by (comp vec conj) order-by-clause))))


;; TODO - we should just use `Instant` or `ZonedDateTime` for this
(defn relative-date
"Return a new Timestamp relative to the current time using a relative date `unit`.
(relative-date :year -1) -> #inst 2014-11-12 ..."
^java.sql.Timestamp [unit amount, ^Timestamp timestamp]
(let [cal (doto (Calendar/getInstance)
(.setTimeZone (TimeZone/getTimeZone "UTC"))
(.setTime timestamp))
[unit multiplier] (case unit
:second [Calendar/SECOND 1]
:minute [Calendar/MINUTE 1]
:hour [Calendar/HOUR 1]
:day [Calendar/DATE 1]
:week [Calendar/DATE 7]
:month [Calendar/MONTH 1]
:quarter [Calendar/MONTH 3]
:year [Calendar/YEAR 1])]
(.set cal unit (+ (.get cal unit)
(* amount multiplier)))
(java.sql.Timestamp. (.getTime (.getTime cal)))))
"Return a new Temporal value relative to `t` using a relative date `unit`.
(relative-date :year -1 (t/zoned-date-time \"2019-11-04T10:57:00-08:00[America/Los_Angeles]\"))
;; ->
(t/zoned-date-time \"2020-11-04T10:57-08:00[America/Los_Angeles]\")"
^java.time.temporal.Temporal [unit amount t]
(if (zero? amount)
t
(t.core/plus t (case unit
:millisecond (t.amount/millis amount)
:second (t.amount/seconds amount)
:minute (t.amount/minutes amount)
:hour (t.amount/hours amount)
:day (t.amount/days amount)
:week (t.amount/days (* amount 7))
:month (t.amount/months amount)
:quarter (t.amount/months (* amount 3))
:year (t.amount/years amount)))))

(s/defn add-datetime-units :- mbql.s/DateTimeValue
"Return a `relative-datetime` clause with `n` units added to it."
Expand All @@ -494,7 +492,6 @@
(let [[_ timestamp unit] absolute-or-relative-datetime]
[:absolute-datetime (relative-date unit n timestamp) unit])))


(defn dispatch-by-clause-name-or-class
"Dispatch function perfect for use with multimethods that dispatch off elements of an MBQL query. If `x` is an MBQL
clause, dispatches off the clause name; otherwise dispatches off `x`'s class."
Expand All @@ -503,7 +500,6 @@
(first x)
(class x)))


(s/defn expression-with-name :- mbql.s/FieldOrExpressionDef
"Return the `Expression` referenced by a given `expression-name`."
[{inner-query :query} :- mbql.s/Query, expression-name :- (s/cond-pre s/Keyword su/NonBlankString)]
Expand All @@ -524,7 +520,6 @@
:tried allowed-names
:found found}))))))))


(s/defn aggregation-at-index :- mbql.s/Aggregation
"Fetch the aggregation at index. This is intended to power aggregate field references (e.g. [:aggregation 0]).
This also handles nested queries, which could be potentially ambiguous if multiple levels had aggregations. To
Expand Down
60 changes: 20 additions & 40 deletions test/metabase/mbql/util_test.clj
View file
Original file line number Diff line number Diff line change
@@ -1,42 +1,22 @@
(ns metabase.mbql.util-test
(:require [clojure.test :refer :all]
[expectations :refer [expect]]
[java-time :as t]
[metabase.mbql.util :as mbql.u]))

;; make sure `relative-date` works as expected
(def ^:private a-timestamp (java.sql.Timestamp. (.getTime #inst "2019-06-14T00:00:00.000Z")))

(expect
#inst "2019-06-14T00:00:05.000000000-00:00"
(mbql.u/relative-date :second 5 a-timestamp))

(expect
#inst "2019-06-14T00:05:00.000000000-00:00"
(mbql.u/relative-date :minute 5 a-timestamp))

(expect
#inst "2019-06-14T05:00:00.000000000-00:00"
(mbql.u/relative-date :hour 5 a-timestamp))

(expect
#inst "2019-06-19T00:00:00.000000000-00:00"
(mbql.u/relative-date :day 5 a-timestamp))

(expect
#inst "2019-07-19T00:00:00.000000000-00:00"
(mbql.u/relative-date :week 5 a-timestamp))

(expect
#inst "2019-11-14T00:00:00.000000000-00:00"
(mbql.u/relative-date :month 5 a-timestamp))

(expect
#inst "2020-09-14T00:00:00.000000000-00:00"
(mbql.u/relative-date :quarter 5 a-timestamp))

(expect
#inst "2024-06-14T00:00:00.000000000-00:00"
(mbql.u/relative-date :year 5 a-timestamp))
(deftest relative-date-test
(let [t (t/zoned-date-time "2019-06-14T00:00:00.000Z[UTC]")]
(doseq [[unit n expected] [[:second 5 "2019-06-14T00:00:05Z[UTC]"]
[:minute 5 "2019-06-14T00:05:00Z[UTC]"]
[:hour 5 "2019-06-14T05:00:00Z[UTC]"]
[:day 5 "2019-06-19T00:00:00Z[UTC]"]
[:week 5 "2019-07-19T00:00:00Z[UTC]"]
[:month 5 "2019-11-14T00:00:00Z[UTC]"]
[:quarter 5 "2020-09-14T00:00:00Z[UTC]"]
[:year 5 "2024-06-14T00:00:00Z[UTC]"]]]
(is (= (t/zoned-date-time expected)
(mbql.u/relative-date unit n t))
(format "%s plus %d %ss should be %s" t n unit expected)))))


;;; +----------------------------------------------------------------------------------------------------------------+
Expand All @@ -45,12 +25,12 @@

;; can we use `match` to find the instances of a clause?
(expect
[[:field-id 10]
[:field-id 20]]
(mbql.u/match {:query {:filter [:=
[:field-id 10]
[:field-id 20]]}}
[:field-id & _]))
[[:field-id 10]
[:field-id 20]]
(mbql.u/match {:query {:filter [:=
[:field-id 10]
[:field-id 20]]}}
[:field-id & _]))

;; is `match` nice enought to automatically wrap raw keywords in appropriate patterns for us?
(expect
Expand Down

0 comments on commit a4ec087

Please sign in to comment.