diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj index 6dca119b95d0eda98073e2087b8aa2600ef0a634..22f361db147ac138783c34702db7fd4654a786f9 100644 --- a/src/metabase/driver/postgres.clj +++ b/src/metabase/driver/postgres.clj @@ -1,7 +1,8 @@ (ns metabase.driver.postgres "Database driver for PostgreSQL databases. Builds on top of the 'Generic SQL' driver, which implements most functionality for JDBC-based drivers." - (:require [clojure + (:require [clojure.java.jdbc :as jdbc] + [clojure [set :as set :refer [rename-keys]] [string :as s]] [honeysql.core :as hsql] @@ -15,8 +16,8 @@ [ssh :as ssh]]) (:import java.util.UUID)) -(def ^:private column->base-type - "Map of Postgres column types -> Field base types. +(def ^:private default-base-types + "Map of default Postgres column types -> Field base types. Add more mappings here as you come across them." {:bigint :type/BigInteger :bigserial :type/BigInteger @@ -77,6 +78,15 @@ (keyword "timestamp with timezone") :type/DateTime (keyword "timestamp without timezone") :type/DateTime}) +(defn- column->base-type + "Actual implementation of `column->base-type`. If `:enum-types` is passed along (usually done by our implementation + of `describe-table` below) we'll give the column a base type of `:type/PostgresEnum` *if* it's an enum type. + Otherwise we'll look in the static `default-base-types` map above." + [driver column] + (if (contains? (:enum-types driver) column) + :type/PostgresEnum + (default-base-types column))) + (defn- column->special-type "Attempt to determine the special-type of a Field given its name and Postgres column type." [column-name column-type] @@ -180,13 +190,35 @@ (if-not value value (cond - (isa? base-type :type/UUID) (UUID/fromString value) - (isa? base-type :type/IPAddress) (hx/cast :inet value) - :else value))) + (isa? base-type :type/UUID) (UUID/fromString value) + (isa? base-type :type/IPAddress) (hx/cast :inet value) + (isa? base-type :type/PostgresEnum) (hx/quoted-cast database-type value) + :else value))) (defn- string-length-fn [field-key] (hsql/call :char_length (hx/cast :VARCHAR field-key))) +(defn- enum-types + "Fetch a set of the enum types associated with `database`. + + (enum-types some-db) ; -> #{:bird_type :bird_status}" + [database] + (set + (map (comp keyword :typname) + (jdbc/query (connection-details->spec (:details database)) + [(str "SELECT DISTINCT t.typname " + "FROM pg_enum e " + "LEFT JOIN pg_type t " + " ON t.oid = e.enumtypid")])))) + +(defn- describe-table + "Describe the Fields present in a `table`. This just hands off to the normal SQL driver implementation of the same + name, but first fetches database enum types so we have access to them. These are simply assoc'ed with `driver`, + since that argument will end up getting passed to the function that can actually do something with the enum types, + namely `column->base-type`, which you will find above." + [driver database table] + (sql/describe-table (assoc driver :enum-types (enum-types database)) database table)) + (defrecord PostgresDriver [] clojure.lang.Named @@ -196,9 +228,10 @@ (def ^:private pg-db-time-query "select to_char(current_timestamp, 'YYYY-MM-DD HH24:MI:SS.MS TZ')") (def PostgresISQLDriverMixin - "Implementations of `ISQLDriver` methods for `PostgresDriver`." + "Implementations of `ISQLDriver` methods for `PostgresDriver`. This is made a 'mixin' because these implementations + are also used by the Redshift driver." (merge (sql/ISQLDriverDefaultsMixin) - {:column->base-type (u/drop-first-arg column->base-type) + {:column->base-type column->base-type :column->special-type (u/drop-first-arg column->special-type) :connection-details->spec (u/drop-first-arg connection-details->spec) :date (u/drop-first-arg date) @@ -210,7 +243,9 @@ (u/strict-extend PostgresDriver driver/IDriver (merge (sql/IDriverSQLDefaultsMixin) - {:date-interval (u/drop-first-arg date-interval) + {:current-db-time (driver/make-current-db-time-fn pg-date-formatter pg-db-time-query) + :date-interval (u/drop-first-arg date-interval) + :describe-table describe-table :details-fields (constantly (ssh/with-tunnel-config [{:name "host" :display-name "Host" @@ -238,8 +273,7 @@ {:name "additional-options" :display-name "Additional JDBC connection string options" :placeholder "prepareThreshold=0"}])) - :humanize-connection-error-message (u/drop-first-arg humanize-connection-error-message) - :current-db-time (driver/make-current-db-time-fn pg-date-formatter pg-db-time-query)}) + :humanize-connection-error-message (u/drop-first-arg humanize-connection-error-message)}) sql/ISQLDriver PostgresISQLDriverMixin) diff --git a/src/metabase/query_processor/annotate.clj b/src/metabase/query_processor/annotate.clj index f84d03ccd3c3f80e0dce0059e33d11092da777e4..c5571a49f989d8465618f94f7086de6a991ce323 100644 --- a/src/metabase/query_processor/annotate.clj +++ b/src/metabase/query_processor/annotate.clj @@ -293,7 +293,7 @@ :visibility-type :visibility_type :remapped-to :remapped_to :remapped-from :remapped_from}) - (dissoc :position :clause-position :parent :parent-id :table-name)))) + (dissoc :position :clause-position :parent :parent-id :table-name :database-type)))) (defn- fk-field->dest-fn "Fetch fk info and return a function that returns the destination Field of a given Field." diff --git a/src/metabase/query_processor/interface.clj b/src/metabase/query_processor/interface.clj index 45a7973f40ff387343fefd6e7a17ccf2d36c653e..5af62aee7abbe22b83e9b3091f46574fc311ba73 100644 --- a/src/metabase/query_processor/interface.clj +++ b/src/metabase/query_processor/interface.clj @@ -108,6 +108,7 @@ (s/defrecord Field [field-id :- su/IntGreaterThanZero field-name :- su/NonBlankString field-display-name :- su/NonBlankString + database-type :- su/NonBlankString base-type :- su/FieldType special-type :- (s/maybe su/FieldType) visibility-type :- (apply s/enum field/visibility-types) diff --git a/src/metabase/query_processor/middleware/resolve.clj b/src/metabase/query_processor/middleware/resolve.clj index e63f26f3e3a950105caacb3e7b0a997d31aea8b2..fd797772a60938fbb5e73029ad3d9f184b00b2a9 100644 --- a/src/metabase/query_processor/middleware/resolve.clj +++ b/src/metabase/query_processor/middleware/resolve.clj @@ -38,6 +38,7 @@ :display_name :field-display-name :special_type :special-type :visibility_type :visibility-type + :database_type :database-type :base_type :base-type :table_id :table-id :parent_id :parent-id})) @@ -290,7 +291,7 @@ Clojure-style as expected by the rest of the QP code." [field-ids] (as-> (db/select [field/Field :name :display_name :base_type :special_type :visibility_type :table_id :parent_id - :description :id :fingerprint] + :description :id :fingerprint :database_type] :visibility_type [:not= "sensitive"] :id [:in field-ids]) fields ;; hydrate values & dimensions for the `fields` we just fetched from the DB diff --git a/src/metabase/types.clj b/src/metabase/types.clj index 4ad5b2f8d3755e0a72d12852d17ecbe39578f0b2..c79718ed1bed28c92b2d746a58d7ede5d4b5bd42 100644 --- a/src/metabase/types.clj +++ b/src/metabase/types.clj @@ -48,6 +48,8 @@ (derive :type/SerializedJSON :type/Text) (derive :type/SerializedJSON :type/Collection) +(derive :type/PostgresEnum :type/Text) + ;;; DateTime Types (derive :type/DateTime :type/*) diff --git a/src/metabase/util/honeysql_extensions.clj b/src/metabase/util/honeysql_extensions.clj index 6b7dee718461cbe9351d296bc7d38111d01de5cd..c9b179d530ab3cc25be40fe69137621d3886690a 100644 --- a/src/metabase/util/honeysql_extensions.clj +++ b/src/metabase/util/honeysql_extensions.clj @@ -111,6 +111,14 @@ [c x] (hsql/call :cast x (hsql/raw (name c)))) +(defn quoted-cast + "Generate a statement like `cast(x AS \"c\")`. + + Like `cast` but quotes the type C. This is useful for cases where we deal with user-defined types or other types + that may have a space in the name, for example Postgres enum types." + [c x] + (hsql/call :cast x (keyword c))) + (defn format "SQL `format` function." [format-str expr] diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj index f4900d14a5297a304d55b49dfb6784af71332c3d..824f26e2e56385cb3871d3bcac57a4b087c10924 100644 --- a/test/metabase/driver/postgres_test.clj +++ b/test/metabase/driver/postgres_test.clj @@ -10,12 +10,14 @@ [util :as u]] [metabase.driver [generic-sql :as sql] - postgres] + [postgres :as postgres]] [metabase.models [database :refer [Database]] [field :refer [Field]] [table :refer [Table]]] + [metabase.query-processor :as qp] [metabase.query-processor.middleware.expand :as ql] + [metabase.sync.sync-metadata :as sync-metadata] [metabase.test [data :as data] [util :as tu]] @@ -302,3 +304,101 @@ (tt/with-temp Database [database {:engine :postgres, :details (assoc details :dbname "time_field_test")}] (sync/sync-database! database) (set (db/select [Field :name :fingerprint] :table_id (db/select-one-id Table :db_id (u/get-id database)))))))) + + +;;; +----------------------------------------------------------------------------------------------------------------+ +;;; | POSTGRES ENUM SUPPORT | +;;; +----------------------------------------------------------------------------------------------------------------+ + +(defn- enums-test-db-details [] (i/database->connection-details pg-driver :db {:database-name "enums_test"})) + +(defn- create-enums-db! + "Create a Postgres database called `enums_test` that has a couple of enum types and a couple columns of those types. + One of those types has a space in the name, which is legal when quoted, to make sure we handle such wackiness + properly." + [] + (drop-if-exists-and-create-db! "enums_test") + (jdbc/with-db-connection [conn (sql/connection-details->spec pg-driver (enums-test-db-details))] + (doseq [sql ["CREATE TYPE \"bird type\" AS ENUM ('toucan', 'pigeon', 'turkey');" + "CREATE TYPE bird_status AS ENUM ('good bird', 'angry bird', 'delicious bird');" + (str "CREATE TABLE birds (" + " name varchar PRIMARY KEY NOT NULL," + " type \"bird type\" NOT NULL," + " status bird_status NOT NULL" + ");") + (str "INSERT INTO birds (\"name\", \"type\", status) VALUES" + " ('Rasta', 'toucan', 'good bird')," + " ('Lucky', 'pigeon', 'angry bird')," + " ('Theodore', 'turkey', 'delicious bird');")]] + (jdbc/execute! conn [sql])))) + +(defn- do-with-enums-db {:style/indent 0} [f] + (create-enums-db!) + (tt/with-temp Database [database {:engine :postgres, :details (enums-test-db-details)}] + (sync-metadata/sync-db-metadata! database) + (f database))) + +;; check that we can actually fetch the enum types from a DB +(expect-with-engine :postgres + #{(keyword "bird type") :bird_status} + (do-with-enums-db + (fn [db] + (#'postgres/enum-types db)))) + +;; check that describe-table properly describes the database & base types of the enum fields +(expect-with-engine :postgres + {:name "birds" + :fields #{{:name "name", + :database-type "varchar" + :base-type :type/Text + :pk? true} + {:name "status" + :database-type "bird_status" + :base-type :type/PostgresEnum} + {:name "type" + :database-type "bird type" + :base-type :type/PostgresEnum}}} + (do-with-enums-db + (fn [db] + (driver/describe-table pg-driver db {:name "birds"})))) + +;; check that when syncing the DB the enum types get recorded appropriately +(expect-with-engine :postgres + #{{:name "name", :database_type "varchar", :base_type :type/Text} + {:name "type", :database_type "bird type", :base_type :type/PostgresEnum} + {:name "status", :database_type "bird_status", :base_type :type/PostgresEnum}} + (do-with-enums-db + (fn [db] + (let [table-id (db/select-one-id Table :db_id (u/get-id db), :name "birds")] + (set (map (partial into {}) + (db/select [Field :name :database_type :base_type] :table_id table-id))))))) + + +;; check that values for enum types get wrapped in appropriate CAST() fn calls in prepare-value +(expect-with-engine :postgres + {:name :cast, :args ["toucan" (keyword "bird type")]} + (#'postgres/prepare-value {:field {:database-type "bird type", :base-type :type/PostgresEnum} + :value "toucan"})) + +;; End-to-end check: make sure everything works as expected when we run an actual query +(expect-with-engine :postgres + {:rows [["Rasta" "good bird" "toucan"]] + :native_form {:query (str "SELECT \"public\".\"birds\".\"name\" AS \"name\"," + " \"public\".\"birds\".\"status\" AS \"status\"," + " \"public\".\"birds\".\"type\" AS \"type\" " + "FROM \"public\".\"birds\" " + "WHERE \"public\".\"birds\".\"type\" = CAST(? AS \"bird type\") " + "LIMIT 10") + :params ["toucan"]}} + (do-with-enums-db + (fn [db] + (let [table-id (db/select-one-id Table :db_id (u/get-id db), :name "birds") + bird-type-field-id (db/select-one-id Field :table_id table-id, :name "type")] + (-> (qp/process-query + {:database (u/get-id db) + :type :query + :query {:source-table table-id + :filter [:= [:field-id (u/get-id bird-type-field-id)] "toucan"] + :limit 10}}) + :data + (select-keys [:rows :native_form])))))) diff --git a/test/metabase/query_processor/expand_resolve_test.clj b/test/metabase/query_processor/expand_resolve_test.clj index a2daf8cb270e28900ac954bdc7798ba4a91a3667..ec2f42503df3e2b1ff005786bb70997d3a055783 100644 --- a/test/metabase/query_processor/expand_resolve_test.clj +++ b/test/metabase/query_processor/expand_resolve_test.clj @@ -88,6 +88,7 @@ {:field-id true :field-name "PRICE" :field-display-name "Price" + :database-type "INTEGER" :base-type :type/Integer :special-type :type/Category :table-id (id :venues) @@ -101,6 +102,7 @@ {:field-id true :field-name "PRICE" :field-display-name "Price" + :database-type "INTEGER" :base-type :type/Integer :special-type :type/Category :table-id (id :venues) @@ -154,6 +156,7 @@ :fk-field-id (id :venues :category_id) :field-name "NAME" :field-display-name "Name" + :database-type "VARCHAR" :base-type :type/Text :special-type :type/Name :table-id (id :categories) @@ -170,6 +173,7 @@ :fk-field-id (id :venues :category_id) :field-name "NAME" :field-display-name "Name" + :database-type "VARCHAR" :base-type :type/Text :special-type :type/Name :table-id (id :categories) @@ -226,6 +230,7 @@ :fk-field-id (id :checkins :user_id) :field-name "LAST_LOGIN" :field-display-name "Last Login" + :database-type "TIMESTAMP" :base-type :type/DateTime :special-type nil :table-id (id :users) @@ -239,6 +244,7 @@ :fk-field-id (id :checkins :user_id) :field-name "LAST_LOGIN" :field-display-name "Last Login" + :database-type "TIMESTAMP" :base-type :type/DateTime :special-type nil :visibility-type :normal @@ -286,7 +292,8 @@ :aggregation [{:aggregation-type :sum :custom-name nil :field (merge field-defaults - {:base-type :type/Integer + {:database-type "INTEGER" + :base-type :type/Integer :table-id (id :venues) :special-type :type/Category :field-name "PRICE" @@ -298,7 +305,8 @@ :fingerprint {:global {:distinct-count 4} :type {:type/Number {:min 1, :max 4, :avg 2.03}}}})}] :breakout [{:field (merge field-defaults - {:base-type :type/Date + {:database-type "DATE" + :base-type :type/Date :table-id (id :checkins) :special-type nil :field-name "DATE"