Handle dimensions in SQL params

.dir-locals.el

@@ -41,4 +41,5 @@
                               (resolve-private-fns 1)
                               (select 1)
                               (sync-in-context 2)
-                              (when-testing-engine 1)))))))
+                              (when-testing-engine 1)
+                              (with-redefs-fn 1)))))))

src/metabase/query_processor/parameters.clj

@@ -100,6 +100,13 @@
          ;; the above values are JodaTime objects, so unparse them to iso8601 strings
          (m/map-vals (partial tf/unparse formatter)))))
 
+(defn date->range
+  "Convert a relative or absolute date range VALUE to a map with `:start` and `:end` keys."
+  [value report-timezone]
+  (if (contains? relative-dates value)
+    (relative-date->range value report-timezone)
+    (absolute-date->range value)))
+
 
 ;;; +-------------------------------------------------------------------------------------------------------+
 ;;; |                                             MBQL QUERIES                                              |

src/metabase/query_processor/sql_parameters.clj

@@ -16,19 +16,41 @@
 (defprotocol ^:private ISQLParamSubstituion
   (^:private ->sql ^String [this]))
 
+(defrecord ^:private Dimension [^FieldInstance field, param])
+
+(defrecord ^:private DateRange [start end])
+
+(defrecord ^:private NumberValue [value])
+
+(defn- dimension-value->sql [dimension-type value]
+  (if (contains? #{"date/range" "date/month-year" "date/quarter-year"} dimension-type)
+    (->sql (map->DateRange ((resolve 'metabase.query-processor.parameters/date->range) value nil))) ; TODO - get timezone from query dict
+    (str "= " (->sql value))))
+
 (extend-protocol ISQLParamSubstituion
   nil           (->sql [_] "NULL")
   Object        (->sql [this]
                   (str this))
   Boolean       (->sql [this]
                   (if this "TRUE" "FALSE"))
+  NumberValue   (->sql [this]
+                  (:value this))
   String        (->sql [this]
                   (str \' (s/replace this #"'" "\\\\'") \'))
   FieldInstance (->sql [this]
                   ;; For SQL drivers, generate appropriate qualified & quoted identifier
                   ;; Mative param substitution is only enabled for SQL for the time being. We'll need to tweak this a bit so when we add support for other DBs in the future.
                   (first (hsql/format (apply hsql/qualify (field/qualified-name-components this))
-                           :quoting ((resolve 'metabase.driver.generic-sql/quote-style) *driver*)))))
+                           :quoting ((resolve 'metabase.driver.generic-sql/quote-style) *driver*))))
+  DateRange     (->sql [{:keys [start end]}]
+                  (if (= start end)
+                    (format "= '%s'" start)
+                    (format "BETWEEN '%s' AND '%s'" start end)))
+  Dimension     (->sql [{:keys [field param]}]
+                  (if-not param
+                    ;; if the param is `nil` just put in something that will always be true, such as `1` (e.g. `WHERE 1`)
+                    "1"
+                    (format "%s %s" (->sql field) (dimension-value->sql (:type param) (:value param))))))
 
 (defn- replace-param [s params match param]
   (let [k (keyword param)
@@ -65,21 +87,25 @@
 
 ;;; ------------------------------------------------------------ Param Resolution ------------------------------------------------------------
 
+(defn param-with-target [params target]
+  (some (fn [param]
+          (when (= (:target param) target)
+            param))
+        params))
+
 (defn- param-value-for-tag [tag params]
   (when (not= (:type tag) "dimension")
-    (some (fn [param]
-            (when (= (:target param) ["variable" ["template-tag" (:name tag)]])
-              (:value param)))
-          params)))
+    (:value (param-with-target params ["variable" ["template-tag" (:name tag)]]))))
 
 (defn- dimension->field-id [dimension]
   (:field-id (ql/expand-ql-sexpr dimension)))
 
-(defn- dimension-value-for-tag [tag]
+(defn- dimension-value-for-tag [tag params]
   (when-let [dimension (:dimension tag)]
-    (if-let [field-id (dimension->field-id dimension)]
-      (db/select-one [Field :name :parent_id :table_id], :id field-id)
-      (throw (Exception. (str "Don't know how to handle dimension: " dimension))))))
+    (let [field-id (or (dimension->field-id dimension)
+                       (throw (Exception. (str "Don't know how to handle dimension: " dimension))))]
+      (map->Dimension {:field (db/select-one [Field :name :parent_id :table_id], :id field-id)
+                       :param (param-with-target params ["dimension" ["template-tag" (:name tag)]])}))))
 
 (defn- default-value-for-tag [{:keys [default display_name required]}]
   (or default
@@ -87,17 +113,15 @@
         (throw (Exception. (format "'%s' is a required param." display_name))))))
 
 (defn- parse-value-for-type [type value]
-  value
-  ;; TODO - do we ever need to do anything special for one of the types?
-  #_(case (keyword type)
-    :number    value
-    :text      value
-    :date      value
-    :dimension value))
+  (cond
+    (= type "number")                                (->NumberValue value)
+    (and (= type "dimension")
+         (= (get-in value [:param :type]) "number")) (update-in value [:param :value] ->NumberValue)
+    :else                                            value))
 
 (defn- value-for-tag [tag params]
   (parse-value-for-type (:type tag) (or (param-value-for-tag tag params)
-                                        (dimension-value-for-tag tag)
+                                        (dimension-value-for-tag tag params)
                                         (default-value-for-tag tag))))
 
 (defn- query->params-map [{tags :template_tags, params :parameters}]

test/metabase/query_processor/sql_parameters_test.clj

 (ns metabase.query-processor.sql-parameters-test
-  (:require [expectations :refer :all]
+  (:require [clj-time.core :as t]
+            [expectations :refer :all]
             [metabase.driver :as driver]
             [metabase.query-processor.sql-parameters :refer :all]
             [metabase.test.data :as data]))
 
 
-;;; simple substitution -- {{x}}
+;;; ------------------------------------------------------------ simple substitution -- {{x}} ------------------------------------------------------------
 
 (expect "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE"
   (substitute "SELECT * FROM bird_facts WHERE toucans_are_cool = {{toucans_are_cool}}"
@@ -24,7 +25,7 @@
     {:toucans_are_cool true}))
 
 
-;;; optional substitution -- [[ ... {{x}} ... ]]
+;;; ------------------------------------------------------------ optional substitution -- [[ ... {{x}} ... ]] ------------------------------------------------------------
 
 (expect
   "SELECT * FROM bird_facts WHERE toucans_are_cool = TRUE"
@@ -132,7 +133,7 @@
     {:num_toucans 5}))
 
 
-;;; ------------------------------------------------------------ end-to-end tests ------------------------------------------------------------
+;;; ------------------------------------------------------------ expansion tests: variables ------------------------------------------------------------
 
 ;; unspecified optional param
 (expect
@@ -151,7 +152,7 @@
 
 ;; default value
 (expect
-  "SELECT * FROM orders WHERE id = '100';"
+  "SELECT * FROM orders WHERE id = 100;"
   (-> (expand-params {:native        {:query "SELECT * FROM orders WHERE id = {{id}};"}
                       :template_tags {:id {:name "id", :display_name "ID", :type "number", :required true, :default "100"}}
                       :parameters    []})
@@ -159,13 +160,13 @@
 
 ;; specified param (numbers)
 (expect
-  "SELECT * FROM orders WHERE id = '2';"
+  "SELECT * FROM orders WHERE id = 2;"
   (-> (expand-params {:native        {:query "SELECT * FROM orders WHERE id = {{id}};"}
                       :template_tags {:id {:name "id", :display_name "ID", :type "number", :required true, :default "100"}}
                       :parameters    [{:type "category", :target ["variable" ["template-tag" "id"]], :value "2"}]})
       :native :query))
 
-;; specified param (date)
+;; specified param (date/single)
 (expect
   "SELECT * FROM orders WHERE created_at > '2016-07-19';"
   (-> (expand-params {:native        {:query "SELECT * FROM orders WHERE created_at > {{created_at}};"}
@@ -181,11 +182,98 @@
                       :parameters    [{:type "category", :target ["variable" ["template-tag" "category"]], :value "Gizmo"}],})
       :native :query))
 
-;; dimension
+
+;;; ------------------------------------------------------------ expansion tests: dimensions ------------------------------------------------------------
+
+#_(:require 'metabase.test.data.h2 :reload)
+
+(defn- expand-with-dimension-param [dimension-param]
+  (with-redefs [t/now (fn [] (t/date-time 2016 06 07 12 0 0))]
+    (-> (expand-params {:driver        (driver/engine->driver :h2)
+                        :native        {:query "SELECT * FROM checkins WHERE {{date}};"}
+                        :template_tags {:date {:name "date", :display_name "Checkin Date", :type "dimension", :dimension ["field-id" (data/id :checkins :date)]}}
+                        :parameters    [(when dimension-param
+                                          (merge {:target ["dimension" ["template-tag" "date"]]}
+                                                 dimension-param))]})
+        :native :query)))
+
+;; TODO dimension (date/single)
 (expect
-  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" > '2016-01-01';"
-  (-> (expand-params {:driver        (driver/engine->driver :h2)
-                      :native        {:query "SELECT * FROM checkins WHERE {{date}} > '2016-01-01';"},
-                      :template_tags {:date {:name "date", :display_name "Checkin Date", :type "dimension", :dimension ["field-id" (data/id :checkins :date)]}},
-                      :parameters    []})
-      :native :query))
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" = '2016-07-01';"
+  (expand-with-dimension-param {:type "date/single", :value "2016-07-01"}))
+
+;; dimension (date/range)
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-07-01' AND '2016-08-01';"
+  (expand-with-dimension-param {:type "date/range", :value "2016-07-01~2016-08-01"}))
+
+
+;; dimension (date/month-year)
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-07-01' AND '2016-07-31';"
+  (expand-with-dimension-param {:type "date/month-year", :value "2016-07"}))
+
+;; dimension (date/quarter-year)
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-01-01' AND '2016-03-31';"
+  (expand-with-dimension-param {:type "date/quarter-year", :value "Q1-2016"}))
+
+;; relative date -- "yesterday"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" = '2016-06-06';"
+  (expand-with-dimension-param {:type "date/range", :value "yesterday"}))
+
+;; relative date -- "past7days"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-05-31' AND '2016-06-06';"
+  (expand-with-dimension-param {:type "date/range", :value "past7days"}))
+
+;; relative date -- "past30days"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-05-08' AND '2016-06-06';"
+  (expand-with-dimension-param {:type "date/range", :value "past30days"}))
+
+;; relative date -- "thisweek"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-06-05' AND '2016-06-11';"
+  (expand-with-dimension-param {:type "date/range", :value "thisweek"}))
+
+;; relative date -- "thismonth"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-06-01' AND '2016-06-30';"
+  (expand-with-dimension-param {:type "date/range", :value "thismonth"}))
+
+;; relative date -- "thisyear"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-01-01' AND '2016-12-31';"
+  (expand-with-dimension-param {:type "date/range", :value "thisyear"}))
+
+;; relative date -- "lastweek"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-05-29' AND '2016-06-04';"
+  (expand-with-dimension-param {:type "date/range", :value "lastweek"}))
+
+;; relative date -- "lastmonth"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2016-05-01' AND '2016-05-31';"
+  (expand-with-dimension-param {:type "date/range", :value "lastmonth"}))
+
+;; relative date -- "lastyear"
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" BETWEEN '2015-01-01' AND '2015-12-31';"
+  (expand-with-dimension-param {:type "date/range", :value "lastyear"}))
+
+;; dimension with no value -- just replace with an always true clause (e.g. "WHERE 1")
+(expect
+  "SELECT * FROM checkins WHERE 1;"
+  (expand-with-dimension-param nil))
+
+;; dimension -- number
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" = 100;"
+  (expand-with-dimension-param {:type "number", :value "100"}))
+
+;; dimension -- text
+(expect
+  "SELECT * FROM checkins WHERE \"PUBLIC\".\"CHECKINS\".\"DATE\" = '100';"
+  (expand-with-dimension-param {:type "text", :value "100"}))