Merge pull request #5940 from metabase/fix-druid-date-binning-master

Date binning should be shown on all DBs

src/metabase/api/table.clj

@@ -161,7 +161,10 @@
 (def ^:private coordinate-default-index
   (dimension-index-for-type "type/Coordinate" #(.contains ^String (:name %) "Auto bin")))
 
-(defn- assoc-field-dimension-options [{:keys [base_type special_type fingerprint] :as field}]
+(defn- supports-numeric-binning? [driver]
+  (and driver (contains? (driver/features driver) :binning)))
+
+(defn- assoc-field-dimension-options [driver {:keys [base_type special_type fingerprint] :as field}]
   (let [{min_value :min, max_value :max} (get-in fingerprint [:type :type/Number])
         [default-option all-options] (cond
 
@@ -170,12 +173,14 @@
                                        [date-default-index datetime-dimension-indexes]
 
                                        (and min_value max_value
-                                            (isa? special_type :type/Coordinate))
+                                            (isa? special_type :type/Coordinate)
+                                            (supports-numeric-binning? driver))
                                        [coordinate-default-index coordinate-dimension-indexes]
 
                                        (and min_value max_value
                                             (isa? base_type :type/Number)
-                                            (or (nil? special_type) (isa? special_type :type/Number)))
+                                            (or (nil? special_type) (isa? special_type :type/Number))
+                                            (supports-numeric-binning? driver))
                                        [numeric-default-index numeric-dimension-indexes]
 
                                        :else
@@ -185,16 +190,10 @@
       :dimension_options all-options)))
 
 (defn- assoc-dimension-options [resp driver]
-  (if (and driver (contains? (driver/features driver) :binning))
-    (-> resp
-        (assoc :dimension_options dimension-options-for-response)
-        (update :fields #(mapv assoc-field-dimension-options %)))
-    (-> resp
-        (assoc :dimension_options [])
-        (update :fields (fn [fields]
-                          (mapv #(assoc %
-                                   :dimension_options []
-                                   :default_dimension_option nil) fields))))))
+  (-> resp
+      (assoc :dimension_options dimension-options-for-response)
+      (update :fields (fn [fields]
+                        (mapv #(assoc-field-dimension-options driver %) fields)))))
 
 (defn- format-fields-for-response [resp]
   (update resp :fields

test/metabase/api/table_test.clj

@@ -22,11 +22,15 @@
              [util :as tu :refer [match-$]]]
             [metabase.test.data
              [dataset-definitions :as defs]
+             [datasets :as datasets]
              [users :refer [user->client]]]
             [toucan
              [db :as db]
              [hydrate :as hydrate]]
-            [toucan.util.test :as tt]))
+            [toucan.util.test :as tt]
+            [metabase.query-processor-test :as qpt]
+            [metabase.timeseries-query-processor-test :as timeseries-qp-test]
+            [metabase.test.data.datasets :as datasets :refer [*driver* *engine*]]))
 
 ;; ## /api/org/* AUTHENTICATION Tests
 ;; We assume that all endpoints for a given context are enforced by the same middleware, so we don't run the same
@@ -602,3 +606,15 @@
   (data/dataset sad-toucan-incidents
     (let [response ((user->client :rasta) :get 200 (format "table/%d/query_metadata" (data/id :incidents)))]
       (dimension-options-for-field response "TIMESTAMP"))))
+
+;; Datetime binning options should showup whether the backend supports binning of numeric values or not
+(datasets/expect-with-engines #{:druid}
+  (var-get #'table-api/datetime-dimension-indexes)
+  (timeseries-qp-test/with-flattened-dbdef
+    (let [response ((user->client :rasta) :get 200 (format "table/%d/query_metadata" (data/id :checkins)))]
+      (dimension-options-for-field response "timestamp"))))
+
+(qpt/expect-with-non-timeseries-dbs
+ (var-get #'table-api/datetime-dimension-indexes)
+ (let [response ((user->client :rasta) :get 200 (format "table/%d/query_metadata" (data/id :checkins)))]
+   (dimension-options-for-field response "DATE")))