Merge release/6.1.0 into master by AbhishekGedela · Pull Request #3 · curefit/metabase-driver

AbhishekGedela · 2026-04-13T17:35:34Z

This PR brings the Starburst release/6.1.0 branch back onto master.

Why this matters:

the release/6.1.0 line matches the good Starburst jar we saw in metabase build 89
it includes the newer execution-path behavior such as statement cancel support and the Starburst connection-pool timeout override
later packaged jars were built from an older driver line, so this PR helps align master with the correct release branch

Main driver behaviors restored on this line:

Starburst Metabase 6.1.0 source/version
explicit prepare / impersonation support
statement cancel support
unreturnedConnectionTimeout pool override

Metabase team gave us a patch branch that disables a failing test We need to use this branch until the fix makes it to a new release

Upgrade to Metabase 1.46.4

Implement impersonation

Fix datetime filter issue with Metabase 0.47

Remote deprecated method

Add query remarks

Support JDBC explicitPrepare flag

Switch to Metabase v1.48.3

Upgrade to Metabase 1.50

Upgrade to Metabase 1.52

Prepare release 5.0.0

Prepare release 6.0.0

Overload connection timeout property to avoid long query timeouts

Prepare release 6.1.0

Copilot

Pull request overview

Merges the release/6.1.0 Starburst driver line back into master, updating the driver implementation and local/CI tooling to match the intended Metabase/Trino versions and restored behaviors (prepared statements, cancel support, pool timeout override, impersonation).

Changes:

Bump driver + dependency versions (Metabase/Trino) and update plugin metadata/changelog.
Refactor Starburst driver internals (HoneySQL 2 migration, execution/sync/connectivity behavior changes, impersonation + “optimized prepared statements” support).
Update test infrastructure (Trino docker catalog setup, dataset loading behavior, CI + Makefile workflows, additional driver tests).

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
`test_test.clj`	Adds custom clj-kondo hook tests used during `make test`.
`resources/docker/trino/entrypoint.sh`	Simplifies Trino catalog setup to a single `test_data` memory catalog.
`package.json`	Introduces rspack-related JS dependencies at repo root.
`drivers/starburst/test/metabase/test/data/starburst.clj`	Updates Starburst test extensions, dataset naming, and data-loading strategy.
`drivers/starburst/test/metabase/driver/starburst_test.clj`	Updates Starburst driver tests for new dataset naming and adds impersonation/prepared-statement tests.
`drivers/starburst/src/metabase/driver/starburst.clj`	Expands driver entrypoint behaviors (remarks + connect handling) and loads implementation modules.
`drivers/starburst/src/metabase/driver/implementation/sync.clj`	Refactors sync/describe implementations and privilege probing.
`drivers/starburst/src/metabase/driver/implementation/query_processor.clj`	Migrates Starburst query processor behavior to HoneySQL 2 and updates temporal/datetime-diff logic.
`drivers/starburst/src/metabase/driver/implementation/messages.clj`	Adds Starburst-specific user-facing message constants.
`drivers/starburst/src/metabase/driver/implementation/execute.clj`	Refactors connection handling, adds impersonation hooks, prepared statement proxying, and pool override.
`drivers/starburst/src/metabase/driver/implementation/driver_helpers.clj`	Updates feature support declarations via `driver/database-supports?`.
`drivers/starburst/src/metabase/driver/implementation/connectivity.clj`	Updates JDBC spec generation for driver version, impersonation, and explicit prepare behavior.
`drivers/starburst/resources/metabase-plugin.yaml`	Updates plugin version and exposes new connection options in UI metadata.
`drivers/starburst/deps.edn`	Bumps `io.trino/trino-jdbc` dependency version.
`app_versions.json`	Updates pinned versions (Trino/Clojure/Metabase).
`README.md`	Updates local dev/testing notes (JDK range, optimized prepared statements testing).
`Makefile`	Updates build/test flow, Metabase clone behavior, adds optimized test target, and tweaks FE build steps.
`CHANGELOG.md`	Adds 6.0.0/6.1.0 release notes and updates compare links.
`.gitignore`	Adjusts metabase repo ignore pattern to be repo-root anchored.
`.github/workflows/ci.yml`	Extends CI timeout and adds Metabase initialization step.
`.github/actions/init-dependencies/action.yml`	Bumps Node.js setup default from 16 to 18.

