Upgrade to Metabase v50 (#50)

Author: bobbyiliev

.github/workflows/dockerhub.yaml

@@ -20,7 +20,7 @@ jobs:
         uses: actions/checkout@v3
         with:
           repository: metabase/metabase
-          ref: v0.49.12
+          ref: v0.50.10
 
       - name: Checkout Driver Repo
         uses: actions/checkout@v3

.github/workflows/release.yaml

@@ -14,7 +14,7 @@ jobs:
         uses: actions/checkout@v3
         with:
           repository: metabase/metabase
-          ref: v0.49.12
+          ref: v0.50.10
 
       - name: Checkout Driver Repo
         uses: actions/checkout@v3

.github/workflows/tests.yaml

@@ -18,7 +18,7 @@ jobs:
         uses: actions/checkout@v3
         with:
           repository: metabase/metabase
-          ref: v0.49.12
+          ref: v0.50.10
 
       - name: Checkout Driver Repo
         uses: actions/checkout@v3

CONTRIBUTING.md

@@ -24,7 +24,7 @@ The easiest way to set up a development environment is as follows (mostly the sa
 ```bash
 git clone https://github.com/metabase/metabase.git
 cd metabase
-git checkout v0.49.12
+git checkout v0.50.10
 git clone https://github.com/MaterializeInc/metabase-materialize-driver.git modules/drivers/materialize
 ```
 

README.md

@@ -60,6 +60,7 @@ v0.46.7          | v0.1.0
 v0.47.0          | v1.0.0
 v0.47.1          | v1.0.1 <br> v1.0.2 <br> v1.0.3
 v0.49.12         | v1.1.0
+v0.50.10         | v1.2.0
 
 ## Contributing
 

bin/build_docker_image.sh

@@ -23,7 +23,7 @@ usage() {
     echo
     echo "Example:"
     echo
-    echo "$0 v0.49.12 /some/path/to/materialize.metabase-driver.jar my-metabase-with-materialize:v0.0.1"
+    echo "$0 v0.50.10 /some/path/to/materialize.metabase-driver.jar my-metabase-with-materialize:v0.0.1"
     exit 1
 }
 

docker-compose.yml

@@ -22,7 +22,7 @@ services:
       }
 
   metabase:
-    image: metabase/metabase:v0.49.12
+    image: metabase/metabase:v0.50.10
     container_name: metabase-with-materialize-driver
     environment:
       'MB_HTTP_TIMEOUT': '5000'

resources/metabase-plugin.yaml

@@ -1,7 +1,7 @@
 # Reference: https://github.com/metabase/metabase/wiki/Metabase-Plugin-Manifest-Reference
 info:
   name: Metabase Materialize Driver
-  version: 1.1.0
+  version: 1.2.0
   description: Allows Metabase to connect to Materialize.
 contact-info:
   name: Materialize Inc.

scripts/exclude_tests.diff

@@ -1,63 +1,27 @@
-diff --git a/test/metabase/query_processor_test/explicit_joins_test.clj b/test/metabase/query_processor_test/explicit_joins_test.clj
-index 166598c4c6..e1bef679f1 100644
---- a/test/metabase/query_processor_test/explicit_joins_test.clj
-+++ b/test/metabase/query_processor_test/explicit_joins_test.clj
-@@ -267,8 +267,8 @@
-
- (deftest ^:parallel select-*-source-query-test
-   (mt/test-drivers (disj (mt/normal-drivers-with-feature :left-join)
--                         ;; mongodb doesn't support foreign keys required by this test
--                         :mongo)
-+                         ;; mongodb and materialize don't support foreign keys required by this test
-+                         :mongo :materialize)
-     (testing "We should be able to run a query that for whatever reason ends up with a `SELECT *` for the source query"
-       (let [{:keys [rows columns]} (mt/format-rows-by [int int]
-                                      (mt/rows+column-names
-@@ -986,6 +986,7 @@
+diff --git a/test/metabase/db/metadata_queries_test.clj b/test/metabase/db/metadata_queries_test.clj
+index 7373655654..25eb5da352 100644
+--- a/test/metabase/db/metadata_queries_test.clj
++++ b/test/metabase/db/metadata_queries_test.clj
+@@ -45,13 +45,7 @@
+                          (sort-by first)
+                          (take 5))]
+       (is (= :type/Text (-> fields first :base_type)))
+-      (is (= expected (fetch! nil)))
+-      (testing "truncates text fields (see #13288)"
+-        (doseq [size [1 4 80]]
+-          (is (= (mapv (fn [[s]] [(subs (or s "") 0 (min size (count s)))])
+-                       expected)
+-                 (fetch! size))
+-              "Did not truncate a text field")))))
++      (is (= expected (fetch! nil)))))
 
- (deftest ^:parallel join-with-brakout-and-aggregation-expression
-   (mt/test-drivers (mt/normal-drivers-with-feature :left-join)
-+    (when (not= driver/*driver* :materialize)
-     (mt/dataset test-data
-       (let [query (mt/mbql-query orders
-                                  {:source-query {:source-table $$orders
-@@ -1004,7 +1004,7 @@
-                   ["Doohickey" "Balistreri-Ankunding" "2018-02-01T00:00:00Z" 315.36 3.1536]
-                   ["Doohickey" "Balistreri-Ankunding" "2018-03-01T00:00:00Z" 315.36 3.1536]]
-                  (mt/formatted-rows [str str str 2.0 4.0]
--                   (qp/process-query query)))))))))
-+                   (qp/process-query query))))))))))
-
- (deftest ^:parallel mlv2-references-in-join-conditions-test
-   (testing "Make sure join conditions that contain MLv2-generated refs with extra info like `:base-type` work correctly (#33083)"
-diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
-index 87d225f5ae..343eb1f5f5 100644
---- a/test/metabase/query_processor_test/date_bucketing_test.clj
-+++ b/test/metabase/query_processor_test/date_bucketing_test.clj
-@@ -182,7 +182,7 @@
-
-                ;; There's a bug here where we are reading in the UTC time as pacific, so we're 7 hours off
-                ;; (This is fixed for Oracle now)
--               (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
-+               (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
-                [["2015-06-01T10:31:00-07:00" 1]
-                 ["2015-06-01T16:06:00-07:00" 1]
-                 ["2015-06-01T17:23:00-07:00" 1]
-@@ -237,7 +237,7 @@
-                 ["2015-06-02 08:20:00" 1]
-                 ["2015-06-02 11:11:00" 1]]
-
--               (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
-+               (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
-                [["2015-06-01T10:31:00-04:00" 1]
-                 ["2015-06-01T16:06:00-04:00" 1]
-                 ["2015-06-01T17:23:00-04:00" 1]
-
+   (testing "substring checking"
+     (with-redefs [driver.u/database->driver (constantly (:engine (mt/db)))
 diff --git a/test/metabase/driver_test.clj b/test/metabase/driver_test.clj
-index bb3b1bea6a..20e0ab83b4 100644
+index a506be0a66..cf358e20b2 100644
 --- a/test/metabase/driver_test.clj
 +++ b/test/metabase/driver_test.clj
-@@ -107,7 +107,7 @@
+@@ -106,7 +106,7 @@
                                (do
                                  (tx/destroy-db! driver/*driver* dbdef)
                                  details))]
@@ -66,7 +30,7 @@ index bb3b1bea6a..20e0ab83b4 100644
                                (binding [h2/*allow-testing-h2-connections* true]
                                  (driver/can-connect? driver/*driver* details))
                                (catch Exception _
-@@ -148,7 +148,7 @@
+@@ -144,7 +144,7 @@
                ;; so fake it by changing the database details
                (let [details     (:details (mt/db))
                      new-details (case driver/*driver*
@@ -75,7 +39,7 @@ index bb3b1bea6a..20e0ab83b4 100644
                                    :oracle                         (assoc details :service-name (mt/random-name))
                                    :presto-jdbc                    (assoc details :catalog (mt/random-name)))]
                  (t2/update! :model/Database (u/the-id db) {:details new-details}))
-@@ -156,9 +156,9 @@
+@@ -152,9 +152,9 @@
                (tx/destroy-db! driver/*driver* dbdef))
              (testing "after deleting a database, sync should fail"
                (testing "1: sync-and-analyze-database! should log a warning and fail early"
@@ -86,25 +50,98 @@ index bb3b1bea6a..20e0ab83b4 100644
 +                (mt/user-http-request :crowberto :post 200 (str "/database/" (u/the-id db) "/sync_schema"))))
              ;; clean up the database
              (t2/delete! :model/Database (u/the-id db))))))))
-
-
+ 
+diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
+index 6e469bb152..f5f817715b 100644
+--- a/test/metabase/query_processor_test/date_bucketing_test.clj
++++ b/test/metabase/query_processor_test/date_bucketing_test.clj
+@@ -184,7 +184,7 @@
+ 
+                ;; There's a bug here where we are reading in the UTC time as pacific, so we're 7 hours off
+                ;; (This is fixed for Oracle now)
+-               (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
++               (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
+                [["2015-06-01T10:31:00-07:00" 1]
+                 ["2015-06-01T16:06:00-07:00" 1]
+                 ["2015-06-01T17:23:00-07:00" 1]
+@@ -242,7 +242,7 @@
+                 ["2015-06-02 08:20:00" 1]
+                 ["2015-06-02 11:11:00" 1]]
+ 
+-               (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
++               (and (qp.test-util/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
+                [["2015-06-01T10:31:00-04:00" 1]
+                 ["2015-06-01T16:06:00-04:00" 1]
+                 ["2015-06-01T17:23:00-04:00" 1]
+diff --git a/test/metabase/query_processor_test/explicit_joins_test.clj b/test/metabase/query_processor_test/explicit_joins_test.clj
+index ded26c8e97..4608b25854 100644
+--- a/test/metabase/query_processor_test/explicit_joins_test.clj
++++ b/test/metabase/query_processor_test/explicit_joins_test.clj
+@@ -270,8 +270,8 @@
+ 
+ (deftest ^:parallel select-*-source-query-test
+   (mt/test-drivers (disj (mt/normal-drivers-with-feature :left-join)
+-                         ;; mongodb doesn't support foreign keys required by this test
+-                         :mongo)
++                         ;; mongodb and materialize don't support foreign keys required by this test
++                         :mongo :materialize)
+     (testing "We should be able to run a query that for whatever reason ends up with a `SELECT *` for the source query"
+       (let [{:keys [rows columns]} (mt/format-rows-by [int int]
+                                      (mt/rows+column-names
 diff --git a/test/metabase/test/data/dataset_definition_test.clj b/test/metabase/test/data/dataset_definition_test.clj
-index 1de46014c9..d44c94cf0a 100644
+index 25ead15772..f830d1c2ff 100644
 --- a/test/metabase/test/data/dataset_definition_test.clj
 +++ b/test/metabase/test/data/dataset_definition_test.clj
-@@ -13,6 +13,7 @@
-                          ;; creating db for athena is expensive and require some extra steps,
-                          ;; so it's not worth testing against, see [[metabase.test.data.athena/*allow-database-creation*]]
-                          :athena
-+                         :materialize
-                          ;; there is no PK in sparksql
-                          :sparksql)
-     (mt/dataset (mt/dataset-definition "custom-pk"
-@@ -53,6 +54,7 @@
-                          ;; creating db for athena is expensive and require some extra steps,
-                          ;; so it's not worth testing against, see [[metabase.test.data.athena/*allow-database-creation*]]
-                          :athena
-+                         :materialize
-                          ;; there is no PK in sparksql
-                          :sparksql)
-     (mt/dataset composite-pk
+@@ -8,52 +8,8 @@
+    [metabase.timeseries-query-processor-test.util :as tqpt]
+    [toucan2.core :as t2]))
+ 
+-(deftest dataset-with-custom-pk-test
+-  (mt/test-drivers (->> (mt/normal-drivers-with-feature :metadata/key-constraints)
+-                        (filter (mt/sql-jdbc-drivers))
+-                        ;; Timeseries drivers currently support only testing with pre-loaded dataset
+-                        (remove (tqpt/timeseries-drivers)))
+-    (mt/dataset (mt/dataset-definition "custom-pk"
+-                  ["user"
+-                   [{:field-name "custom_id" :base-type :type/Integer :pk? true}]
+-                   [[1]]]
+-                  ["group"
+-                   [{:field-name "user_custom_id" :base-type :type/Integer :fk "user"}]
+-                   [[1]]])
+-      (let [user-fields  (t2/select [:model/Field :name :semantic_type :fk_target_field_id] :table_id (mt/id :user))
+-            group-fields (t2/select [:model/Field :name :semantic_type :fk_target_field_id] :table_id (mt/id :group))
+-            format-name  #(ddl.i/format-name driver/*driver* %)]
+-        (testing "user.custom_id is a PK"
+-          (is (= [{:name               (format-name "custom_id")
+-                   :fk_target_field_id nil
+-                   :semantic_type      :type/PK}]
+-                 user-fields)))
+-        (when (driver.u/supports? driver/*driver* :foreign-keys (mt/db))
+-          (testing "user_custom_id is a FK non user.custom_id"
+-            (is (= #{{:name               (format-name "user_custom_id")
+-                      :fk_target_field_id (mt/id :user :custom_id)
+-                      :semantic_type      :type/FK}
+-                     {:name               (format-name "id")
+-                      :fk_target_field_id nil
+-                      :semantic_type      :type/PK}}
+-                   (set group-fields)))))))))
+-
+ (mt/defdataset composite-pk
+   [["songs"
+     [{:field-name "artist_id", :base-type :type/Integer, :pk? true}
+      {:field-name "song_id",   :base-type :type/Integer, :pk? true}]
+     [[1 2]]]])
+-
+-(deftest dataset-with-custom-composite-pk-test
+-  (mt/test-drivers (->> (mt/normal-drivers-with-feature :metadata/key-constraints)
+-                        (filter (mt/sql-jdbc-drivers))
+-                        ;; Timeseries drivers currently support only testing with pre-loaded dataset
+-                        (remove (tqpt/timeseries-drivers)))
+-    (mt/dataset composite-pk
+-      (let [format-name #(ddl.i/format-name driver/*driver* %)]
+-        (testing "(artist_id, song_id) is a PK"
+-          (is (= #{(format-name "artist_id")
+-                   (format-name "song_id")}
+-                 (t2/select-fn-set :name :model/Field
+-                                   :table_id (mt/id :songs)
+-                                   :semantic_type :type/PK))))))))

src/metabase/driver/materialize.clj

@@ -2,13 +2,16 @@
   "Metabase Materialize Driver."
   (:require [clojure
              [set :as set]]
+            [honey.sql :as sql]
+            [honey.sql.helpers :as sql.helpers]
             [metabase.db.spec :as db.spec]
             [metabase.config :as config]
             [metabase.driver :as driver]
             [metabase.util :as u]
             [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
             [metabase.driver.sql.query-processor :as sql.qp]
             [metabase.util.honey-sql-2 :as h2x]
+            [metabase.driver.sync :as driver.s]
             [metabase.driver.sql-jdbc
              [common :as sql-jdbc.common]
              [connection :as sql-jdbc.conn]
@@ -88,3 +91,56 @@
   (sql-jdbc.sync/describe-table driver database table))
 
 (defmethod sql-jdbc.sync/excluded-schemas :materialize [_driver] #{"mz_catalog" "mz_internal" "pg_catalog"})
+
+(defn ^:private get-tables-sql
+  "Materialize doesn't support the pg_stat_user_tables table
+   Overriding the default implementation to exclude the pg_stat_user_tables table"
+  [schemas table-names]
+  (sql/format
+   (cond->  {:select    [[:n.nspname :schema]
+                         [:c.relname :name]
+                         [[:case-expr :c.relkind
+                           [:inline "r"] [:inline "TABLE"]
+                           [:inline "p"] [:inline "PARTITIONED TABLE"]
+                           [:inline "v"] [:inline "VIEW"]
+                           [:inline "f"] [:inline "FOREIGN TABLE"]
+                           [:inline "m"] [:inline "MATERIALIZED VIEW"]
+                           :else nil]
+                          :type]
+                         [:d.description :description]]
+             :from      [[:pg_catalog.pg_class :c]]
+             :join      [[:pg_catalog.pg_namespace :n]   [:= :c.relnamespace :n.oid]]
+             :left-join [[:pg_catalog.pg_description :d] [:and [:= :c.oid :d.objoid] 
+                                                               [:= :d.objsubid 0] 
+                                                               [:= :d.classoid [:raw "'pg_class'::regclass"]]]]
+             :where     [:and [:= :c.relnamespace :n.oid]
+                         ;; filter out system tables (pg_ and mz_)
+                         [:and
+                          [(keyword "!~") :n.nspname "^pg_"]
+                          [(keyword "!~") :n.nspname "^mz_"]
+                          [:<> :n.nspname "information_schema"]]
+                         ;; only get tables of type: TABLE, PARTITIONED TABLE, VIEW, FOREIGN TABLE, MATERIALIZED VIEW
+                         [:raw "c.relkind in ('r', 'p', 'v', 'f', 'm')"]]
+             :order-by  [:type :schema :name]}
+     (seq schemas)
+     (sql.helpers/where [:in :n.nspname schemas])
+
+     (seq table-names)
+     (sql.helpers/where [:in :c.relname table-names]))
+   {:dialect :ansi}))
+
+(defn- describe-database-tables
+  [database]
+  (let [[inclusion-patterns
+         exclusion-patterns] (driver.s/db-details->schema-filter-patterns database)
+        syncable? (fn [schema]
+                    (driver.s/include-schema? inclusion-patterns exclusion-patterns schema))]
+    (eduction
+     (comp (filter (comp syncable? :schema))
+           (map #(dissoc % :type)))
+     (sql-jdbc.execute/reducible-query database (get-tables-sql nil nil)))))
+
+(defmethod driver/describe-database :materialize
+ [_driver database]
+  ;; TODO: change this to return a reducible so we don't have to hold 100k tables in memory in a set like this
+  {:tables (into #{} (describe-database-tables database))})