Merge pull request #332 from metabase/fix_date_filtering

Fix date filtering

src/metabase/api/meta/dataset.clj

@@ -11,7 +11,9 @@
   "Execute an MQL query and retrieve the results as JSON."
   [:as {{:keys [database] :as body} :body}]
   (read-check Database database)
-  (driver/dataset-query body {:executed_by *current-user-id*}))
+  (let [{:keys [status] :as response} (driver/dataset-query body {:executed_by *current-user-id*})]
+    {:status (if (= status :completed) 200 500)
+     :body response}))
 
 (defendpoint GET "/csv"
   "Execute an MQL query and download the result data as a CSV file."

src/metabase/driver/generic_sql/query_processor.clj

@@ -151,15 +151,18 @@
                                                                                 {lon-kw ['> lon-min]}]))
     [_ field-id & _] {(field-id->kw field-id)
                       (match subclause
-                        [">"  _ value]        ['>    value]
-                        ["<"  _ value]        ['<    value]
-                        [">=" _ value]        ['>=   value]
-                        ["<=" _ value]        ['<=   value]
-                        ["="  _ value]        ['=    value]
-                        ["!=" _ value]        ['not= value]
                         ["NOT_NULL" _]        ['not= nil]
                         ["IS_NULL" _]         ['=    nil]
-                        ["BETWEEN" _ min max] ['between [min max]])}))
+                        ["BETWEEN" _ min max] ['between [min max]]
+                        [_ _ value]           (let [value (if (date-field-id? field-id) `(raw ~(format "CAST('%s' AS DATE)" value)) ; cast YYYY-MM-DD string from UI
+                                                              value)]                                                               ; to SQL date if applicable
+                                                (match subclause
+                                                  [">"  _ _] ['>    value]
+                                                  ["<"  _ _] ['<    value]
+                                                  [">=" _ _] ['>=   value]
+                                                  ["<=" _ _] ['<=   value]
+                                                  ["="  _ _] ['=    value]
+                                                  ["!=" _ _] ['not= value])))}))
 
 (defmethod apply-form :filter [[_ filter-clause]]
   (match filter-clause

src/metabase/driver/generic_sql/util.clj

@@ -77,11 +77,18 @@
   (if (contains? #{:DateField :DateTimeField} field-base-type) `(korma/raw ~(format "CAST(\"%s\" AS DATE)" field-name))
       (keyword field-name)))
 
-;; TODO - should we memoize this?
-(defn field-id->kw
+(def field-id->kw
   "Given a metabase `Field` ID, return a keyword for use in the Korma form (or a casted raw string for date fields)."
-  [field-id]
-  {:pre [(integer? field-id)]}
-  (if-let [{field-name :name, field-type :base_type} (sel :one [Field :name :base_type] :id field-id)]
-    (castify-field field-name field-type)
-    (throw (Exception. (format "Field with ID %d doesn't exist!" field-id)))))
+  (memoize                         ; This can be memozied since the names and base_types of Fields never change
+   (fn [field-id]                   ; *  if a field is renamed the old field will just be marked as `inactive` and a new Field will be created
+     {:pre [(integer? field-id)]}  ; *  if a field's type *actually* changes we have no logic in driver.generic-sql.sync to handle that case any way (TODO - fix issue?)
+     (if-let [{field-name :name, field-type :base_type} (sel :one [Field :name :base_type] :id field-id)]
+       (castify-field field-name field-type)
+       (throw (Exception. (format "Field with ID %d doesn't exist!" field-id)))))))
+
+(def date-field-id?
+  "Does FIELD-ID correspond to a field that is a Date?"
+  (memoize        ; memoize since the base_type of a Field isn't going to change
+   (fn [field-id]
+     (contains? #{:DateField :DateTimeField}
+                (sel :one :field [Field :base_type] :id field-id)))))