Comments suppressed due to low confidence (1)

drivers/starburst/src/metabase/driver/implementation/connectivity.clj:23

clojure.tools.logging is required but not used in this namespace. Consider removing the require to avoid lint noise.

  (:require [clojure.set :as set]
            [clojure.string :as str]
            [clojure.tools.logging :as log]
            [metabase.api.common :as api]
            [metabase.db.spec :as mdb.spec]
            [metabase.driver.sql-jdbc.common :as sql-jdbc.common]
            [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
            [metabase.util.i18n :refer [trs]]))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T17:42:54Z

      - name: Initialize dependencies
        uses: ./.github/actions/init-dependencies
        id: init-dependencies
        with:
          java-version: ${{ matrix.runners.java_version }}
          java-architecture: ${{ matrix.runners.architecture }}
          clojure-version: ${{ env.CLOJURE_VERSION }}


The workflow references matrix.runners.*, but this job does not define a strategy.matrix. This will resolve to empty/undefined values and break setup-java/dependency initialization. Define a matrix or replace these inputs with concrete values or env vars.

Copilot · 2026-04-13T17:42:54Z

+(defn handle-execution-error
+  [e details]
+  (let [message (.getMessage e)
+        execute-immediate (get details :prepared-optimized false)]
+    (cond
+      (and (clojure.string/includes? message "Expecting: 'USING'") execute-immediate)
+      (throw (Exception. msg/STARBURST_INCOMPATIBLE_WITH_OPTIMIZED_PREPARED))
+      :else (throw e))))


handle-execution-error calls clojure.string/includes?, but this namespace isn't required in metabase.driver.starburst. This will fail to compile with No such namespace: clojure.string. Add [clojure.string :as str] to the ns requires (and update calls accordingly) or otherwise ensure the namespace is loaded.

Copilot · 2026-04-13T17:42:54Z

