diff --git a/project.clj b/project.clj
index 9027b350a1040de533a172ec90ae07b832451a1d..39d788309d2ebc445469620d2b6573f71e0aa8e2 100644
--- a/project.clj
+++ b/project.clj
@@ -48,7 +48,7 @@
[org.clojure/math.combinatorics "0.1.4"] ; combinatorics functions
[org.clojure/math.numeric-tower "0.0.4"] ; math functions like `ceil`
[org.clojure/tools.logging "1.1.0"] ; logging framework
- [org.clojure/tools.namespace "0.2.11"]
+ [org.clojure/tools.namespace "1.0.0"]
[org.clojure/tools.trace "0.7.10"] ; function tracing
[amalloy/ring-buffer "1.2.2"
:exclusions [org.clojure/clojure
@@ -369,8 +369,8 @@
:cloverage
[:test-common
- {:dependencies [[cloverage "1.2.0" :exclusions [riddley]]]
- :plugins [[lein-cloverage "1.2.0"]]
+ {:dependencies [[camsaul/cloverage "1.2.1.1" :exclusions [riddley]]]
+ :plugins [[camsaul/lein-cloverage "1.2.1.1"]]
:source-paths ^:replace ["src" "backend/mbql/src"]
:test-paths ^:replace ["test" "backend/mbql/test"]
:cloverage {:fail-threshold 69
diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index ae4fd4d1dcd920c60bceecba5aa200d60b5fb73f..58e5ece6a5c29b500ac7de5f714dd42aa1a36826 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -611,8 +611,9 @@
(qp/process-query-and-save-execution! query info context)))}}]
{:pre [(u/maybe? sequential? parameters)]}
(let [card (api/read-check (Card card-id))
- query (assoc (query-for-card card parameters constraints middleware)
- :async? true)
+ query (-> (assoc (query-for-card card parameters constraints middleware)
+ :async? true)
+ (assoc-in [:middleware :js-int-to-string?] true))
info {:executed-by api/*current-user-id*
:context context
:card-id card-id
diff --git a/src/metabase/api/dataset.clj b/src/metabase/api/dataset.clj
index fcce07f5322dcd8b7154fee107c8fb624d895631..67bd74d11796ef8115cad075982b88ed088eccb2 100644
--- a/src/metabase/api/dataset.clj
+++ b/src/metabase/api/dataset.clj
@@ -49,7 +49,8 @@
info {:executed-by api/*current-user-id*
:context :ad-hoc
:card-id source-card-id
- :nested? (boolean source-card-id)}]
+ :nested? (boolean source-card-id)}
+ query (update query :middleware assoc :js-int-to-string? true)]
(qp.streaming/streaming-response [context :api]
(qp/process-query-and-save-with-max-results-constraints! query info context))))
diff --git a/src/metabase/query_processor.clj b/src/metabase/query_processor.clj
index ce46e5afe24a80af341b980c9118bc444df2c37a..12204ff12eb3d4103ea18cbb6cea73fc5733dfbb 100644
--- a/src/metabase/query_processor.clj
+++ b/src/metabase/query_processor.clj
@@ -29,6 +29,7 @@
[expand-macros :as expand-macros]
[fetch-source-query :as fetch-source-query]
[format-rows :as format-rows]
+ [large-int-id :as large-int-id]
[limit :as limit]
[mbql-to-native :as mbql-to-native]
[normalize-query :as normalize]
@@ -68,6 +69,7 @@
#'cumulative-ags/handle-cumulative-aggregations
#'resolve-joins/resolve-joins
#'add-implicit-joins/add-implicit-joins
+ #'large-int-id/convert-id-to-string
#'limit/limit
#'format-rows/format-rows
#'desugar/desugar
diff --git a/src/metabase/query_processor/middleware/large_int_id.clj b/src/metabase/query_processor/middleware/large_int_id.clj
new file mode 100644
index 0000000000000000000000000000000000000000..116cf11352e0bdd3f99ff66f0a22e10f766d63e1
--- /dev/null
+++ b/src/metabase/query_processor/middleware/large_int_id.clj
@@ -0,0 +1,48 @@
+(ns metabase.query-processor.middleware.large-int-id
+ "Middleware for handling conversion of IDs to strings for proper display of large numbers"
+ (:require [clojure.tools.logging :as log]
+ [metabase.mbql.util :as mbql.u]
+ [metabase.models.field :refer [Field]]))
+
+(defn- result-int->string
+ [field-indexes rf]
+ (fn
+ ([] (rf))
+ ([result] (rf result))
+ ([result row]
+ (rf result (reduce #(update-in %1 [%2] str) row field-indexes)))))
+
+(defn convert-id-to-string
+ "Converts any ID (:type/PK and :type/FK) in a result to a string to handle a number > 2^51
+ or < -2^51, the JavaScript float mantissa. This will allow proper display of large numbers,
+ like IDs from services like social media. All ID numbers are converted to avoid the performance
+ penalty of a comparison based on size."
+ [qp]
+ (fn [{{:keys [js-int-to-string?] :or {js-int-to-string? false}} :middleware, :as query} rff context]
+ ;; currently, this excludes `:field-literal` values like aggregations.
+ ;;
+ ;; for a query like below, *no* conversion will occur
+ ;; (mt/mbql-query venues
+ ;; {:source-query {:source-table $$venues
+ ;; :aggregation [[:aggregation-options
+ ;; [:avg $id]
+ ;; {:name "some_generated_name", :display-name "My Cool Ag"}]]
+ ;; :breakout [$price]}})
+ ;; when you run in this fashion, you lose the ability to determine if it's an ID - you get a `:fields` value
+ ;; like: `:fields [[:field-literal "PRICE" :type/Integer] [:field-literal "some_generated_name" :type/BigInteger]]`
+ ;; so, short of turning all `:type/Integer` derived values into strings, this is the best approximation
+ ;; of a fix that can be accomplished.
+ (let [field-indexes (keep-indexed
+ (fn [idx val]
+ (let [field-id (mbql.u/field-clause->id-or-literal val)]
+ (when-let [field (when (number? field-id)
+ (Field field-id))]
+ (when (and (or (isa? (:special_type field) :type/PK)
+ (isa? (:special_type field) :type/FK))
+ (isa? (:base_type field) :type/Integer))
+ idx))))
+ (log/spy :error (:fields (:query query))))]
+ (qp query (if (and js-int-to-string? (seq field-indexes))
+ #(result-int->string field-indexes (rff %))
+ rff)
+ context))))
diff --git a/test/metabase/api/dataset_test.clj b/test/metabase/api/dataset_test.clj
index 881c5fd7a96259848171e5261330d23a231e79f6..651fe21945e2b533dacd9a845bc3e52ac8e91d38 100644
--- a/test/metabase/api/dataset_test.clj
+++ b/test/metabase/api/dataset_test.clj
@@ -51,7 +51,8 @@
(defn- most-recent-query-execution [] (db/select-one QueryExecution {:order-by [[:id :desc]]}))
(def ^:private query-defaults
- {:middleware {:add-default-userland-constraints? true}})
+ {:middleware {:add-default-userland-constraints? true
+ :js-int-to-string? true}})
(deftest basic-query-test
(testing "POST /api/dataset"
diff --git a/test/metabase/query_processor/middleware/large_int_id_test.clj b/test/metabase/query_processor/middleware/large_int_id_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..671c6d80e05f1f333fd7a3868bbef295a010ccee
--- /dev/null
+++ b/test/metabase/query_processor/middleware/large_int_id_test.clj
@@ -0,0 +1,97 @@
+(ns metabase.query-processor.middleware.large-int-id-test
+ (:require [clojure.test :refer :all]
+ [metabase
+ [query-processor :as qp]
+ [test :as mt]]))
+
+(deftest convert-ids
+ (let [query (mt/mbql-query users
+ {:order-by [[:asc $id]]
+ :limit 5})]
+ (testing "PKs become strings when middleware enabled"
+ (is (= [["1" "Plato Yeshua" "2014-04-01T08:30:00Z"]
+ ["2" "Felipinho Asklepios" "2014-12-05T15:15:00Z"]
+ ["3" "Kaneonuskatew Eiran" "2014-11-06T16:15:00Z"]
+ ["4" "Simcha Yan" "2014-01-01T08:30:00Z"]
+ ["5" "Quentin Sören" "2014-10-03T17:30:00Z"]]
+ (mt/rows
+ (qp/process-query (assoc query :middleware {:js-int-to-string? true}))))))
+
+ (testing "PKs are left alone when middleware disabled (default)"
+ (is (= [[1 "Plato Yeshua" "2014-04-01T08:30:00Z"]
+ [2 "Felipinho Asklepios" "2014-12-05T15:15:00Z"]
+ [3 "Kaneonuskatew Eiran" "2014-11-06T16:15:00Z"]
+ [4 "Simcha Yan" "2014-01-01T08:30:00Z"]
+ [5 "Quentin Sören" "2014-10-03T17:30:00Z"]]
+ (mt/rows
+ (qp/process-query (assoc query :middleware {})))))))
+
+ (let [query (mt/mbql-query users
+ {:fields [$name]
+ :order-by [[:asc $name]]
+ :limit 5})]
+ (testing "handle when there are no ID columns in the query but the middleware is enabled"
+ (is (= [["Broen Olujimi"]
+ ["Conchúr Tihomir"]
+ ["Dwight Gresham"]
+ ["Felipinho Asklepios"]
+ ["Frans Hevel"]]
+ (mt/rows
+ (qp/process-query (assoc query :middleware {:js-int-to-string? true})))))))
+
+ (let [query (mt/mbql-query venues
+ {:order-by [[:asc $id]]
+ :limit 5})]
+ (testing "FKs become strings when middleware enabled"
+ (is (= [["1" "Red Medicine" "4" 10.0646 -165.374 3]
+ ["2" "Stout Burgers & Beers" "11" 34.0996 -118.329 2]
+ ["3" "The Apple Pan" "11" 34.0406 -118.428 2]
+ ["4" "Wurstküche" "29" 33.9997 -118.465 2]
+ ["5" "Brite Spot Family Restaurant" "20" 34.0778 -118.261 2]]
+ (mt/rows
+ (qp/process-query (assoc query :middleware {:js-int-to-string? true}))))))
+
+ (testing "FKs are left alone when middleware disabled (default)"
+ (is (= [[1 "Red Medicine" 4 10.0646 -165.374 3]
+ [2 "Stout Burgers & Beers" 11 34.0996 -118.329 2]
+ [3 "The Apple Pan" 11 34.0406 -118.428 2]
+ [4 "Wurstküche" 29 33.9997 -118.465 2]
+ [5 "Brite Spot Family Restaurant" 20 34.0778 -118.261 2]]
+ (mt/rows
+ (qp/process-query (assoc query :middleware {})))))))
+
+ (let [query (mt/mbql-query checkins
+ {:fields [$id $user_id->users.id $user_id->users.name $venue_id->venues.id $venue_id->venues.name]
+ :order-by [[:asc $id]]
+ :limit 5})]
+ (testing "joins work correctly"
+ (is (= [["1" "5" "Quentin Sören" "12" "The Misfit Restaurant + Bar"]
+ ["2" "1" "Plato Yeshua" "31" "Bludso's BBQ"]
+ ["3" "8" "Szymon Theutrich" "56" "Philippe the Original"]
+ ["4" "5" "Quentin Sören" "4" "Wurstküche"]
+ ["5" "3" "Kaneonuskatew Eiran" "49" "Hotel Biron"]]
+ (mt/rows
+ (qp/process-query (assoc query :middleware {:js-int-to-string? true})))))))
+
+ (let [query (mt/mbql-query venues
+ {:source-query {:source-table $$venues
+ :aggregation [[:aggregation-options
+ [:avg $id]
+ {:name "some_generated_name", :display-name "My Cool Ag"}]]
+ :breakout [$price]}})]
+ ;; see comment in `metabase.query-processor.middleware.large-int-id/convert-id-to-string`
+ ;; for why this value does not change
+ (testing "aggregations are not converted to strings with middleware enabled"
+ (is (= [[1 55]
+ [2 48]
+ [3 47]
+ [4 62]]
+ (mt/rows
+ (qp/process-query (assoc query :middleware {:js-int-to-string? true}))))) )
+ (testing "aggregation does not convert to strings with middleware disabled (default)"
+ (is (= [[1 55]
+ [2 48]
+ [3 47]
+ [4 62]]
+ (mt/rows
+ (qp/process-query (assoc query :middleware {}))))))))