MBQL: fix integer divisions (#11116)

Cast nominator to to float when doing integer division

modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj

@@ -281,6 +281,10 @@
     [_ _ expr]
     (vary-meta (hsql/call bigquery-fn expr) assoc :bigquery/temporal-type :timestamp)))
 
+(defmethod sql.qp/->float :bigquery
+  [_ value]
+  (hx/cast :float64 value))
+
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                                Query Processor                                                 |

modules/drivers/presto/src/metabase/driver/presto.clj

@@ -214,6 +214,10 @@
   [_ [_ t]]
   (hx/cast :time (u.date/format-sql (t/local-time t))))
 
+(defmethod sql.qp/->float :presto
+  [_ value]
+  (hx/cast :double value))
+
 ;; See https://prestodb.io/docs/current/functions/datetime.html
 
 ;; This is only needed for test purposes, because some of the sample data still uses legacy types

src/metabase/driver/mysql.clj

@@ -136,6 +136,13 @@
 (defn- date-format [format-str expr] (hsql/call :date_format expr (hx/literal format-str)))
 (defn- str-to-date [format-str expr] (hsql/call :str_to_date expr (hx/literal format-str)))
 
+
+(defmethod sql.qp/->float :mysql
+  [_ value]
+  ;; no-op as MySQL doesn't support cast to float
+  value)
+
+
 ;; Since MySQL doesn't have date_trunc() we fake it by formatting a date to an appropriate string and then converting
 ;; back to a date. See http://dev.mysql.com/doc/refman/5.6/en/date-and-time-functions.html#function_date-format for an
 ;; explanation of format specifiers

src/metabase/driver/sql/query_processor.clj

@@ -280,16 +280,29 @@
 ;;
 ;; also, we want to gracefully handle situations where the column is ZERO and just swap it out with NULL instead, so
 ;; we don't get divide by zero errors. SQL DBs always return NULL when dividing by NULL (AFAIK)
+
+(defmulti ->float
+  "Cast to float"
+  {:arglists '([driver value])}
+  driver/dispatch-on-initialized-driver
+  :hierarchy #'driver/hierarchy)
+
+(defmethod ->float :sql
+  [_ value]
+  (hx/cast :float value))
+
 (defmethod ->honeysql [:sql :/]
   [driver [_ & args]]
-  (let [args (for [arg args]
-               (->honeysql driver (if (integer? arg)
-                                    (double arg)
-                                    arg)))]
-    (apply hsql/call :/ (first args) (for [arg (rest args)]
-                                       (hsql/call :case
-                                         (hsql/call := arg 0) nil
-                                         :else                arg)))))
+  (let [[numerator & denominators] (for [arg args]
+                                     (->honeysql driver (if (integer? arg)
+                                                          (double arg)
+                                                          arg)))]
+    (apply hsql/call :/
+           (->float driver numerator)
+           (for [denominator denominators]
+             (hsql/call :case
+               (hsql/call := denominator 0) nil
+               :else                        denominator)))))
 
 (defmethod ->honeysql [:sql :sum-where]
   [driver [_ arg pred]]

test/metabase/api/automagic_dashboards_test.clj

@@ -171,9 +171,9 @@
 ;;; ------------------- Transforms -------------------
 
 (expect
-  [[4 1 10.0646 -165.374 "Red Medicine" 3 1 4 3 2 1]
-   [11 2 34.0996 -118.329 "Stout Burgers & Beers" 2 2 11 2 1 1]
-   [11 3 34.0406 -118.428 "The Apple Pan" 2 2 11 2 1 1]]
+  [[4 1 10.0646 -165.374 "Red Medicine" 3 1.5 4 3 2 1]
+   [11 2 34.0996 -118.329 "Stout Burgers & Beers" 2 2.0 11 2 1 1]
+   [11 3 34.0406 -118.428 "The Apple Pan" 2 2.0 11 2 1 1]]
   (test-users/with-test-user :rasta
     (transforms.test/with-test-transform-specs
       (de.test/with-test-domain-entity-specs

test/metabase/api/transform_test.clj

@@ -23,9 +23,9 @@
 
 ;; Run the transform and make sure it produces the correct result
 (expect
-  [[4 1 10.0646 -165.374 "Red Medicine" 3 1 4 3 2 1]
-   [11 2 34.0996 -118.329 "Stout Burgers & Beers" 2 2 11 2 1 1]
-   [11 3 34.0406 -118.428 "The Apple Pan" 2 2 11 2 1 1]]
+  [[4 1 10.0646 -165.374 "Red Medicine" 3 1.5 4 3 2 1]
+   [11 2 34.0996 -118.329 "Stout Burgers & Beers" 2 2.0 11 2 1 1]
+   [11 3 34.0406 -118.428 "The Apple Pan" 2 2.0 11 2 1 1]]
   (test-users/with-test-user :rasta
     (with-test-transform-specs
       (with-test-domain-entity-specs

test/metabase/query_processor_test/expressions_test.clj

@@ -39,6 +39,20 @@
                   :order-by    [[:asc $id]]}))))
         "Make sure FLOATING POINT division is done")))
 
+;; Make sure FLOATING POINT division is done when dividing by expressions/fields
+(datasets/expect-with-drivers (qp.test/non-timeseries-drivers-with-feature :expressions)
+  [[0.6]
+   [0.5]
+   [0.5]]
+  (qp.test/format-rows-by [1.0]
+    (qp.test/rows
+      (data/run-mbql-query venues
+        {:expressions {:big-price         [:+ $price 2]
+                       :my-cool-new-field [:/ $price [:expression "big-price"]]}
+         :fields      [[:expression "my-cool-new-field"]]
+         :limit       3
+         :order-by    [[:asc $id]]}))))
+
 ;; Can we do NESTED EXPRESSIONS ?
 (datasets/expect-with-drivers (qp.test/normal-drivers-with-feature :expressions)
   [[1 "Red Medicine"           4 10.0646 -165.374 3 3.0]

test/metabase/query_processor_test/share_test.clj

 (ns metabase.query-processor-test.share-test
-  (:require [metabase.models
+  (:require [metabase
+             [query-processor-test :refer :all]
+             [util :as u]]
+            [metabase.models
              [metric :refer [Metric]]
              [segment :refer [Segment]]]
-            [metabase.query-processor-test :refer :all]
             [metabase.test
              [data :as data]
              [util :as tu]]
@@ -64,7 +66,8 @@
        (data/run-mbql-query venues)
        rows
        ffirst
-       double))
+       double
+       (u/round-to-decimals 2)))
 
 (datasets/expect-with-drivers (non-timeseries-drivers-with-feature :basic-aggregations)
   0.94

test/metabase/transforms/core_test.clj

@@ -135,9 +135,9 @@
 
 ;; Run the transform and make sure it produces the correct result
 (expect
-  [[4 1 10.0646 -165.374 "Red Medicine" 3 1 4 3 2 1]
-   [11 2 34.0996 -118.329 "Stout Burgers & Beers" 2 2 11 2 1 1]
-   [11 3 34.0406 -118.428 "The Apple Pan" 2 2 11 2 1 1]]
+  [[4 1 10.0646 -165.374 "Red Medicine" 3 1.5 4 3 2 1]
+   [11 2 34.0996 -118.329 "Stout Burgers & Beers" 2 2.0 11 2 1 1]
+   [11 3 34.0406 -118.428 "The Apple Pan" 2 2.0 11 2 1 1]]
   (test-users/with-test-user :rasta
     (with-test-domain-entity-specs
       (tu/with-model-cleanup [Card Collection]