+(defn impersonate-user
+  [conn]
+  (if
+    (clojure.string/includes? (.getProperty (.getClientInfo conn) "ClientInfo" "") "impersonate:true")
+    (let [email (get (deref api/*current-user*) :email)]
+      (.setSessionUser (.unwrap conn TrinoConnection) email))
+    nil))
+
+(defn remove-impersonation
+  [conn]
+  (if
+    (clojure.string/includes? (.getProperty (.getClientInfo conn) "ClientInfo" "") "impersonate:true")
+    (.clearSessionUser (.unwrap conn TrinoConnection))
+    nil))


The impersonation detection checks (-> conn .getClientInfo (.getProperty "ClientInfo" ...)), but the connection property being set in connection-details->spec is :clientInfo (lowercase) and JDBC client-info keys are case-sensitive. As written, this is likely to never match and impersonation won't be applied/cleared. Use the correct key (and ideally access it consistently via getClientInfo(String) or the TrinoConnection API).

Copilot · 2026-04-13T17:42:55Z

+(ns metabase.driver.implementation.messages "Starburst messages")
+
+(def STARBURST_INCOMPATIBLE_WITH_OPTIMIZED_PREPARED "\"Optimized prepared statements\" require Starburst Galaxy, Starburst Enterprise (version 420-e or higher), or Trino (version 418 or higher)")
+(def STARBURST_MAYBE_INCOMPATIBLE ". If the database has the \"Optimized prepared statements\" option on, it require Starburst Galaxy, Starburst Enterprise (version 420-e or higher), or Trino (version 418 or higher)")


Grammar: the message says "it require"; this should be "it requires" for readability.

Suggested change

(def STARBURST_MAYBE_INCOMPATIBLE ". If the database has the \"Optimized prepared statements\" option on, it require Starburst Galaxy, Starburst Enterprise (version 420-e or higher), or Trino (version 418 or higher)")

(def STARBURST_MAYBE_INCOMPATIBLE ". If the database has the \"Optimized prepared statements\" option on, it requires Starburst Galaxy, Starburst Enterprise (version 420-e or higher), or Trino (version 418 or higher)")

Copilot · 2026-04-13T17:42:55Z

 (defmethod load-data/do-insert! :starburst
-  [driver spec table-identifier row-or-rows]
-  (let [statements (ddl/insert-rows-ddl-statements driver table-identifier row-or-rows)]
-    (with-open [conn (jdbc-spec->connection spec)]
-      (doseq [[^String sql & params] statements]
-        (try
-          (with-open [^PreparedStatement stmt (.prepareStatement conn sql)]
-            (sql-jdbc.execute/set-parameters! driver stmt params)
-            (let [tbl-nm        ((comp last :components) (into {} table-identifier))
-                  rows-affected (.executeUpdate stmt)]
-              (println (format "[%s] Inserted %d rows into starburst table %s." driver rows-affected tbl-nm))))
-          (catch Throwable e
-            (throw (ex-info (format "[%s] Error executing SQL: %s" driver (ex-message e))
-                            {:driver driver, :sql sql, :params params}
-                            e))))))))
+  [driver ^Connection conn table-identifier rows]
+  (dorun
+   (pmap
+    (fn [rows]
+      (let [statements (ddl/insert-rows-dml-statements driver table-identifier rows)]
+        (doseq [[^String sql & params] statements]
+          (assert (empty? params))
+          (try
+            (with-open [stmt (.createStatement conn)]
+              (let [[_tag _identifier-type components] table-identifier
+                    table-name                         (last components)]
+                (.execute stmt sql)
+                (log/infof "[%s] Inserted %d rows into %s." driver (count rows) table-name)))
+            (catch Throwable e
+              (throw (ex-info (format "[%s] Error executing SQL: %s" driver (ex-message e))
+                              {:driver driver, :sql sql}
+                              e)))))))
+    (partition-all 500 rows))))


load-data/do-insert! uses pmap but shares a single java.sql.Connection (conn) across parallel threads. JDBC connections/statements are generally not thread-safe, so this can cause intermittent failures or corrupted behavior. Either avoid parallelism here or create/borrow a separate connection per parallel task.

Copilot · 2026-04-13T17:42:57Z

+                  (assoc :source "Starburst Metabase 6.1.0")
+                  (assoc-if :clientInfo "impersonate:true" (:impersonation details-map))
+                  (assoc-if :explicitPrepare "false" (:prepared-optimized details-map))
+                  (dissoc (if (remove-role? details-map) :roles :test))


(dissoc (if (remove-role? details-map) :roles :test)) is hard to read and uses :test as a placeholder key to remove when no role should be removed. This obscures intent; use a conditional dissoc (or cond->) to remove :roles only when needed.

Suggested change

(dissoc (if (remove-role? details-map) :roles :test))

(cond-> (remove-role? details-map)

(dissoc :roles))

Copilot · 2026-04-13T17:42:57Z

+          cd metabase |
+          yarn build-static-viz |


The Initialize Metabase step uses | at the end of each line, which creates pipes/subshells and prevents cd metabase from affecting subsequent commands. Replace the pipes with proper line breaks and/or && so yarn build-static-viz runs inside the metabase directory.

Suggested change

cd metabase |

yarn build-static-viz |

cd metabase

yarn build-static-viz

Copilot · 2026-04-13T17:42:57Z

+            ;; set session time zone if defined
+            (.setTimeZoneId underlying-conn (get options :session-timezone))))
+        (try
+          (.setReadOnly conn true)


do-with-connection-with-options unconditionally sets the connection to read-only. This ignores the :write? option and can break code paths that legitimately need a writable connection (e.g., DDL/data loading during tests or sync). Set read-only based on :write? (e.g., only when write? is false).

Suggested change

(.setReadOnly conn true)

(.setReadOnly conn (not write?))

Copilot · 2026-04-13T17:42:57Z

+
+(defmethod ddl/insert-rows-dml-statements :starburst
+  [driver table-identifier rows]
+  (def %rows rows)


(def %rows rows) introduces a global var as a side effect each time this method runs. This looks like leftover debugging and can leak large data into memory / interfere with REPL/test runs. Remove it or replace with scoped logging if needed.

Suggested change

(def %rows rows)

Maiquu and others added 30 commits May 17, 2023 22:14

Updated tests to use same catalog

c5de49f

Migrated to Honey SQL 2

0afd85a

Updated datetime_diff implementation to use the new multimethod

bb6bef0

Bumped metabase and clojure versions

b7c58f2

Updated test script to use trino_port

4a32d4b

Updated front_end build script to work with bumped metabase version

f89e7ec

Enabled now support

e74ec16

Fixed failing tests

4e035ba

Updated .gitignore to only ignore root metabase directory

190bd50

Set metabase version to patch branch

25737d1

Metabase team gave us a patch branch that disables a failing test We need to use this branch until the fix makes it to a new release

Bump max-old-space-size to 4 Gb

b962c0c

Bump ci timeout

6463b5e

check out depth 1

2465d94

Update changelog

906ffef

Prepare release 3.0.0

736cef2

Upgrade to Metabase 1.46.4

2b9709f

Upgrade to Metabase 1.46.4

Update build docker script

ca41839

Implement impersonation

1e47533

Implement impersonation

Fix datetime filter issue with Metabase 0.47

4a88097

Fix datetime filter issue with Metabase 0.47

Prepare release 4.0.0

17c34c7

Update CHANGELOG

0be659c

Remote deprecated method

e168ff2

Remote deprecated method

Hardens the reading of ClientInfo

76d8596

Add query remarks

7e384ae

Add query remarks

Support JDBC explicitPrepare flag

b603b14

Support JDBC explicitPrepare flag

Prepare release 4.1.0

4350b5c

Switch to Metabase v1.48.3

0eb1f76

Switch to Metabase v1.48.3

Re-enable :foreign-keys

f4fe016

Upgrade to Metabase 1.50

3be06fb

Upgrade to Metabase 1.50

Upgrade to Metabase 1.52

a7bc1dd

Upgrade to Metabase 1.52

web-flow and others added 5 commits January 10, 2025 12:49

Prepare release 5.0.0

27498cf

Prepare release 5.0.0

Prepare release 6.0.0

22f229d

Prepare release 6.0.0

Overload connection timeout property to avoid long query timeouts

4bebff1

Overload connection timeout property to avoid long query timeouts

Add cancel support for statements

3c2bfdc

Prepare release 6.1.0

26af3c6

Prepare release 6.1.0

Copilot AI review requested due to automatic review settings April 13, 2026 17:35

Copilot started reviewing on behalf of AbhishekGedela April 13, 2026 17:36 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

AbhishekGedela requested a review from nileshsinha1993 April 13, 2026 17:49

nileshsinha1993 approved these changes Apr 13, 2026

View reviewed changes

Merge branch 'master' into release/6.1.0

4c6ae4d

AbhishekGedela merged commit 5402872 into master Apr 13, 2026
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge release/6.1.0 into master#3

Merge release/6.1.0 into master#3
AbhishekGedela merged 36 commits intomasterfrom
release/6.1.0

AbhishekGedela commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

	(def STARBURST_MAYBE_INCOMPATIBLE ". If the database has the \"Optimized prepared statements\" option on, it require Starburst Galaxy, Starburst Enterprise (version 420-e or higher), or Trino (version 418 or higher)")
	(def STARBURST_MAYBE_INCOMPATIBLE ". If the database has the \"Optimized prepared statements\" option on, it requires Starburst Galaxy, Starburst Enterprise (version 420-e or higher), or Trino (version 418 or higher)")

	(dissoc (if (remove-role? details-map) :roles :test))
	(cond-> (remove-role? details-map)
	(dissoc :roles))

Conversation

AbhishekGedela commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants