From 3822d2cda86aedd4d442a119e0640a1ed371fc03 Mon Sep 17 00:00:00 2001 From: Cam Saul <1455846+camsaul@users.noreply.github.com> Date: Thu, 20 Jan 2022 13:11:54 -0800 Subject: [PATCH] Add test for #7487 (#19810) * Add test for #7487 * Fix error message * Update test for new error message --- .../test/metabase/driver/redshift_test.clj | 48 ++++++++++++++----- src/metabase/models/field.clj | 10 ++-- test/metabase/models/field_test.clj | 3 +- .../test/data/dataset_definitions.clj | 2 +- 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/modules/drivers/redshift/test/metabase/driver/redshift_test.clj b/modules/drivers/redshift/test/metabase/driver/redshift_test.clj index 72bfba97762..30a7347bd4f 100644 --- a/modules/drivers/redshift/test/metabase/driver/redshift_test.clj +++ b/modules/drivers/redshift/test/metabase/driver/redshift_test.clj @@ -216,13 +216,13 @@ user-pw "Password1234" db-det (:details (mt/db))] (redshift.test/execute! (str "CREATE SCHEMA %s;" - "CREATE USER %s PASSWORD '%s';%n" - "GRANT USAGE ON SCHEMA %s TO %s;%n") - random-schema - temp-username - user-pw - random-schema - temp-username) + "CREATE USER %s PASSWORD '%s';%n" + "GRANT USAGE ON SCHEMA %s TO %s;%n") + random-schema + temp-username + user-pw + random-schema + temp-username) (try (binding [redshift.test/*use-original-syncable-schemas-impl?* true] (mt/with-temp Database [db {:engine :redshift, :details (assoc db-det :user temp-username :password user-pw)}] @@ -236,12 +236,12 @@ (is (not (contains? schemas redshift.test/session-schema-name)))))))) (finally (redshift.test/execute! (str "REVOKE USAGE ON SCHEMA %s FROM %s;%n" - "DROP USER IF EXISTS %s;%n" - "DROP SCHEMA IF EXISTS %s;%n") - random-schema - temp-username - temp-username - random-schema))))) + "DROP USER IF EXISTS %s;%n" + "DROP SCHEMA IF EXISTS %s;%n") + random-schema + temp-username + temp-username + random-schema))))) (testing "Should filter out non-existent schemas (for which nobody has permissions)" (let [fake-schema-name (u/qualified-name ::fake-schema)] @@ -265,3 +265,25 @@ (is (contains? (schemas) fake-schema-name)))) (testing "normally, ::fake-schema should be filtered out (because it does not exist)" (is (not (contains? (schemas) fake-schema-name))))))))))))) + +(mt/defdataset numeric-unix-timestamps + [["timestamps" + [{:field-name "timestamp", :base-type {:native "numeric"}}] + [[1642704550656]]]]) + +(deftest numeric-unix-timestamp-test + (mt/test-driver :redshift + (testing "NUMERIC columns should work with UNIX timestamp conversion (#7487)" + (mt/dataset numeric-unix-timestamps + (testing "without coercion strategy" + (let [query (mt/mbql-query timestamps)] + (mt/with-native-query-testing-context query + (is (= [1 1642704550656M] + (mt/first-row (qp/process-query query))))))) + (testing "WITH coercion strategy" + (mt/with-temp-vals-in-db Field (mt/id :timestamps :timestamp) {:coercion_strategy :Coercion/UNIXMilliSeconds->DateTime + :effective_type :type/Instant} + (let [query (mt/mbql-query timestamps)] + (mt/with-native-query-testing-context query + (is (= [1 "2022-01-20T18:49:10.656Z"] + (mt/first-row (qp/process-query query)))))))))))) diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj index df73bafb266..0569206769d 100644 --- a/src/metabase/models/field.clj +++ b/src/metabase/models/field.clj @@ -66,11 +66,13 @@ (when-not (some (partial isa? k) ancestor-types) - (let [message (tru "Invalid Field {0} {1}" column-name k)] + (let [message (tru "Invalid value for Field column {0}: {1} is not a descendant of any of these types: {2}" + (pr-str column-name) (pr-str k) (pr-str ancestor-types))] (throw (ex-info message - {:status-code 400 - :errors {column-name message} - :value k})))) + {:status-code 400 + :errors {column-name message} + :value k + :allowed-ancestors ancestor-types})))) (u/qualified-name k)))) (defn- hierarchy-keyword-out [column-name & {:keys [fallback-type ancestor-types]}] diff --git a/test/metabase/models/field_test.clj b/test/metabase/models/field_test.clj index e2f54816af1..72349f8470f 100644 --- a/test/metabase/models/field_test.clj +++ b/test/metabase/models/field_test.clj @@ -29,6 +29,7 @@ (testing (format "Should throw an Exception if you attempt to save a Field with an invalid %s" column) (is (thrown-with-msg? clojure.lang.ExceptionInfo - (re-pattern (format "Invalid Field %s %s" column unknown-type)) + (re-pattern (format "Invalid value for Field column %s: %s is not a descendant of any of these types:" + column unknown-type)) (mt/with-temp Field [field {column unknown-type}] field)))))) diff --git a/test/metabase/test/data/dataset_definitions.clj b/test/metabase/test/data/dataset_definitions.clj index d06ebd2d256..e045ef8eb71 100644 --- a/test/metabase/test/data/dataset_definitions.clj +++ b/test/metabase/test/data/dataset_definitions.clj @@ -209,7 +209,7 @@ (t/local-time t) ; time (t/offset-time t) ; time-ltz (t/offset-time t) ; time-tz - cnt])]]) ; num-crows + cnt])]]) ; num-crows (tx/defdataset dots-in-names [["objects.stuff" -- GitLab