Skip to content
Snippets Groups Projects
Unverified Commit 710cd8ba authored by Jeff Evans's avatar Jeff Evans Committed by GitHub
Browse files

Add support for the "CHARACTER VARYING" and NUMERIC Redshift type (#14496)

CHARACTER VARYING seems to be an official Redshift type; see: https://docs.aws.amazon.com/redshift/latest/dg/r_Character_types.html#r_Character_types-varchar-or-character-varying

It doesn't always seem to show up.  For example, in a plain CREATE TABLE then that DDL actually is seen by Metabase as "varchar" still.  But if a late binding view is created pointing to that table, then it can show up thusly

NUMERIC is a straight up synonym for DECIMAL; see: https://docs.aws.amazon.com/redshift/latest/dg/r_Numeric_types201.html#r_Numeric_types201-decimal-or-numeric-type

Using the database-type->base-type mechanism to recognize these type name and treat them as :type/Text and :type/Decimal respectively, while delegating to the parent (Postgres) types for any others

Adding a test that creates a table with both and late binding view of that, and asserts the correct types are synced
parent 6b1b4a1e
Branches
Tags
No related merge requests found
......@@ -10,6 +10,7 @@
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql-jdbc.execute.legacy-impl :as legacy]
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.mbql.util :as mbql.u]
[metabase.public-settings :as pubset]
......@@ -92,6 +93,18 @@
;;; | metabase.driver.sql impls |
;;; +----------------------------------------------------------------------------------------------------------------+
;; custom Redshift type handling
(def ^:private database-type->base-type
(sql-jdbc.sync/pattern-based-database-type->base-type
[[#"(?i)CHARACTER VARYING" :type/Text] ; Redshift uses CHARACTER VARYING (N) as a synonym for VARCHAR(N)
[#"(?i)NUMERIC" :type/Decimal]])) ; and also has a NUMERIC(P,S) type, which is the same as DECIMAL(P,S)
(defmethod sql-jdbc.sync/database-type->base-type :redshift
[driver column-type]
(or (database-type->base-type column-type)
((get-method sql-jdbc.sync/database-type->base-type :postgres) driver column-type)))
(defmethod sql.qp/add-interval-honeysql-form :redshift
[_ hsql-form amount unit]
(hsql/call :dateadd (hx/literal unit) amount (hx/->timestamp hsql-form)))
......
(ns metabase.driver.redshift-test
(:require [clojure.string :as str]
(:require [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[clojure.test :refer :all]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.execute :as execute]
[metabase.models.database :refer [Database]]
[metabase.models.field :refer [Field]]
[metabase.models.table :refer [Table]]
[metabase.plugins.jdbc-proxy :as jdbc-proxy]
[metabase.public-settings :as pubset]
[metabase.query-processor :as qp]
[metabase.sync :as sync]
[metabase.test :as mt]
[metabase.test.data.interface :as tx]
[metabase.test.data.redshift :as rstest]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u])
[metabase.util :as u]
[toucan.db :as db])
(:import metabase.plugins.jdbc_proxy.ProxyDriver))
(use-fixtures :once (fixtures/initialize :plugins))
......@@ -154,3 +162,33 @@
:parameters [{:type :date/all-options
:target [:dimension [:template-tag "date"]]
:value "past30years"}]})))))))
(deftest redshift-types-test
(mt/test-driver
:redshift
(testing "Redshift specific types should be synced correctly"
(let [db-details (tx/dbdef->connection-details :redshift)
tbl-nm "redshift_specific_types"
qual-tbl-nm (str rstest/session-schema-name "." tbl-nm)
view-nm "late_binding_view"
qual-view-nm (str rstest/session-schema-name "." view-nm)]
(mt/with-temp Database [database {:engine :redshift, :details db-details}]
;; create a table with a CHARACTER VARYING and a NUMERIC column, and a late bound view that selects from it
(#'rstest/execute!
(str "DROP TABLE IF EXISTS %1$s;%n"
"CREATE TABLE %1$s(weird_varchar CHARACTER VARYING(50), numeric_col NUMERIC(10,2));%n"
"CREATE OR REPLACE VIEW %2$s AS SELECT * FROM %1$s WITH NO SCHEMA BINDING;")
qual-tbl-nm
qual-view-nm)
;; sync the schema again to pick up the new view (and table, though we aren't checking that)
(sync/sync-database! database)
(is (contains?
(db/select-field :name Table :db_id (u/the-id database)) ; the new view should have been synced
view-nm))
(let [table-id (db/select-one-id Table :db_id (u/the-id database), :name view-nm)]
;; and its columns' :base_type should have been identified correctly
(is (= [{:name "weird_varchar", :database_type "character varying(50)", :base_type :type/Text}
{:name "numeric_col", :database_type "numeric(10,2)", :base_type :type/Decimal}]
(map
(partial into {})
(db/select [Field :name :database_type :base_type] :table_id table-id))))))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment