Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Database syncing settings #48

Merged
merged 10 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/dockerhub.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
uses: actions/checkout@v3
with:
repository: metabase/metabase
ref: v0.47.1
ref: v0.49.12

- name: Checkout Driver Repo
uses: actions/checkout@v3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
uses: actions/checkout@v3
with:
repository: metabase/metabase
ref: v0.47.1
ref: v0.49.12

- name: Checkout Driver Repo
uses: actions/checkout@v3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
uses: actions/checkout@v3
with:
repository: metabase/metabase
ref: v0.47.1
ref: v0.49.12

- name: Checkout Driver Repo
uses: actions/checkout@v3
Expand Down
12 changes: 11 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
checkout v0.47.1
git checkout v0.49.12
git clone https://github.com/MaterializeInc/metabase-materialize-driver.git modules/drivers/materialize
```

Expand Down Expand Up @@ -80,6 +80,16 @@ mz_deps=$(cat modules/drivers/materialize/.github/deps.edn | sed -e "s|PWD|$PWD|
DRIVERS=materialize clojure -Sdeps ${mz_deps} -X:dev:drivers:drivers-dev:test:user/materialize :only metabase.query-processor.middleware.parameters.mbql-test
```

## Excluding tests

Some tests are not applicable to Materialize, and you can exclude them by adding the following to the test command:

```bash
git apply modules/drivers/materialize/scripts/exclude_tests.diff
```

The diff file contains the list of tests that are excluded from the Materialize driver. This often needs to be updated as new tests are added to new Metabase versions.

## Building a jar

You need to add an entry for Materialize in `modules/drivers/deps.edn`
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Next, use the following information to connect:
| Host | Materialize host name. |
| Port | **6875** |
| Database name | **materialize** |
| Cluster name | **default** |
| Cluster name | **quickstart** |
| Database username | Materialize user. |
| Database password | App-specific password. |
| SSL | **Enabled** |
Expand All @@ -59,6 +59,7 @@ Metabase Release | Driver Version
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

## Contributing

Expand Down
2 changes: 1 addition & 1 deletion bin/build_docker_image.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ usage() {
echo
echo "Example:"
echo
echo "$0 v0.47.1 /some/path/to/materialize.metabase-driver.jar my-metabase-with-materialize:v0.0.1"
echo "$0 v0.49.12 /some/path/to/materialize.metabase-driver.jar my-metabase-with-materialize:v0.0.1"
exit 1
}

Expand Down
3 changes: 2 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ services:
- PGPASSWORD=materialize

metabase:
image: metabase/metabase:v0.47.1
image: metabase/metabase:v0.49.12
container_name: metabase-with-materialize-driver
environment:
'MB_HTTP_TIMEOUT': '5000'
'JAVA_OPTS': '-Xms2g -Xmx4g -XX:+UseParallelGC'
ports:
- '3000:3000'
volumes:
Expand Down
6 changes: 4 additions & 2 deletions resources/metabase-plugin.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Reference: https://github.com/metabase/metabase/wiki/Metabase-Plugin-Manifest-Reference
info:
name: Metabase Materialize Driver
version: 1.0.3
version: 1.1.0
description: Allows Metabase to connect to Materialize.
contact-info:
name: Materialize Inc.
Expand All @@ -25,7 +25,7 @@ driver:
placeholder: materialize
- name: cluster
display-name: Cluster name
placeholder: default
placeholder: quickstart
required: true
helper-text: Your Materialize cluster name.
- user
Expand All @@ -36,6 +36,8 @@ driver:
- name: schema-filters
type: schema-filters
display-name: Schemas
- advanced-options-start
- default-advanced-options
Comment on lines +39 to +40
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omg, that ended up being so simple in the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha yea, but it was also a good reminder to get some other pending things updated as well!

init:
- step: load-namespace
namespace: metabase.driver.materialize
Expand Down
85 changes: 72 additions & 13 deletions scripts/exclude_tests.diff
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ diff --git a/test/metabase/query_processor_test/explicit_joins_test.clj b/test/m
index 166598c4c6..e1bef679f1 100644
--- a/test/metabase/query_processor_test/explicit_joins_test.clj
+++ b/test/metabase/query_processor_test/explicit_joins_test.clj
@@ -262,8 +262,8 @@
@@ -267,8 +267,8 @@

