From b6158cf2a2a872b94e1c8e76e7db5c5872008ed7 Mon Sep 17 00:00:00 2001
From: Howon Lee <hlee.howon@gmail.com>
Date: Fri, 8 Jul 2022 22:56:38 -0700
Subject: [PATCH] 22732 rework (#23722)
Pursuant to reopening of #22732.
Proximate cause is occasional Java bigints getting in where the types assume it'll only be Clojure bigints. On the current reproduction I have it fixes the bug but I am still determining ultimate cause and if there are other phenomena. There was also differential behavior of backport vs. on-master implementation which I still don't understand.
This one needs some sync to have gone through, or else it will prehistoric empetuses towards decelization
---
.../driver/sql_jdbc/sync/describe_table.clj | 15 +++++++++++++--
.../driver/sql_jdbc/sync/describe_table_test.clj | 13 ++++++++++++-
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/src/metabase/driver/sql_jdbc/sync/describe_table.clj b/src/metabase/driver/sql_jdbc/sync/describe_table.clj
index 4caef482a12..00c2330fdae 100644
--- a/src/metabase/driver/sql_jdbc/sync/describe_table.clj
+++ b/src/metabase/driver/sql_jdbc/sync/describe_table.clj
@@ -298,14 +298,17 @@
;; JSON itself has the single number type, but Java serde of JSON is stricter
java.lang.Long :type/Integer
clojure.lang.BigInt :type/BigInteger
+ java.math.BigInteger :type/BigInteger
java.lang.Integer :type/Integer
java.lang.Double :type/Float
+ java.lang.Float :type/Float
java.math.BigDecimal :type/Decimal
java.lang.Number :type/Number
java.lang.Boolean :type/Boolean
java.time.LocalDateTime :type/DateTime
clojure.lang.PersistentVector :type/Array
- clojure.lang.PersistentArrayMap :type/Structured})
+ clojure.lang.PersistentArrayMap :type/Structured
+ clojure.lang.PersistentHashMap :type/Structured})
(def db-type-map
"Map from MBQL types to database types.
@@ -314,7 +317,15 @@
although as of writing this is just geared towards Postgres types"
{:type/Text "text"
:type/Integer "integer"
- :type/BigInteger "bigint"
+ ;; You might think that the ordinary 'bigint' type in Postgres and MySQL should be this.
+ ;; However, Bigint in those DB's maxes out at 2 ^ 64.
+ ;; JSON, like Javascript itself, will happily represent 1.8 * (10^308),
+ ;; Losing digits merrily along the way.
+ ;; We can't really trust anyone to use MAX_SAFE_INTEGER, in JSON-land..
+ ;; So really without forcing arbitrary precision ('decimal' type),
+ ;; we have too many numerical regimes to test.
+ ;; (#22732) was basically the consequence of missing one.
+ :type/BigInteger "decimal"
:type/Float "double precision"
:type/Number "double precision"
:type/Decimal "decimal"
diff --git a/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj b/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
index dcc1c291633..81c9ca9b0ad 100644
--- a/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
+++ b/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj
@@ -114,4 +114,15 @@
(let [row {:bob {:dobbs {:robbs 123} :cobbs [1 2 3]}}
types {[:bob :cobbs] clojure.lang.PersistentVector
[:bob :dobbs :robbs] java.lang.Long}]
- (is (= types (#'sql-jdbc.describe-table/row->types row))))))
+ (is (= types (#'sql-jdbc.describe-table/row->types row)))))
+ (testing "JSON row->types handles bigint that comes in and gets interpreted as Java bigint OK (#22732)"
+ (let [int-row {:zlob {"blob" (java.math.BigInteger. "123124124312134235234235345344324352")}}]
+ (is (= #{{:name "zlob → blob",
+ :database-type "decimal",
+ :base-type :type/BigInteger,
+ :database-position 0,
+ :visibility-type :normal,
+ :nfc-path [:zlob "blob"]}}
+ (-> int-row
+ (#'sql-jdbc.describe-table/row->types)
+ (#'sql-jdbc.describe-table/field-types->fields)))))))
--
GitLab