diff --git a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj index 6deec425eeb3cd9c67af8240afa65885fc1df290..b4713fde16a995276d5824f45f1d6d6df8c8e7c6 100644 --- a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj +++ b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj @@ -92,16 +92,17 @@ (defn- bigquery-type->base-type [field-type] (case field-type - "BOOLEAN" :type/Boolean - "FLOAT" :type/Float - "INTEGER" :type/Integer - "RECORD" :type/Dictionary ; RECORD -> field has a nested schema - "STRING" :type/Text - "DATE" :type/Date - "DATETIME" :type/DateTime - "TIMESTAMP" :type/DateTimeWithLocalTZ - "TIME" :type/Time - "NUMERIC" :type/Decimal + "BOOLEAN" :type/Boolean + "FLOAT" :type/Float + "INTEGER" :type/Integer + "RECORD" :type/Dictionary ; RECORD -> field has a nested schema + "STRING" :type/Text + "DATE" :type/Date + "DATETIME" :type/DateTime + "TIMESTAMP" :type/DateTimeWithLocalTZ + "TIME" :type/Time + "NUMERIC" :type/Decimal + "BIGNUMERIC" :type/Decimal :type/*)) (s/defn ^:private table-schema->metabase-field-info diff --git a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/params.clj b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/params.clj index e109ce5dfa3ce12093f035b1a88abbca128c0152..b544a1bc1bfb0089a012a2f6549c49a862f5b14c 100644 --- a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/params.clj +++ b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/params.clj @@ -37,7 +37,19 @@ (defmethod ->QueryParameterValue clojure.lang.BigInt [v] (param "INT64" v)) (defmethod ->QueryParameterValue Float [v] (param "FLOAT64" v)) (defmethod ->QueryParameterValue Double [v] (param "FLOAT64" v)) -(defmethod ->QueryParameterValue java.math.BigDecimal [v] (param "FLOAT64" v)) + +;; use the min and max values for the NUMERIC types to figure out if we need to set decimal params as NUMERIC +;; or BIGNUMERIC +(def ^:private ^:const ^BigDecimal max-bq-numeric-val (bigdec "9.9999999999999999999999999999999999999E+28")) +(def ^:private ^:const ^BigDecimal min-bq-numeric-val (.negate max-bq-numeric-val)) + +(defmethod ->QueryParameterValue java.math.BigDecimal [^BigDecimal v] + (if (or (and (< 0 (.signum v)) + (< 0 (.compareTo v min-bq-numeric-val))) + (and (> 0 (.signum v)) + (> 0 (.compareTo v max-bq-numeric-val)))) + (param "BIGNUMERIC" v) + (param "NUMERIC" v))) (defmethod ->QueryParameterValue java.time.LocalDate [t] (param "DATE" (u.date/format t))) (defmethod ->QueryParameterValue java.time.LocalDateTime [t] (param "DATETIME" (u.date/format t))) diff --git a/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/params_test.clj b/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/params_test.clj index b0b2aeece210883ec117eb178c7467872f8a9b20..f33a7d1ee79e55d01f4d105d62a89c1e2bace590 100644 --- a/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/params_test.clj +++ b/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/params_test.clj @@ -17,7 +17,12 @@ [(bigint 100) {:base-type :type/Integer}] [(float 100.0) {:base-type :type/Float}] [(double 100.0) {:base-type :type/Float}] - [(bigdec 100.0) {:base-type :type/Float, :v 100.0}] + ;; one case for NUMERIC/DECIMAL + [(bigdec "-2.3E+16") {:base-type :type/Decimal + :v "-23000000000000000"}] + ;; and one for BIGNUMERIC/BIGDECIMAL + [(bigdec "1.6E+31") {:base-type :type/Decimal + :v "16000000000000000000000000000000"}] ;; LocalDate [#t "2020-05-26" {:base-type :type/Date :v #t "2020-05-26T00:00Z[UTC]"}] diff --git a/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk_test.clj b/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk_test.clj index a7aa1bfb624aa56a3980f13da88eedc64a351f8e..0a4a22904eb5229c9cb419c80cff7fe8463af721 100644 --- a/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk_test.clj +++ b/modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk_test.clj @@ -144,29 +144,50 @@ (is (= "UTC" (tu/db-timezone-id))))) -(defn- do-with-view [f] +(defn- do-with-temp-obj [name-fmt-str create-args-fn drop-args-fn f] (driver/with-driver :bigquery-cloud-sdk - (let [view-name (format "view_%s" (tu/random-name))] + (let [obj-name (format name-fmt-str (tu/random-name))] (mt/with-temp-copy-of-db (try - (bigquery.tx/execute! - (str "CREATE VIEW `v3_test_data.%s` " - "AS " - "SELECT v.id AS id, v.name AS venue_name, c.name AS category_name " - "FROM `%s.v3_test_data.venues` v " - "LEFT JOIN `%s.v3_test_data.categories` c " - "ON v.category_id = c.id " - "ORDER BY v.id ASC " - "LIMIT 3") - view-name - (bigquery.tx/project-id) - (bigquery.tx/project-id)) - (f view-name) + (apply bigquery.tx/execute! (create-args-fn obj-name)) + (f obj-name) (finally - (bigquery.tx/execute! "DROP VIEW IF EXISTS `v3_test_data.%s`" view-name))))))) + (apply bigquery.tx/execute! (drop-args-fn obj-name)))))))) -(defmacro ^:private with-view [[view-name-binding] & body] - `(do-with-view (fn [~(or view-name-binding '_)] ~@body))) +(defmacro with-view [[view-name-binding] & body] + `(do-with-temp-obj "view_%s" + (fn [view-nm#] [(str "CREATE VIEW `v3_test_data.%s` AS " + "SELECT v.id AS id, v.name AS venue_name, c.name AS category_name " + "FROM `%s.v3_test_data.venues` v " + "LEFT JOIN `%s.v3_test_data.categories` c " + "ON v.category_id = c.id " + "ORDER BY v.id ASC " + "LIMIT 3") + view-nm# + (bigquery.tx/project-id) + (bigquery.tx/project-id)]) + (fn [view-nm#] ["DROP VIEW IF EXISTS `v3_test_data.%s`" view-nm#]) + (fn [~(or view-name-binding '_)] ~@body))) + +(def ^:private numeric-val "-1.2E20") +(def ^:private decimal-val "2.3E16") +(def ^:private bignumeric-val "-7.5E30") +(def ^:private bigdecimal-val "5.2E35") + +(defmacro with-numeric-types-table [[table-name-binding] & body] + `(do-with-temp-obj "table_%s" + (fn [tbl-nm#] [(str "CREATE TABLE `v3_test_data.%s` AS SELECT " + "NUMERIC '%s' AS numeric_col, " + "DECIMAL '%s' AS decimal_col, " + "BIGNUMERIC '%s' AS bignumeric_col, " + "BIGDECIMAL '%s' AS bigdecimal_col") + tbl-nm# + ~numeric-val + ~decimal-val + ~bignumeric-val + ~bigdecimal-val]) + (fn [tbl-nm#] ["DROP TABLE IF EXISTS `v3_test_data.%s`" tbl-nm#]) + (fn [~(or table-name-binding '_)] ~@body))) (deftest sync-views-test (mt/test-driver :bigquery-cloud-sdk @@ -256,3 +277,41 @@ (mt/run-mbql-query taxi_trips {:filter [:= [:field (mt/id :taxi_trips :unique_key) nil] "67794e631648a002f88d4b7f3ab0bcb6a9ed306a"]})))))))))) + +(deftest bigquery-specific-types-test + (testing "Table with decimal types" + (with-numeric-types-table [tbl-nm] + (is (contains? (:tables (driver/describe-database :bigquery-cloud-sdk (mt/db))) + {:schema nil, :name tbl-nm}) + "`describe-database` should see the table") + (is (= {:schema nil + :name tbl-nm + :fields #{{:name "numeric_col", :database-type "NUMERIC", :base-type :type/Decimal, :database-position 0} + {:name "decimal_col", :database-type "NUMERIC", :base-type :type/Decimal, :database-position 1} + {:name "bignumeric_col" + :database-type "BIGNUMERIC" + :base-type :type/Decimal + :database-position 2} + {:name "bigdecimal_col" + :database-type "BIGNUMERIC" + :base-type :type/Decimal + :database-position 3}}} + (driver/describe-table :bigquery-cloud-sdk (mt/db) {:name tbl-nm})) + "`describe-table` should see the fields in the table") + (sync/sync-database! (mt/db)) + (testing "We should be able to run queries against the table" + (doseq [[col-nm param-v] [[:numeric_col (bigdec numeric-val)] + [:decimal_col (bigdec decimal-val)] + [:bignumeric_col (bigdec bignumeric-val)] + [:bigdecimal_col (bigdec bigdecimal-val)]]] + (testing (format "filtering against %s" col-nm)) + (is (= 1 + (-> (mt/first-row + (mt/run-mbql-query nil + {:source-table (mt/id tbl-nm) + :aggregation [[:count]] + :parameters [{:name col-nm + :type :number/= + :target [:field (mt/id tbl-nm col-nm)] + :value [param-v]}]})) + first))))))))