metabase / metabase

The simplest, fastest way to get business intelligence and analytics to everyone in your company :yum:
https://metabase.com
Other
37.24k stars 4.94k forks source link

Driver: CockroachDB #5050

Open benkoller opened 7 years ago

benkoller commented 7 years ago

Hi,

as CockroachDB just reached a state of "production ready" with their 1.0 release it would be great to see them being supported by Metabase. We're already evaluating it for a couple of projects in our backend and it like it so far, but as we're doing most of our analytics via Metabase we'd need support for CockroachDB before making any serious moves.

SQL-wise it shouldn't be a huge hassle AFAICT.

If there is something we / I can help with let me know, happy to do so.

Cheers, Ben

:arrow_down: Please click the :+1: reaction instead of leaving a +1 or update? comment

camsaul commented 7 years ago

Hi @benkoller, we're adding support for various DBs based on community demand. Since you're the only person who's asked for this DB thus far, that might be a while πŸ˜‰

If you're familiar with Clojure and feeling adventurous you can try writing a driver yourself. A few of our drivers (Presto, Vertica and Crate) were contributed by community members. I'm available to help with any questions you might run into.

Anyone else interested in this driver, please add a :+1: reaction to the issue description rather than adding +1 or πŸ‘ comments. One way we prioritize issues is by the number of πŸ‘ reactions.

wkrause13 commented 6 years ago

Just some additional information should anyone wish to use metabase with cockroachdb. I am able to execute SQL queries against a cockroachdb instance through metabase. However, metabase is not listing the tables in the UI, so the query builder does not work.

While the query builder is really nice, I'm comfortable writing my own SQL queries. The core dataviz and dashboarding features work with cockroachdb using the postgres driver as far as I can tell.

tobbbles commented 6 years ago

@wkrause13 Awesome. How does it handle Field Filters?

wkrause13 commented 6 years ago

@mnzt, it doesn't unfortunately. When creating a new question, you need to select "switch to SQL" and then write your own SQL statement that is compatible with CockroachDB. If you do that, then you can save the question, add it to a dashboard, etc...

Found the logs, and it appears to be failing on all tables with the following query:

:cause ERROR: syntax error at or near "."
SELECT NULL AS TABLE_CAT, n.nspname AS TABLE_SCHEM, ct.relname AS TABLE_NAME, a.attname AS COLUMN_NAME, (i.keys).n AS KEY_SEQ, ci.relname AS PK_NAME FROM pg_catalog.pg_class ct JOIN pg_catalog.pg_attribute a ON (ct.oid = a.attrelid) JOIN pg_catalog.pg_namespace n ON (ct.relnamespace = n.oid) JOIN (SELECT i.indexrelid, i.indrelid, i.indisprimary, information_schema._pg_expandarray(i.indkey) AS keys FROM pg_catalog.pg_index i) i ON (a.attnum = (i.keys).x AND a.attrelid = i.indrelid) JOIN pg_catalog.pg_class ci ON (ci.oid = i.indexrelid) WHERE true AND ct.relname = 'categories' AND i.indisprimary ORDER BY table_name, pk_name, key_seq

After a little digging it looks like the _pg_expandarray function is not implemented in cockroachdb and is the likely point of failure. Unless someone knows a workaround, I'd say keep an eye on this issue: https://github.com/cockroachdb/cockroach/issues/16971

If you really need the query builder, you could export a snapshot of your cockroachdb data and import it into an instance of postgres. You could then try porting over the SQL the query builder generates to apply to your cockroachdb. I'm not doing anything too complex at the moment, so haven't fully explored that path myself.

ans-4175 commented 5 years ago

Any updates on using cockroachDB on metabase?

christianhuening commented 5 years ago

Would be interested as well.

darsenault commented 5 years ago

+1 for CockroachDB support. The last couple of releases have greatly improved some of the areas of non-compatibility with PG like some of the catalog tables, etc. I tried Metabase with CRDB last week and got it to connect OK but it could not access tables in CRDB. I will did in and report back on what the logs say.

FrancisVarga commented 5 years ago

Is coming along?

Dipen-Dedania commented 5 years ago

+1 for CockroachDB support.

SinaZK commented 4 years ago

+1 for CockroachDB support.

mazameli commented 4 years ago

Please click the πŸ‘ reaction on the initial description of this issue to have your support for this feature recorded. "+1" comments are not taken into account and only create noise for people who are following this issue. Thanks!

mazameli commented 4 years ago

CockroachDB ranks only around 19th on our list of most-requested database drivers for Metabase, and it is unlikely we will get to this soon. However, anyone who is interested is welcome to try writing their own driver and including it as a plugin. See this guide for more information: https://github.com/metabase/metabase/wiki/Writing-A-Driver

Please see this Amazon Athena driver for a great example of a community-developed database driver.

54Ezreal commented 3 years ago

+1 for CockroachDB support.

paoliniluis commented 1 year ago

Just did some tests with cockroachdb and our sample dataset with the postgres driver. Hit this issue: https://github.com/cockroachdb/cockroach/issues/40195

andrewgy8 commented 1 year ago

For all those interested, I just was able to connect our CRDB (22.1.6) to metabase. At first we received the error

ERROR: unimplemented: multiple active portals not supported Detail: cannot perform operation sql.PrepareStmt while a different portal is open Hint: You have attempted to use a feature that is not yet implemented. See: https://go.crdb.dev/issue-v/40195/v22.1

However, after some trial and error, I realized that if I limited the results select * from my_table limit 100, I was able to retrieve results. After I did that query, I am now able to execute all select * from my_table without error.

Not sure what is happening with autocommit and how metabase is handling the connection to CRDB, but that seemed to solve it for us.

I was told this by CRDB support, even though we have caching on metabase disabled.

Generally the error occurs when a connection is reused whilst it still has a query/result in progress. Maybe with the cache it doesn’t go back to the database (and so reuse the connection) in every case. A low number of rows may also fit into the cache so there is no paging taking place. Paging through a result will leave the statement open and so leave the connection exposed to an active portal error should metabase try to reuse it for some other purpose.

supernomad commented 1 year ago

Alright I figured out how to solve your issue permanently @andrewgy8 for some reason this section of the sql_jdbc driver is the culprit:

https://github.com/metabase/metabase/blob/master/src/metabase/driver/sql_jdbc/execute.clj#L153-L171

Which sets the timezone for the current connection, by creating a wrapper plugin around the existing postgres driver and overriding just the connection-with-timezone call to not include the call to the above set timezone functionality Metabase is fully working with CRDB.

This is literally the entierty of the plugin:

(ns metabase.driver.crdb
  (:refer-clojure :exclude [second])
  (:require
   [metabase.driver :as driver]
   [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
   [clojure.tools.logging :as log]
   [metabase.util.i18n :refer [trs]])
  (:import
   (java.sql Connection ResultSet)))

(driver/register! :crdb, :parent :postgres)

(defmethod driver/display-name :crdb [_]
  "CockroachDB")

;; This impl is basically the same as the default impl in `sql-jdbc.execute`, but doesn't set the timezone.
(defmethod sql-jdbc.execute/connection-with-timezone :crdb
  [driver database ^String _timezone-id]
  (let [conn (.getConnection (sql-jdbc.execute/datasource-with-diagnostic-info! driver database))]
    (try
      (sql-jdbc.execute/set-best-transaction-level! driver conn)
      (try
        (.setHoldability conn ResultSet/CLOSE_CURSORS_AT_COMMIT)
        (catch Throwable e
          (log/debug e (trs "Error setting default holdability for connection"))))
      conn
      (catch Throwable e
        (.close ^Connection conn)
        (throw e)))))

This seems like it should not have fixed the problem but it does completely... Something about the set timezone call isn't closing the result set?? But my understanding of with-open (which is likely heavily flawed) is that it could close the resources after the statement completes? That however isn't happening so we set the timezone, the result isn't fully closed so the PG portal under the hood remains open, and then the next call to prepare the "real" query fails due to the multi-portal issue you linked to.

Note I have never written or looked at Clojure before so my understanding here is fuzzy, what I do know is that this plugin has fully solved the problem.

Newtoniano commented 1 year ago

Good find @supernomad , however after testing it myself I think the lines that are causing the issue are the ones that set autocommit to false .

In your code, you didn't just remove the timezone line, but also the ones below. Removing just the autocommit part fixes it for me:

(defmethod connection-with-timezone :sql-jdbc
  [driver database ^String timezone-id]
  (let [conn (.getConnection (datasource-with-diagnostic-info! driver database))]
    (try
      (set-best-transaction-level! driver conn)
      (set-time-zone-if-supported! driver conn timezone-id)
      (try
        ;; Setting the connection to read-only does not prevent writes on some databases, and is meant
        ;; to be a hint to the driver to enable database optimizations
        ;; See https://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#setReadOnly-boolean-
        (.setReadOnly conn true)
        (catch Throwable e
          (log/debug e (trs "Error setting connection to read-only"))))
      (try
        (.setHoldability conn ResultSet/CLOSE_CURSORS_AT_COMMIT)
        (catch Throwable e
          (log/debug e (trs "Error setting default holdability for connection"))))
      conn
      (catch Throwable e
        (.close conn)
        (throw e)))))

If @paoliniluis or anyone else from the team can take a look at this and push a very simple fix/plugin, we'd have compatibility with CockroachDB up and running with very little effort.

paoliniluis commented 1 year ago

This is not a trivial change, we might be fixing something for CRDB but breaking everything for all postgres-related DB's. My 2c here would be that someone should take the Redshift driver as a template and build a specific driver for CRDB that can be used in Metabase without breaking all others

jonknyc commented 1 year ago

Metabase was working for me + cockroach until I was upgraded to v1.45.3

Now I have found that aggregation queries still work, but any time I try to return actual non-aggregated rows & columns, it errors.

This is going to potentially force me to churn from metabase.

Please expedite supporting CockroachDB. Prisma added native cockroach DB support last year - if they can do it, you can do it!

paoliniluis commented 8 months ago

Just running on a native question SET multiple_active_portals_enabled=true will make the active portal problem go away for the duration of the session.

EDIT: only in cockroachdb > 23

paoliniluis commented 8 months ago

CockroachDB v22 does not work with Metabase v47+, but CockroachDB v23 does. If you're using Metabase 47+ please upgrade CRDB to the latest version