diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj index 77046bb0795346fce85e15129cbb91932a45e31b..3a736e876367a00b4abbe4d590c6a28539c9ce34 100644 --- a/src/metabase/driver/postgres.clj +++ b/src/metabase/driver/postgres.clj @@ -281,51 +281,67 @@ (defmethod sql-jdbc.sync/describe-fields-sql :postgres ;; The implementation is based on `getColumns` in https://github.com/pgjdbc/pgjdbc/blob/fcc13e70e6b6bb64b848df4b4ba6b3566b5e95a3/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java [driver & {:keys [schema-names table-names]}] - (sql/format {:select [[:c.column_name :name] - [:c.udt_name :database-type] - [[:- :c.ordinal_position [:inline 1]] :database-position] - [:c.table_schema :table-schema] - [:c.table_name :table-name] - [[:not= :pk.column_name nil] :pk?] - - [[:col_description - [:cast [:cast [:format [:inline "%I.%I"] [:cast :c.table_schema :text] [:cast :c.table_name :text]] :regclass] :oid] - :c.ordinal_position] - :field-comment] - [[:and - [:or [:= :column_default nil] [:= [:lower :column_default] [:inline "null"]]] - [:= :is_nullable [:inline "NO"]] - ;;_ IS_AUTOINCREMENT from: https://github.com/pgjdbc/pgjdbc/blob/fcc13e70e6b6bb64b848df4b4ba6b3566b5e95a3/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java#L1852-L1856 - [:not [:or - [:and [:!= :column_default nil] [:like :column_default [:inline "%nextval(%"]]] - [:!= :is_identity [:inline "NO"]]]]] - :database-required] - [[:or - [:and [:!= :column_default nil] [:like :column_default [:inline "%nextval(%"]]] - [:!= :is_identity [:inline "NO"]]] - :database-is-auto-increment]] - :from [[:information_schema.columns :c]] - :left-join [[{:select [:tc.table_schema - :tc.table_name - :kc.column_name] - :from [[:information_schema.table_constraints :tc]] - :join [[:information_schema.key_column_usage :kc] - [:and - [:= :tc.constraint_name :kc.constraint_name] - [:= :tc.table_schema :kc.table_schema] - [:= :tc.table_name :kc.table_name]]] - :where [:= :tc.constraint_type [:inline "PRIMARY KEY"]]} - :pk] + (sql/format + {:union-all + [{:select [[:c.column_name :name] + [:c.udt_name :database-type] + [[:- :c.ordinal_position [:inline 1]] :database-position] + [:c.table_schema :table-schema] + [:c.table_name :table-name] + [[:not= :pk.column_name nil] :pk?] + [[:col_description + [:cast [:cast [:format [:inline "%I.%I"] [:cast :c.table_schema :text] [:cast :c.table_name :text]] :regclass] :oid] + :c.ordinal_position] + :field-comment] + [[:and + [:or [:= :column_default nil] [:= [:lower :column_default] [:inline "null"]]] + [:= :is_nullable [:inline "NO"]] + ;;_ IS_AUTOINCREMENT from: https://github.com/pgjdbc/pgjdbc/blob/fcc13e70e6b6bb64b848df4b4ba6b3566b5e95a3/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java#L1852-L1856 + [:not [:or + [:and [:!= :column_default nil] [:like :column_default [:inline "%nextval(%"]]] + [:!= :is_identity [:inline "NO"]]]]] + :database-required] + [[:or + [:and [:!= :column_default nil] [:like :column_default [:inline "%nextval(%"]]] + [:!= :is_identity [:inline "NO"]]] + :database-is-auto-increment]] + :from [[:information_schema.columns :c]] + :left-join [[{:select [:tc.table_schema + :tc.table_name + :kc.column_name] + :from [[:information_schema.table_constraints :tc]] + :join [[:information_schema.key_column_usage :kc] [:and - [:= :c.table_schema :pk.table_schema] - [:= :c.table_name :pk.table_name] - [:= :c.column_name :pk.column_name]]] - :where [:and - [:raw "c.table_schema !~ '^information_schema|catalog_history|pg_'"] - (when schema-names [:in :c.table_schema schema-names]) - (when table-names [:in :c.table_name table-names])] - :order-by [:table-schema :table-name :database-position]} - :dialect (sql.qp/quote-style driver))) + [:= :tc.constraint_name :kc.constraint_name] + [:= :tc.table_schema :kc.table_schema] + [:= :tc.table_name :kc.table_name]]] + :where [:= :tc.constraint_type [:inline "PRIMARY KEY"]]} + :pk] + [:and + [:= :c.table_schema :pk.table_schema] + [:= :c.table_name :pk.table_name] + [:= :c.column_name :pk.column_name]]] + :where [:and + [:raw "c.table_schema !~ '^information_schema|catalog_history|pg_'"] + (when schema-names [:in :c.table_schema schema-names]) + (when table-names [:in :c.table_name table-names])]} + {:select [[:pa.attname :name] + [:pt.typname :database-type] + [[:- :pa.attnum [:inline 1]] :database-position] + [:pn.nspname :table-schema] + [:pc.relname :table-name] + [false :pk?] + [nil :field-comment] + [false :database-required] + [false :database-is-auto-increment]] + :from [[:pg_catalog.pg_class :pc]] + :join [[:pg_catalog.pg_namespace :pn] [:= :pn.oid :pc.relnamespace] + [:pg_catalog.pg_attribute :pa] [:= :pa.attrelid :pc.oid] + [:pg_catalog.pg_type :pt] [:= :pt.oid :pa.atttypid]] + :where [:and [:= :pc.relkind [:inline "m"]] + [:>= :pa.attnum 1]]}] + :order-by [:table-schema :table-name :database-position]} + :dialect (sql.qp/quote-style driver))) (defmethod sql-jdbc.sync/describe-fks-sql :postgres [driver & {:keys [schema-names table-names]}] diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj index 20f3f25bb576fe6411e38b41a2e1c57caa4e468c..98bc25a3127c47ed30b0b13d3a5fc01253c167af 100644 --- a/test/metabase/driver/postgres_test.clj +++ b/test/metabase/driver/postgres_test.clj @@ -37,6 +37,7 @@ [metabase.sync.sync-metadata.tables :as sync-tables] [metabase.sync.util :as sync-util] [metabase.test :as mt] + [metabase.test.data.sql :as sql.tx] [metabase.util :as u] [metabase.util.honey-sql-2 :as h2x] [metabase.util.log :as log] @@ -897,6 +898,75 @@ :data (select-keys [:rows :native_form])))))))))) +(deftest enums-test-3 + (mt/test-driver :postgres + (do-with-enums-db! + (fn [db] + (jdbc/execute! (sql-jdbc.conn/db->pooled-connection-spec db) + [(sql.tx/create-materialized-view-of-table-sql driver/*driver* db "birds_m" "birds")]) + (sync/sync-database! db) + + (testing "check that describe-table properly describes the database & base types of the enum fields" + (is (=? [{:table-schema "public" + :table-name "birds_m" + :name "name" + :database-type "varchar" + :base-type :type/Text + :pk? false + :database-position 0 + :database-required false + :database-is-auto-increment false + :json-unfolding false} + {:table-schema "public" + :table-name "birds_m" + :name "status" + :database-type "bird_status" + :base-type :type/PostgresEnum + :database-position 1 + :database-required false + :database-is-auto-increment false + :json-unfolding false} + {:table-schema "public" + :table-name "birds_m" + :name "type" + :database-type "bird type" + :base-type :type/PostgresEnum + :database-position 2 + :database-required false + :database-is-auto-increment false + :json-unfolding false}] + (->> (driver/describe-fields :postgres db {:table-names ["birds_m"]}) + (into #{}) + (sort-by :database-position))))) + + (testing "check that when syncing the DB the enum types get recorded appropriately" + (let [table-id (t2/select-one-pk Table :db_id (u/the-id db), :name "birds_m")] + (is (= #{{: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}} + (set (map (partial into {}) + (t2/select [Field :name :database_type :base_type] :table_id table-id))))))) + + (testing "End-to-end check: make sure everything works as expected when we run an actual query" + (let [table-id (t2/select-one-pk Table :db_id (u/the-id db), :name "birds_m") + bird-type-field-id (t2/select-one-pk Field :table_id table-id, :name "type")] + (is (= {:rows [["Rasta" "good bird" "toucan"]] + :native_form {:query (str "SELECT \"public\".\"birds_m\".\"name\" AS \"name\"," + " \"public\".\"birds_m\".\"status\" AS \"status\"," + " \"public\".\"birds_m\".\"type\" AS \"type\" " + "FROM \"public\".\"birds_m\" " + "WHERE \"public\".\"birds_m\".\"type\" = CAST('toucan' AS \"bird type\") " + "LIMIT 10") + :params nil}} + (-> (qp/process-query + {:database (u/the-id db) + :type :query + :query {:source-table table-id + :filter [:= [:field (u/the-id bird-type-field-id) nil] "toucan"] + :limit 10}}) + :data + (select-keys [:rows :native_form])))))))))) + (deftest enums-actions-test (mt/test-driver :postgres (testing "actions with enums" 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 41419f813d0822b31a37b594384e20c50def0d10..4757c3988f31c6c2407768b98477b352b9096405 100644 --- a/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj +++ b/test/metabase/driver/sql_jdbc/sync/describe_table_test.clj @@ -754,3 +754,53 @@ (sql.tx/create-index-sql driver/*driver* "conditional_index" ["column"] {:condition "id > 2"})) (is (= #{{:type :normal-column-index :value "id"}} (describe-table-indexes (t2/select-one :model/Table (mt/id :conditional_index)))))))))) + +(defmethod driver/database-supports? [::driver/driver ::materialized-view-fields] + [_driver _feature _database] + false) + +;; TODO Make all drivers that support materialized-views pass this test +(doseq [driver [:postgres + #_:redshift + #_:vertica + #_:athena + #_:bigquery-cloud-sdk]] + (defmethod driver/database-supports? [driver ::describe-materialized-view-fields] + [_driver _feature _database] + true)) + +(defmethod driver/database-supports? [:redshift ::describe-materialized-view-fields] + [_driver _feature _database] + false) + +(deftest describe-materialized-view-fields + (mt/test-drivers (set/intersection (mt/normal-drivers-with-feature ::describe-materialized-view-fields) + (mt/sql-jdbc-drivers)) + (do-with-temporary-dataset + (mt/dataset-definition "describe-materialized-views-test" + ["orders" + [{:field-name "amount" :base-type :type/Integer}] + [[1 2]]]) + (fn [] + (try + ;; Move creating and dropping view to a sql-jdbc defmethod of a metabase.test.data defmulti + (jdbc/execute! (sql-jdbc.conn/db->pooled-connection-spec (mt/db)) + [(sql.tx/create-materialized-view-of-table-sql driver/*driver* (mt/db) "orders_m" "orders")]) + (sync/sync-database! (mt/db)) + (let [orders-id (t2/select-one-pk :model/Table :db_id (mt/id) [:lower :name] "orders") + orders-m-id (t2/select-one-pk :model/Table :db_id (mt/id) [:lower :name] "orders_m")] + (is (= [["id" :type/Integer 0] + ["amount" :type/Integer 1]] + (t2/select-fn-vec + (juxt (comp u/lower-case-en :name) :base_type :database_position) + :model/Field + :table_id orders-id + {:order-by [:database_position]}) + (t2/select-fn-vec + (juxt (comp u/lower-case-en :name) :base_type :database_position) + :model/Field + :table_id orders-m-id + {:order-by [:database_position]})))) + (finally + (jdbc/execute! (sql-jdbc.conn/db->pooled-connection-spec (mt/db)) + [(sql.tx/drop-materialized-view-sql driver/*driver* (mt/db) "orders_m")]))))))) diff --git a/test/metabase/test/data/sql.clj b/test/metabase/test/data/sql.clj index 3203168d4a3d27e3597ace4177b26f235a9388bf..560abefbc89f15dd6dad60de93a44fe076c84488 100644 --- a/test/metabase/test/data/sql.clj +++ b/test/metabase/test/data/sql.clj @@ -343,6 +343,41 @@ (defmethod session-schema :sql/test-extensions [_] nil) +(defmulti create-materialized-view-of-table-sql + "Return a `CREATE MATERIALIZED VIEW` statement. + The view should be a simple view of the table, like `select * from table` + `view-name` is the name of the new view + `table-name` is the name of the table." + {:arglists '([driver database view-name table-name])} + tx/dispatch-on-driver-with-test-extensions + :hierarchy #'driver/hierarchy) + +(defmethod create-materialized-view-of-table-sql :sql/test-extensions + [driver _database view-name table-name] + (let [qualified-view-name (sql.u/quote-name driver :table view-name) + qualified-table-name (sql.u/quote-name driver :table table-name) + schema-name (sql.u/quote-name driver :schema (session-schema driver))] + (format "CREATE MATERIALIZED VIEW %s.%s AS select * from %s.%s;" + schema-name + qualified-view-name + schema-name + qualified-table-name))) + +(defmulti drop-materialized-view-sql + "Return a `DROP MATERIALIZED VIEW` statement. + `view-name` is the name of the new view." + {:arglists '([driver database view-name])} + tx/dispatch-on-driver-with-test-extensions + :hierarchy #'driver/hierarchy) + +(defmethod drop-materialized-view-sql :sql/test-extensions + [driver _database view-name] + (let [qualified-view-name (sql.u/quote-name driver :table view-name) + schema-name (sql.u/quote-name driver :schema (session-schema driver))] + (format "DROP MATERIALIZED VIEW %s.%s;" + schema-name + qualified-view-name))) + (defmethod tx/native-query-with-card-template-tag :sql [_driver card-template-tag-name] (let [source-table-name (u/lower-case-en (u.random/random-name))]