(deftest select-*-source-query-test
(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)
Expand All @@ -13,39 +13,98 @@ index 166598c4c6..e1bef679f1 100644
(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
@@ -910,6 +910,7 @@

(deftest join-with-brakout-and-aggregation-expression
@@ -986,6 +986,7 @@
(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 sample-dataset
(mt/dataset test-data
(let [query (mt/mbql-query orders
{:source-query {:source-table $$orders
@@ -928,4 +929,4 @@
@@ -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
@@ -177,7 +177,7 @@
@@ -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/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
+ (and (qp.test/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
- (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]
@@ -232,7 +232,7 @@
@@ -237,7 +237,7 @@
["2015-06-02 08:20:00" 1]
["2015-06-02 11:11:00" 1]]

- (and (qp.test/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle))
+ (and (qp.test/tz-shifted-driver-bug? driver/*driver*) (not= driver/*driver* :oracle) (not= driver/*driver* :materialize))
- (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/driver_test.clj b/test/metabase/driver_test.clj
index bb3b1bea6a..20e0ab83b4 100644
--- a/test/metabase/driver_test.clj
+++ b/test/metabase/driver_test.clj
@@ -107,7 +107,7 @@
(do
(tx/destroy-db! driver/*driver* dbdef)
details))]
- (is (false? (try
+ (is (true? (try
(binding [h2/*allow-testing-h2-connections* true]
(driver/can-connect? driver/*driver* details))
(catch Exception _
@@ -148,7 +148,7 @@
;; so fake it by changing the database details
(let [details (:details (mt/db))
new-details (case driver/*driver*
- (:redshift :snowflake :vertica) (assoc details :db (mt/random-name))
+ (:redshift :snowflake :vertica :materialize) (assoc details :db (mt/random-name))
: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 @@
(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"
- (is (true? (cant-sync-logged?))))
+ (is (false? (cant-sync-logged?))))
(testing "2: triggering the sync via the POST /api/database/:id/sync_schema endpoint should fail"
- (mt/user-http-request :crowberto :post 422 (str "/database/" (u/the-id db) "/sync_schema"))))
+ (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/test/data/dataset_definition_test.clj b/test/metabase/test/data/dataset_definition_test.clj
index 1de46014c9..d44c94cf0a 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
4 changes: 3 additions & 1 deletion src/metabase/driver/materialize.clj
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
:percentile-aggregations false
;; Disabling the support for the `:connection-impersonation` feature as it's not supported
:connection-impersonation false
;; Disable uploads
:uploads false
:test/jvm-timezone-setting false}]
(defmethod driver/database-supports? [:materialize feature] [_driver _feature _db] supported?))

Expand All @@ -60,7 +62,7 @@
; ;;; +----------------------------------------------------------------------------------------------------------------+

(def ^:private default-materialize-connection-details
{:host "materialize", :port 6875, :db "materialize", :cluster "default"})
{:host "materialize", :port 6875, :db "materialize", :cluster "quickstart"})

(defn- validate-connection-details
[{:keys [host]}]
Expand Down
6 changes: 2 additions & 4 deletions test/metabase/driver/materialize_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,15 @@
(:require
[clojure.test :refer :all]
#_{:clj-kondo/ignore [:discouraged-namespace]}
[honeysql.format :as hformat]
[honey.sql :as sql]
[metabase.driver :as driver]
[metabase.driver.materialize :as materialize]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.public-settings.premium-features :as premium-features]
[metabase.query-processor :as qp]
[metabase.test.fixtures :as fixtures]
[metabase.test :as mt]
#_{:clj-kondo/ignore [:discouraged-namespace]}
[metabase.util.honeysql-extensions :as hx]))
[metabase.test :as mt]))

(set! *warn-on-reflection* true)

Expand Down
4 changes: 2 additions & 2 deletions test/metabase/test/data/materialize.clj
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
{:host (tx/db-test-env-var-or-throw :materialize :host "localhost")
:ssl (tx/db-test-env-var :materialize :ssl false)
:port (tx/db-test-env-var-or-throw :materialize :port 6877)
:cluster (tx/db-test-env-var :materialize :cluster "default")
:cluster (tx/db-test-env-var :materialize :cluster "quickstart")
:user (tx/db-test-env-var-or-throw :materialize :user "mz_system")}
(when-let [password (tx/db-test-env-var :materialize :password)]
{:password password})
Expand Down Expand Up @@ -84,7 +84,7 @@
(str/replace sql #", PRIMARY KEY \([^)]+\)" "")))

(defmethod load-data/load-data! :materialize [& args]
(apply load-data/load-data-add-ids-chunked! args))
(apply load-data/load-data-maybe-add-ids-chunked! args))

(defmethod tx/sorts-nil-first? :materialize
[_driver _base-type]
Expand Down
Loading