diff --git a/.circleci/config.yml b/.circleci/config.yml
index dba1264c59e45de4112aeb5c676ed29b60a7e69c..915d1bd9ab271672d83969180041cfce90b39052 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -134,7 +134,7 @@ executors:
- image: metabase/ci:lein-2.9.5-clojure-1.10.3.814
- image: circleci/mongo:4.0
- presto:
+ presto-186:
working_directory: /home/circleci/metabase/metabase/
docker:
- image: metabase/ci:lein-2.9.5-clojure-1.10.3.814
@@ -145,6 +145,25 @@ executors:
# OOM sometimes with the default medium size.
resource_class: large
+ presto-jdbc-env:
+ working_directory: /home/circleci/metabase/metabase/
+ docker:
+ - image: metabase/ci:lein-2.9.5-clojure-1.10.3.814
+ - image: metabase/presto-mb-ci:latest # version 0.254
+ environment:
+ JAVA_TOOL_OPTIONS: "-Xmx2g"
+ MB_PRESTO_JDBC_TEST_CATALOG: test_data
+ MB_PRESTO_JDBC_TEST_HOST: localhost
+ MB_PRESTO_JDBC_TEST_PORT: 8443
+ MB_PRESTO_JDBC_TEST_SSL: true
+ MB_PRESTO_JDBC_TEST_USER: metabase
+ MB_PRESTO_JDBC_TEST_PASSWORD: metabase
+ MB_ENABLE_PRESTO_JDBC_DRIVER: true
+ MB_PRESTO_JDBC_TEST_ADDITIONAL_OPTIONS: >
+ SSLTrustStorePath=/tmp/cacerts-with-presto-ssl.jks&SSLTrustStorePassword=changeit
+ # (see above)
+ resource_class: large
+
sparksql:
working_directory: /home/circleci/metabase/metabase/
docker:
@@ -487,6 +506,15 @@ commands:
wget --output-document=plugins/<< parameters.dest >> ${<< parameters.source >>}
no_output_timeout: 15m
+ run-command:
+ parameters:
+ command:
+ type: string
+ steps:
+ - run:
+ name: Run command
+ command: << parameters.command >>
+
jobs:
########################################################################################################################
@@ -652,6 +680,9 @@ jobs:
before-steps:
type: steps
default: []
+ after-steps:
+ type: steps
+ default: []
description:
type: string
default: ""
@@ -675,6 +706,7 @@ jobs:
no_output_timeout: << parameters.timeout >>
- store_test_results:
path: /home/circleci/metabase/metabase/target/junit
+ - steps: << parameters.after-steps >>
test-build-scripts:
executor: clojure-and-node
@@ -793,7 +825,9 @@ jobs:
- /home/circleci/metabase/metabase/modules/drivers/googleanalytics/target
- /home/circleci/metabase/metabase/modules/drivers/mongo/target
- /home/circleci/metabase/metabase/modules/drivers/oracle/target
+ - /home/circleci/metabase/metabase/modules/drivers/presto-common/target
- /home/circleci/metabase/metabase/modules/drivers/presto/target
+ - /home/circleci/metabase/metabase/modules/drivers/presto-jdbc/target
- /home/circleci/metabase/metabase/modules/drivers/redshift/target
- /home/circleci/metabase/metabase/modules/drivers/snowflake/target
- /home/circleci/metabase/metabase/modules/drivers/sparksql/target
@@ -1093,12 +1127,48 @@ workflows:
name: be-tests-presto-ee
requires:
- be-tests-ee
- e: presto
+ e: presto-186
before-steps:
- wait-for-port:
port: 8080
driver: presto
+ - test-driver:
+ name: be-tests-presto-jdbc-ee
+ requires:
+ - be-tests-ee
+ e: presto-jdbc-env # specific env for running Presto JDBC tests (newer Presto version, SSL, etc.)
+ before-steps:
+ - wait-for-port:
+ port: 8443
+ - run:
+ name: Create temp cacerts file based on bundled JDK one
+ command: cp $JAVA_HOME/lib/security/cacerts /tmp/cacerts-with-presto-ssl.jks
+ - run:
+ name: Install openssl
+ command: apk add openssl
+ - run:
+ name: Capture Presto server self signed CA
+ command: |
+ while [[ ! -s /tmp/presto-ssl-ca.pem ]];
+ do echo "Waiting to capture SSL CA" \
+ && openssl s_client -connect localhost:8443 2>/dev/null </dev/null | sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' > /tmp/presto-ssl-ca.pem \
+ && sleep 1; done
+ - run:
+ name: Convert Presto CA from PEM to DER
+ command: openssl x509 -outform der -in /tmp/presto-ssl-ca.pem -out /tmp/presto-ssl-ca.der
+ - run:
+ name: Import Presto CA into temp cacerts file
+ command: |
+ keytool -noprompt -import -alias presto -keystore /tmp/cacerts-with-presto-ssl.jks \
+ -storepass changeit -file /tmp/presto-ssl-ca.der -trustcacerts
+ after-steps:
+ - run:
+ name: Capture max memory usage
+ command: cat /sys/fs/cgroup/memory/memory.max_usage_in_bytes
+ when: always
+ driver: presto-jdbc
+
- test-driver:
name: be-tests-redshift-ee
requires:
diff --git a/enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj b/enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj
index f3b223cc40fc3c3229b371c65d339c0b0c661107..088d893eee11c5d326fdc61048ac2c81c9598b12 100644
--- a/enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj
@@ -107,7 +107,7 @@
{:query (mt/native-query
{:query
(format-honeysql
- {:select [(identifier :venues :name)]
+ {:select [(identifier :venues :id) (identifier :venues :name)]
:from [(identifier :venues)]
:order-by [(identifier :venues :id)]})})})
@@ -271,10 +271,11 @@
(run-venues-count-query)))))
(testing "Make sure that you can still use a SQL-based GTAP without needing to have SQL read perms for the Database"
- (is (= [["Red Medicine"] ["Stout Burgers & Beers"]]
- (mt/rows
+ (is (= [[1 "Red Medicine"]
+ [2 "Stout Burgers & Beers"]]
+ (mt/formatted-rows [int str]
(mt/with-gtaps {:gtaps {:venues (venue-names-native-gtap-def)}}
- (mt/run-mbql-query venues {:limit 2}))))))
+ (mt/run-mbql-query venues {:limit 2, :order-by [[:asc [:field (mt/id :venues :id)]]]}))))))
(testing (str "When no card_id is included in the GTAP, should default to a query against the table, with the GTAP "
"criteria applied")
@@ -327,10 +328,12 @@
(defn- row-level-restrictions-fk-drivers
"Drivers to test row-level restrictions against foreign keys with. Includes BigQuery, which for whatever reason does
- not normally have FK tests ran for it."
+ not normally have FK tests ran for it. Excludes Presto JDBC, because that driver does NOT support fetching foreign
+ keys from the JDBC metadata, even though we enable the feature in the UI."
[]
(cond-> (mt/normal-drivers-with-feature :nested-queries :foreign-keys)
- (@tx.env/test-drivers :bigquery) (conj :bigquery)))
+ (@tx.env/test-drivers :bigquery) (conj :bigquery)
+ true (disj :presto-jdbc)))
(deftest e2e-fks-test
(mt/test-drivers (row-level-restrictions-fk-drivers)
@@ -914,8 +917,13 @@
(mt/rows (mt/run-mbql-query orders {:limit 1})))))))))))))
(deftest pivot-query-test
- ;; sample-dataset doesn't work on Redshift yet -- see #14784
- (mt/test-drivers (disj (mt/normal-drivers-with-feature :foreign-keys :nested-queries :left-join) :redshift)
+ (mt/test-drivers (disj
+ (mt/normal-drivers-with-feature :foreign-keys :nested-queries :left-join)
+ ;; sample-dataset doesn't work on Redshift yet -- see #14784
+ :redshift
+ ;; this test relies on a FK relation between $product_id->products.category, so skip for Presto
+ ;; JDBC, because that driver doesn't support resolving FKs from the JDBC metadata
+ :presto-jdbc)
(testing "Pivot table queries should work with sandboxed users (#14969)"
(mt/dataset sample-dataset
(mt/with-gtaps {:gtaps (mt/$ids
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
index fa14d3c385075ef3bd10e9353779f4e495fce104..8b7234ed735e504002668988f70d85996b2be95b 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/load_test.clj
@@ -265,7 +265,7 @@
;; in case it already exists
(u/ignore-exceptions
(delete-directory! dump-dir))
- (mt/test-drivers (-> (mt/normal-drivers-with-feature :basic-aggregations :binning :expressions)
+ (mt/test-drivers (-> (mt/normal-drivers-with-feature :basic-aggregations :binning :expressions :foreign-keys)
;; We will run this roundtrip test against any database supporting these features ^ except
;; certain ones for specific reasons, outlined below.
;;
diff --git a/modules/drivers/presto-common/project.clj b/modules/drivers/presto-common/project.clj
new file mode 100644
index 0000000000000000000000000000000000000000..6fd0ab15bb36b3cc832085e3bcf6576c5a454746
--- /dev/null
+++ b/modules/drivers/presto-common/project.clj
@@ -0,0 +1,24 @@
+(defproject metabase/presto-common-driver "1.0.0-SNAPSHOT"
+ :min-lein-version "2.5.0"
+
+ :description "Common code for all Presto drivers. Defines HoneySQL behavior, query generation, etc."
+
+ :aliases
+ {"install-for-building-drivers" ["with-profile" "+install-for-building-drivers" "install"]}
+
+ :profiles
+ {:provided
+ {:dependencies
+ [[org.clojure/clojure "1.10.1"]
+ [metabase-core "1.0.0-SNAPSHOT"]]}
+
+ :install-for-building-drivers
+ {:auto-clean true
+ :aot :all}
+
+ :uberjar
+ {:auto-clean true
+ :aot :all
+ :javac-options ["-target" "1.8", "-source" "1.8"]
+ :target-path "target/%s"
+ :uberjar-name "presto-common.metabase-driver.jar"}})
diff --git a/modules/drivers/presto-common/resources/metabase-plugin.yaml b/modules/drivers/presto-common/resources/metabase-plugin.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..9d6c8c96c48c5e70e6473dca4171646403880ae0
--- /dev/null
+++ b/modules/drivers/presto-common/resources/metabase-plugin.yaml
@@ -0,0 +1,11 @@
+info:
+ name: Presto Common Driver
+ version: 1.0.0-SNAPSHOT
+ description: Common code for all Presto drivers. Defines HoneySQL behavior, query generation, etc.
+driver:
+ name: presto-common
+ abstract: true
+ lazy-load: true
+init:
+ - step: load-namespace
+ namespace: metabase.driver.presto-common
diff --git a/modules/drivers/presto-common/src/metabase/driver/presto_common.clj b/modules/drivers/presto-common/src/metabase/driver/presto_common.clj
new file mode 100644
index 0000000000000000000000000000000000000000..498a424e3691770187a0bd2df7a601e021bb5152
--- /dev/null
+++ b/modules/drivers/presto-common/src/metabase/driver/presto_common.clj
@@ -0,0 +1,212 @@
+(ns metabase.driver.presto-common
+ "Abstract common driver for Presto. It only defines SQL generation logic and doesn't involve the transport/execution
+ mechanism for actually connecting to Presto."
+ (:require [buddy.core.codecs :as codecs]
+ [honeysql.core :as hsql]
+ [honeysql.format :as hformat]
+ [honeysql.helpers :as h]
+ [java-time :as t]
+ [metabase.driver :as driver]
+ [metabase.driver.common :as driver.common]
+ [metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
+ [metabase.driver.sql.query-processor :as sql.qp]
+ [metabase.driver.sql.util :as sql.u]
+ [metabase.driver.sql.util.unprepare :as unprepare]
+ [metabase.util :as u]
+ [metabase.util.date-2 :as u.date]
+ [metabase.util.honeysql-extensions :as hx])
+ (:import java.sql.Time
+ [java.time OffsetDateTime ZonedDateTime]))
+
+(driver/register! :presto-common, :abstract? true, :parent :sql)
+
+;;; Presto API helpers
+
+(def presto-type->base-type
+ "Function that returns a `base-type` for the given `presto-type` (can be a keyword or string)."
+ (sql-jdbc.sync/pattern-based-database-type->base-type
+ [[#"(?i)boolean" :type/Boolean]
+ [#"(?i)tinyint" :type/Integer]
+ [#"(?i)smallint" :type/Integer]
+ [#"(?i)integer" :type/Integer]
+ [#"(?i)bigint" :type/BigInteger]
+ [#"(?i)real" :type/Float]
+ [#"(?i)double" :type/Float]
+ [#"(?i)decimal.*" :type/Decimal]
+ [#"(?i)varchar.*" :type/Text]
+ [#"(?i)char.*" :type/Text]
+ [#"(?i)varbinary.*" :type/*]
+ [#"(?i)json" :type/Text] ; TODO - this should probably be Dictionary or something
+ [#"(?i)date" :type/Date]
+ [#"(?i)^timestamp$" :type/DateTime]
+ [#"(?i)^timestamp with time zone$" :type/DateTimeWithTZ]
+ [#"(?i)^time$" :type/Time]
+ [#"(?i)^time with time zone$" :type/TimeWithTZ]
+ #_[#"(?i)time.+" :type/DateTime] ; TODO - get rid of this one?
+ [#"(?i)array" :type/Array]
+ [#"(?i)map" :type/Dictionary]
+ [#"(?i)row.*" :type/*] ; TODO - again, but this time we supposedly have a schema
+ [#".*" :type/*]]))
+
+(defmethod sql.qp/add-interval-honeysql-form :presto-common
+ [_ hsql-form amount unit]
+ (hsql/call :date_add (hx/literal unit) amount hsql-form))
+
+(defn describe-catalog-sql
+ "The SHOW SCHEMAS statement that will list all schemas for the given `catalog`."
+ {:added "0.39.0"}
+ [driver catalog]
+ (str "SHOW SCHEMAS FROM " (sql.u/quote-name driver :database catalog)))
+
+(defn describe-schema-sql
+ "The SHOW TABLES statement that will list all tables for the given `catalog` and `schema`."
+ {:added "0.39.0"}
+ [driver catalog schema]
+ (str "SHOW TABLES FROM " (sql.u/quote-name driver :schema catalog schema)))
+
+(defn describe-table-sql
+ "The DESCRIBE statement that will list information about the given `table`, in the given `catalog` and schema`."
+ {:added "0.39.0"}
+ [driver catalog schema table]
+ (str "DESCRIBE " (sql.u/quote-name driver :table catalog schema table)))
+
+(def excluded-schemas
+ "The set of schemas that should be excluded when querying all schemas."
+ #{"information_schema"})
+
+(defmethod driver/db-start-of-week :presto-common
+ [_]
+ :monday)
+
+(defmethod sql.qp/cast-temporal-string [:presto-common :Coercion/YYYYMMDDHHMMSSString->Temporal]
+ [_ _coercion-strategy expr]
+ (hsql/call :date_parse expr (hx/literal "%Y%m%d%H%i%s")))
+
+(defmethod sql.qp/cast-temporal-byte [:presto-common :Coercion/YYYYMMDDHHMMSSBytes->Temporal]
+ [driver _coercion-strategy expr]
+ (sql.qp/cast-temporal-string driver :Coercion/YYYYMMDDHHMMSSString->Temporal
+ (hsql/call :from_utf8 expr)))
+
+(defmethod sql.qp/->honeysql [:presto-common Boolean]
+ [_ bool]
+ (hsql/raw (if bool "TRUE" "FALSE")))
+
+(defmethod sql.qp/->honeysql [:presto-common :time]
+ [_ [_ t]]
+ (hx/cast :time (u.date/format-sql (t/local-time t))))
+
+(defmethod sql.qp/->float :presto-common
+ [_ value]
+ (hx/cast :double value))
+
+(defmethod sql.qp/->honeysql [:presto-common :regex-match-first]
+ [driver [_ arg pattern]]
+ (hsql/call :regexp_extract (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern)))
+
+(defmethod sql.qp/->honeysql [:presto-common :median]
+ [driver [_ arg]]
+ (hsql/call :approx_percentile (sql.qp/->honeysql driver arg) 0.5))
+
+(defmethod sql.qp/->honeysql [:presto-common :percentile]
+ [driver [_ arg p]]
+ (hsql/call :approx_percentile (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver p)))
+
+;; Presto mod is a function like mod(x, y) rather than an operator like x mod y
+(defmethod hformat/fn-handler (u/qualified-name ::mod)
+ [_ x y]
+ (format "mod(%s, %s)" (hformat/to-sql x) (hformat/to-sql y)))
+
+(def ^:dynamic *param-splice-style*
+ "How we should splice params into SQL (i.e. 'unprepare' the SQL). Either `:friendly` (the default) or `:paranoid`.
+ `:friendly` makes a best-effort attempt to escape strings and generate SQL that is nice to look at, but should not
+ be considered safe against all SQL injection -- use this for 'convert to SQL' functionality. `:paranoid` hex-encodes
+ strings so SQL injection is impossible; this isn't nice to look at, so use this for actually running a query."
+ :friendly)
+
+(defmethod unprepare/unprepare-value [:presto-common String]
+ [_ ^String s]
+ (case *param-splice-style*
+ :friendly (str \' (sql.u/escape-sql s :ansi) \')
+ :paranoid (format "from_utf8(from_hex('%s'))" (codecs/bytes->hex (.getBytes s "UTF-8")))))
+
+;; See https://prestodb.io/docs/current/functions/datetime.html
+
+;; This is only needed for test purposes, because some of the sample data still uses legacy types
+(defmethod unprepare/unprepare-value [:presto-common Time]
+ [driver t]
+ (unprepare/unprepare-value driver (t/local-time t)))
+
+(defmethod unprepare/unprepare-value [:presto-common OffsetDateTime]
+ [_ t]
+ (format "timestamp '%s %s %s'" (t/local-date t) (t/local-time t) (t/zone-offset t)))
+
+(defmethod unprepare/unprepare-value [:presto-common ZonedDateTime]
+ [_ t]
+ (format "timestamp '%s %s %s'" (t/local-date t) (t/local-time t) (t/zone-id t)))
+
+;;; `:sql-driver` methods
+
+(defmethod sql.qp/apply-top-level-clause [:presto-common :page]
+ [_ _ honeysql-query {{:keys [items page]} :page}]
+ (let [offset (* (dec page) items)]
+ (if (zero? offset)
+ ;; if there's no offset we can simply use limit
+ (h/limit honeysql-query items)
+ ;; if we need to do an offset we have to do nesting to generate a row number and where on that
+ (let [over-clause (format "row_number() OVER (%s)"
+ (first (hsql/format (select-keys honeysql-query [:order-by])
+ :allow-dashed-names? true
+ :quoting :ansi)))]
+ (-> (apply h/select (map last (:select honeysql-query)))
+ (h/from (h/merge-select honeysql-query [(hsql/raw over-clause) :__rownum__]))
+ (h/where [:> :__rownum__ offset])
+ (h/limit items))))))
+
+(defmethod sql.qp/date [:presto-common :default] [_ _ expr] expr)
+(defmethod sql.qp/date [:presto-common :minute] [_ _ expr] (hsql/call :date_trunc (hx/literal :minute) expr))
+(defmethod sql.qp/date [:presto-common :minute-of-hour] [_ _ expr] (hsql/call :minute expr))
+(defmethod sql.qp/date [:presto-common :hour] [_ _ expr] (hsql/call :date_trunc (hx/literal :hour) expr))
+(defmethod sql.qp/date [:presto-common :hour-of-day] [_ _ expr] (hsql/call :hour expr))
+(defmethod sql.qp/date [:presto-common :day] [_ _ expr] (hsql/call :date_trunc (hx/literal :day) expr))
+(defmethod sql.qp/date [:presto-common :day-of-month] [_ _ expr] (hsql/call :day expr))
+(defmethod sql.qp/date [:presto-common :day-of-year] [_ _ expr] (hsql/call :day_of_year expr))
+
+(defmethod sql.qp/date [:presto-common :day-of-week]
+ [driver _ expr]
+ (sql.qp/adjust-day-of-week driver (hsql/call :day_of_week expr)))
+
+(defmethod sql.qp/date [:presto-common :week]
+ [driver _ expr]
+ (sql.qp/adjust-start-of-week driver (partial hsql/call :date_trunc (hx/literal :week)) expr))
+
+(defmethod sql.qp/date [:presto-common :month] [_ _ expr] (hsql/call :date_trunc (hx/literal :month) expr))
+(defmethod sql.qp/date [:presto-common :month-of-year] [_ _ expr] (hsql/call :month expr))
+(defmethod sql.qp/date [:presto-common :quarter] [_ _ expr] (hsql/call :date_trunc (hx/literal :quarter) expr))
+(defmethod sql.qp/date [:presto-common :quarter-of-year] [_ _ expr] (hsql/call :quarter expr))
+(defmethod sql.qp/date [:presto-common :year] [_ _ expr] (hsql/call :date_trunc (hx/literal :year) expr))
+
+(defmethod sql.qp/unix-timestamp->honeysql [:presto-common :seconds]
+ [_ _ expr]
+ (hsql/call :from_unixtime expr))
+
+(defmethod driver.common/current-db-time-date-formatters :presto-common
+ [_]
+ (driver.common/create-db-time-formatters "yyyy-MM-dd'T'HH:mm:ss.SSSZ"))
+
+(defmethod driver.common/current-db-time-native-query :presto-common
+ [_]
+ "select to_iso8601(current_timestamp)")
+
+(defmethod driver/current-db-time :presto-common
+ [& args]
+ (apply driver.common/current-db-time args))
+
+(doseq [[feature supported?] {:set-timezone true
+ :basic-aggregations true
+ :standard-deviation-aggregations true
+ :expressions true
+ :native-parameters true
+ :expression-aggregations true
+ :binning true
+ :foreign-keys true}]
+ (defmethod driver/supports? [:presto-common feature] [_ _] supported?))
diff --git a/modules/drivers/presto-common/test/metabase/test/data/presto_common.clj b/modules/drivers/presto-common/test/metabase/test/data/presto_common.clj
new file mode 100644
index 0000000000000000000000000000000000000000..182d3278046cef7b35bfdc68b8395663ee1a2673
--- /dev/null
+++ b/modules/drivers/presto-common/test/metabase/test/data/presto_common.clj
@@ -0,0 +1,15 @@
+(ns metabase.test.data.presto-common
+ "Common functionality for handling test datasets in Presto."
+ (:require [metabase.test.data.interface :as tx]))
+
+(defmethod tx/aggregate-column-info :presto-common
+ ([driver ag-type]
+ ((get-method tx/aggregate-column-info ::tx/test-extensions) driver ag-type))
+
+ ([driver ag-type field]
+ (merge
+ ((get-method tx/aggregate-column-info ::tx/test-extensions) driver ag-type field)
+ (when (= ag-type :sum)
+ {:base_type :type/BigInteger}))))
+
+(prefer-method tx/aggregate-column-info :presto-common ::tx/test-extensions)
diff --git a/modules/drivers/presto-jdbc/parents b/modules/drivers/presto-jdbc/parents
new file mode 100644
index 0000000000000000000000000000000000000000..1b4a8dec8e88c0a560332d2c956a0644eae955b0
--- /dev/null
+++ b/modules/drivers/presto-jdbc/parents
@@ -0,0 +1 @@
+presto-common
diff --git a/modules/drivers/presto-jdbc/project.clj b/modules/drivers/presto-jdbc/project.clj
new file mode 100644
index 0000000000000000000000000000000000000000..9bd5b358a66a1c307d403e927e91f108f67a1ef2
--- /dev/null
+++ b/modules/drivers/presto-jdbc/project.clj
@@ -0,0 +1,19 @@
+(defproject metabase/presto-jdbc-driver "1.0.0-0.254-SNAPSHOT"
+ :min-lein-version "2.5.0"
+
+ :dependencies
+ [[com.facebook.presto/presto-jdbc "0.254"]]
+
+ :profiles
+ {:provided
+ {:dependencies
+ [[org.clojure/clojure "1.10.1"]
+ [metabase-core "1.0.0-SNAPSHOT"]
+ [metabase/presto-common-driver "1.0.0-SNAPSHOT"]]}
+
+ :uberjar
+ {:auto-clean true
+ :aot :all
+ :javac-options ["-target" "1.8", "-source" "1.8"]
+ :target-path "target/%s"
+ :uberjar-name "presto-jdbc.metabase-driver.jar"}})
diff --git a/modules/drivers/presto-jdbc/resources/metabase-plugin.yaml b/modules/drivers/presto-jdbc/resources/metabase-plugin.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..973edfb617e3d041edf3188841f6b843fad644d4
--- /dev/null
+++ b/modules/drivers/presto-jdbc/resources/metabase-plugin.yaml
@@ -0,0 +1,37 @@
+info:
+ name: Metabase Presto JDBC Driver
+ version: 1.0.0-350-SNAPSHOT
+ description: Allows Metabase to connect to Presto databases using the Presto JDBC driver
+dependencies:
+ - plugin: Presto Common Driver
+driver:
+ name: presto-jdbc
+ display-name: Presto
+ lazy-load: true
+ parent: presto-common
+ connection-properties:
+ - host
+ - merge:
+ - port
+ - default: 8080
+ - merge:
+ - dbname
+ - name: catalog
+ placeholder: hive
+ required: false
+ - name: schema
+ required: false
+ - user
+ - password
+ - ssl
+ - merge:
+ - additional-options
+ - placeholder: "trustServerCertificate=false"
+init:
+ - step: load-namespace
+ namespace: metabase.driver.presto-common
+ - step: load-namespace
+ namespace: metabase.driver.presto-jdbc
+ - step: register-jdbc-driver
+ class: com.facebook.presto.jdbc.PrestoDriver
+
diff --git a/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj b/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj
new file mode 100644
index 0000000000000000000000000000000000000000..3a9cd285c197d49bc6c08c5bc5eb0f47b38ac94f
--- /dev/null
+++ b/modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj
@@ -0,0 +1,477 @@
+(ns metabase.driver.presto-jdbc
+ "Presto JDBC driver. See https://prestodb.io/docs/current/ for complete dox."
+ (:require [clojure.java.jdbc :as jdbc]
+ [clojure.set :as set]
+ [clojure.string :as str]
+ [clojure.tools.logging :as log]
+ [honeysql.core :as hsql]
+ [honeysql.format :as hformat]
+ [java-time :as t]
+ [metabase.db.spec :as db.spec]
+ [metabase.driver :as driver]
+ [metabase.driver.presto-common :as presto-common]
+ [metabase.driver.sql-jdbc.common :as sql-jdbc.common]
+ [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
+ [metabase.driver.sql-jdbc.execute.legacy-impl :as legacy]
+ [metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
+ [metabase.driver.sql-jdbc.sync.describe-database :as sql-jdbc.describe-database]
+ [metabase.driver.sql.parameters.substitution :as sql.params.substitution]
+ [metabase.driver.sql.query-processor :as sql.qp]
+ [metabase.query-processor.timezone :as qp.timezone]
+ [metabase.util :as u]
+ [metabase.util.date-2 :as u.date]
+ [metabase.util.honeysql-extensions :as hx]
+ [metabase.util.i18n :refer [trs]])
+ (:import com.facebook.presto.jdbc.PrestoConnection
+ com.mchange.v2.c3p0.C3P0ProxyConnection
+ [java.sql Connection PreparedStatement ResultSet ResultSetMetaData Time Types]
+ [java.time LocalDateTime LocalTime OffsetDateTime OffsetTime ZonedDateTime]
+ java.time.format.DateTimeFormatter
+ [java.time.temporal ChronoField Temporal]))
+
+(driver/register! :presto-jdbc, :parent #{:presto-common :sql-jdbc ::legacy/use-legacy-classes-for-read-and-set})
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Custom HoneySQL Clause Impls |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(def ^:private ^:const timestamp-with-time-zone-db-type "timestamp with time zone")
+
+(defmethod sql.qp/->honeysql [:presto-jdbc :log]
+ [driver [_ field]]
+ ;; recent Presto versions have a `log10` function (not `log`)
+ (hsql/call :log10 (sql.qp/->honeysql driver field)))
+
+(defmethod sql.qp/->honeysql [:presto-jdbc :count-where]
+ [driver [_ pred]]
+ ;; Presto will use the precision given here in the final expression, which chops off digits
+ ;; need to explicitly provide two digits after the decimal
+ (sql.qp/->honeysql driver [:sum-where 1.00M pred]))
+
+(defmethod sql.qp/->honeysql [:presto-jdbc :time]
+ [_ [_ t]]
+ ;; make time in UTC to avoid any interpretation by Presto in the connection (i.e. report) time zone
+ (hx/cast "time with time zone" (u.date/format-sql (t/offset-time (t/local-time t) 0))))
+
+(defmethod sql.qp/->honeysql [:presto-jdbc ZonedDateTime]
+ [_ ^ZonedDateTime t]
+ ;; use the Presto cast to `timestamp with time zone` operation to interpret in the correct TZ, regardless of
+ ;; connection zone
+ (hx/cast timestamp-with-time-zone-db-type (u.date/format-sql t)))
+
+(defmethod sql.qp/->honeysql [:presto-jdbc OffsetDateTime]
+ [_ ^OffsetDateTime t]
+ ;; use the Presto cast to `timestamp with time zone` operation to interpret in the correct TZ, regardless of
+ ;; connection zone
+ (hx/cast timestamp-with-time-zone-db-type (u.date/format-sql t)))
+
+(defrecord AtTimeZone
+ ;; record type to support applying Presto's `AT TIME ZONE` operator to an expression
+ [expr zone]
+ hformat/ToSql
+ (to-sql [_]
+ (format "%s AT TIME ZONE %s"
+ (hformat/to-sql expr)
+ (hformat/to-sql (hx/literal zone)))))
+
+(defn- in-report-zone
+ "Returns a HoneySQL form to interpret the `expr` (a temporal value) in the current report time zone, via Presto's
+ `AT TIME ZONE` operator. See https://prestodb.io/docs/current/functions/datetime.html"
+ [expr]
+ (let [report-zone (qp.timezone/report-timezone-id-if-supported :presto-jdbc)
+ ;; if the expression itself has type info, use that, or else use a parent expression's type info if defined
+ type-info (hx/type-info expr)
+ db-type (hx/type-info->db-type type-info)]
+ (if (and ;; AT TIME ZONE is only valid on these Presto types; if applied to something else (ex: `date`), then
+ ;; an error will be thrown by the query analyzer
+ (contains? #{"timestamp" "timestamp with time zone" "time" "time with time zone"} db-type)
+ ;; if one has already been set, don't do so again
+ (not (::in-report-zone? (meta expr)))
+ report-zone)
+ (-> (hx/with-database-type-info (->AtTimeZone expr report-zone) timestamp-with-time-zone-db-type)
+ (vary-meta assoc ::in-report-zone? true))
+ expr)))
+
+;; most date extraction and bucketing functions need to account for report timezone
+
+(defmethod sql.qp/date [:presto-jdbc :default]
+ [_ _ expr]
+ expr)
+
+(defmethod sql.qp/date [:presto-jdbc :minute]
+ [_ _ expr]
+ (hsql/call :date_trunc (hx/literal :minute) (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :minute-of-hour]
+ [_ _ expr]
+ (hsql/call :minute (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :hour]
+ [_ _ expr]
+ (hsql/call :date_trunc (hx/literal :hour) (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :hour-of-day]
+ [_ _ expr]
+ (hsql/call :hour (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :day]
+ [_ _ expr]
+ (hsql/call :date (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :day-of-week]
+ [_ _ expr]
+ (sql.qp/adjust-day-of-week :presto-jdbc (hsql/call :day_of_week (in-report-zone expr))))
+
+(defmethod sql.qp/date [:presto-jdbc :day-of-month]
+ [_ _ expr]
+ (hsql/call :day (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :day-of-year]
+ [_ _ expr]
+ (hsql/call :day_of_year (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :week]
+ [_ _ expr]
+ (sql.qp/adjust-start-of-week :presto-jdbc (partial hsql/call :date_trunc (hx/literal :week)) (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :month]
+ [_ _ expr]
+ (hsql/call :date_trunc (hx/literal :month) (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :month-of-year]
+ [_ _ expr]
+ (hsql/call :month (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :quarter]
+ [_ _ expr]
+ (hsql/call :date_trunc (hx/literal :quarter) (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :quarter-of-year]
+ [_ _ expr]
+ (hsql/call :quarter (in-report-zone expr)))
+
+(defmethod sql.qp/date [:presto-jdbc :year]
+ [_ _ expr]
+ (hsql/call :date_trunc (hx/literal :year) (in-report-zone expr)))
+
+(defmethod sql.qp/unix-timestamp->honeysql [:presto-jdbc :seconds]
+ [_ _ expr]
+ (let [report-zone (qp.timezone/report-timezone-id-if-supported :presto-jdbc)]
+ (hsql/call :from_unixtime expr (hx/literal (or report-zone "UTC")))))
+
+(defmethod sql.qp/unix-timestamp->honeysql [:presto-jdbc :milliseconds]
+ [_ _ expr]
+ ;; from_unixtime doesn't support milliseconds directly, but we can add them back in
+ (let [report-zone (qp.timezone/report-timezone-id-if-supported :presto-jdbc)
+ millis (hsql/call (u/qualified-name ::mod) expr 1000)]
+ (hsql/call :date_add
+ (hx/literal "millisecond")
+ millis
+ (hsql/call :from_unixtime (hsql/call :/ expr 1000) (hx/literal (or report-zone "UTC"))))))
+
+(defmethod sql.qp/unix-timestamp->honeysql [:presto-jdbc :microseconds]
+ [driver _ expr]
+ ;; Presto can't even represent microseconds, so convert to millis and call that version
+ (sql.qp/unix-timestamp->honeysql driver :milliseconds (hsql/call :/ expr 1000)))
+
+(defmethod sql.qp/current-datetime-honeysql-form :presto-jdbc
+ [_]
+ ;; the current_timestamp in Presto returns a `timestamp with time zone`, so this needs to be overridden
+ (hx/with-type-info :%now {::hx/database-type timestamp-with-time-zone-db-type}))
+
+(defmethod hformat/fn-handler (u/qualified-name ::mod)
+ [_ x y]
+ ;; Presto mod is a function like mod(x, y) rather than an operator like x mod y
+ (format "mod(%s, %s)" (hformat/to-sql x) (hformat/to-sql y)))
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Connectivity |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(defn- db-name
+ "Creates a \"DB name\" for the given catalog `c` and (optional) schema `s`. If both are specified, a slash is
+ used to separate them. See examples at:
+ https://prestodb.io/docs/current/installation/jdbc.html#connecting"
+ [c s]
+ (cond
+ (str/blank? c)
+ ""
+
+ (str/blank? s)
+ c
+
+ :else
+ (str c "/" s)))
+
+(defn- jdbc-spec
+ "Creates a spec for `clojure.java.jdbc` to use for connecting to Presto via JDBC, from the given `opts`."
+ [{:keys [host port catalog schema]
+ :or {host "localhost", port 5432, catalog ""}
+ :as opts}]
+ (-> (merge
+ {:classname "com.facebook.presto.jdbc.PrestoDriver"
+ :subprotocol "presto"
+ :subname (db.spec/make-subname host port (db-name catalog schema))}
+ (dissoc opts :host :port :db :catalog :schema))
+ sql-jdbc.common/handle-additional-options))
+
+(defmethod sql-jdbc.conn/connection-details->spec :presto-jdbc
+ [_ {ssl? :ssl, :as details-map}]
+ (let [props (-> details-map
+ (update :port (fn [port]
+ (if (string? port)
+ (Integer/parseInt port)
+ port)))
+ (assoc :SSL ssl?)
+ (dissoc :ssl))]
+ (jdbc-spec props)))
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Sync |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(defmethod sql-jdbc.sync/database-type->base-type :presto-jdbc
+ [_ field-type]
+ (presto-common/presto-type->base-type field-type))
+
+(defn- have-select-privilege?
+ "Checks whether the connected user has permission to select from the given `table-name`, in the given `schema`.
+ Adapted from the legacy Presto driver implementation."
+ [driver conn schema table-name]
+ (try
+ (let [sql (sql-jdbc.describe-database/simple-select-probe-query driver schema table-name)]
+ ;; if the query completes without throwing an Exception, we can SELECT from this table
+ (jdbc/reducible-query {:connection conn} sql)
+ true)
+ (catch Throwable _
+ false)))
+
+(defn- describe-schema
+ "Gets a set of maps for all tables in the given `catalog` and `schema`. Adapted from the legacy Presto driver
+ implementation."
+ [driver conn catalog schema]
+ (let [sql (presto-common/describe-schema-sql driver catalog schema)]
+ (log/trace (trs "Running statement in describe-schema: {0}" sql))
+ (into #{} (comp (filter (fn [{table-name :table}]
+ (have-select-privilege? driver conn schema table-name)))
+ (map (fn [{table-name :table}]
+ {:name table-name
+ :schema schema})))
+ (jdbc/reducible-query {:connection conn} sql))))
+
+(defn- all-schemas
+ "Gets a set of maps for all tables in all schemas in the given `catalog`. Adapted from the legacy Presto driver
+ implementation."
+ [driver conn catalog]
+ (let [sql (presto-common/describe-catalog-sql driver catalog)]
+ (log/trace (trs "Running statement in all-schemas: {0}" sql))
+ (into []
+ (map (fn [{:keys [schema] :as full}]
+ (when-not (contains? presto-common/excluded-schemas schema)
+ (describe-schema driver conn catalog schema))))
+ (jdbc/reducible-query {:connection conn} sql))))
+
+(defmethod driver/describe-database :presto-jdbc
+ [driver {{:keys [catalog schema] :as details} :details :as database}]
+ (with-open [conn (-> (sql-jdbc.conn/db->pooled-connection-spec database)
+ jdbc/get-connection)]
+ (let [schemas (if schema #{(describe-schema driver conn catalog schema)}
+ (all-schemas driver conn catalog))]
+ {:tables (reduce set/union schemas)})))
+
+(defmethod driver/describe-table :presto-jdbc
+ [driver {{:keys [catalog] :as details} :details :as database} {schema :schema, table-name :name}]
+ (with-open [conn (-> (sql-jdbc.conn/db->pooled-connection-spec database)
+ jdbc/get-connection)]
+ (let [sql (presto-common/describe-table-sql driver catalog schema table-name)]
+ (log/trace (trs "Running statement in describe-table: {0}" sql))
+ {:schema schema
+ :name table-name
+ :fields (into
+ #{}
+ (map-indexed (fn [idx {:keys [column type] :as col}]
+ {:name column
+ :database-type type
+ :base-type (presto-common/presto-type->base-type type)
+ :database-position idx}))
+ (jdbc/reducible-query {:connection conn} sql))})))
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | sql-jdbc implementations |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(defmethod sql-jdbc.execute/prepared-statement :presto-jdbc
+ [driver ^Connection conn ^String sql params]
+ ;; with Presto JDBC driver, result set holdability must be HOLD_CURSORS_OVER_COMMIT
+ ;; defining this method simply to omit setting the holdability
+ (let [stmt (.prepareStatement conn
+ sql
+ ResultSet/TYPE_FORWARD_ONLY
+ ResultSet/CONCUR_READ_ONLY)]
+ (try
+ (try
+ (.setFetchDirection stmt ResultSet/FETCH_FORWARD)
+ (catch Throwable e
+ (log/debug e (trs "Error setting prepared statement fetch direction to FETCH_FORWARD"))))
+ (sql-jdbc.execute/set-parameters! driver stmt params)
+ stmt
+ (catch Throwable e
+ (.close stmt)
+ (throw e)))))
+
+
+(defmethod sql-jdbc.execute/statement :presto-jdbc
+ [_ ^Connection conn]
+ ;; and similarly for statement (do not set holdability)
+ (let [stmt (.createStatement conn
+ ResultSet/TYPE_FORWARD_ONLY
+ ResultSet/CONCUR_READ_ONLY)]
+ (try
+ (.setFetchDirection stmt ResultSet/FETCH_FORWARD)
+ (catch Throwable e
+ (log/debug e (trs "Error setting statement fetch direction to FETCH_FORWARD"))))
+ stmt))
+
+(defmethod driver/can-connect? :sql-jdbc
+ [driver details]
+ (sql-jdbc.conn/can-connect? driver details))
+
+(defn- ^PrestoConnection pooled-conn->presto-conn
+ "Unwraps the C3P0 `pooled-conn` and returns the underlying `PrestoConnection` it holds."
+ [^C3P0ProxyConnection pooled-conn]
+ (.unwrap pooled-conn PrestoConnection))
+
+(defmethod sql-jdbc.execute/connection-with-timezone :presto-jdbc
+ [driver database ^String timezone-id]
+ ;; Presto supports setting the session timezone via a `PrestoConnection` instance method. Under the covers,
+ ;; this is equivalent to the `X-Presto-Time-Zone` header in the HTTP request (i.e. the `:presto` driver)
+ (let [conn (.getConnection (sql-jdbc.execute/datasource-with-diagnostic-info! driver database))
+ underlying-conn (pooled-conn->presto-conn conn)]
+ (try
+ (sql-jdbc.execute/set-best-transaction-level! driver conn)
+ (when-not (str/blank? timezone-id)
+ ;; set session time zone if defined
+ (.setTimeZoneId underlying-conn timezone-id))
+ (try
+ (.setReadOnly conn true)
+ (catch Throwable e
+ (log/debug e (trs "Error setting connection to read-only"))))
+ ;; as with statement and prepared-statement, cannot set holdability on the connection level
+ conn
+ (catch Throwable e
+ (.close conn)
+ (throw e)))))
+
+(defn- date-time->substitution [ts-str]
+ (sql.params.substitution/make-stmt-subs "from_iso8601_timestamp(?)" [ts-str]))
+
+(defmethod sql.params.substitution/->prepared-substitution [:presto-jdbc ZonedDateTime]
+ [_ ^ZonedDateTime t]
+ ;; for native query parameter substitution, in order to not conflict with the `PrestoConnection` session time zone
+ ;; (which was set via report time zone), it is necessary to use the `from_iso8601_timestamp` function on the string
+ ;; representation of the `ZonedDateTime` instance, but converted to the report time zone
+ #_(date-time->substitution (.format (t/offset-date-time (t/local-date-time t) (t/zone-offset 0)) DateTimeFormatter/ISO_OFFSET_DATE_TIME))
+ (let [report-zone (qp.timezone/report-timezone-id-if-supported :presto-jdbc)
+ ^ZonedDateTime ts (if (str/blank? report-zone) t (t/with-zone-same-instant t (t/zone-id report-zone)))]
+ ;; the `from_iso8601_timestamp` only accepts timestamps with an offset (not a zone ID), so only format with offset
+ (date-time->substitution (.format ts DateTimeFormatter/ISO_OFFSET_DATE_TIME))))
+
+(defmethod sql.params.substitution/->prepared-substitution [:presto-jdbc LocalDateTime]
+ [_ ^LocalDateTime t]
+ ;; similar to above implementation, but for `LocalDateTime`
+ ;; when Presto parses this, it will account for session (report) time zone
+ (date-time->substitution (.format t DateTimeFormatter/ISO_LOCAL_DATE_TIME)))
+
+(defmethod sql.params.substitution/->prepared-substitution [:presto-jdbc OffsetDateTime]
+ [_ ^OffsetDateTime t]
+ ;; similar to above implementation, but for `ZonedDateTime`
+ ;; when Presto parses this, it will account for session (report) time zone
+ (date-time->substitution (.format t DateTimeFormatter/ISO_OFFSET_DATE_TIME)))
+
+(defn- set-time-param
+ "Converts the given instance of `java.time.temporal`, assumed to be a time (either `LocalTime` or `OffsetTime`)
+ into a `java.sql.Time`, including milliseconds, and sets the result as a parameter of the `PreparedStatement` `ps`
+ at index `i`."
+ [^PreparedStatement ps ^Integer i ^Temporal t]
+ ;; for some reason, `java-time` can't handle passing millis to java.sql.Time, so this is the most straightforward way
+ ;; I could find to do it
+ ;; reported as https://github.com/dm3/clojure.java-time/issues/74
+ (let [millis-of-day (.get t ChronoField/MILLI_OF_DAY)]
+ (.setTime ps i (Time. millis-of-day))))
+
+(defmethod sql-jdbc.execute/set-parameter [:presto-jdbc OffsetTime]
+ [_ ^PreparedStatement ps ^Integer i t]
+ ;; necessary because `PrestoPreparedStatement` does not implement the `setTime` overload having the final `Calendar`
+ ;; param
+ (let [adjusted-tz (t/with-offset-same-instant t (t/zone-offset 0))]
+ (set-time-param ps i adjusted-tz)))
+
+(defmethod sql-jdbc.execute/set-parameter [:presto-jdbc LocalTime]
+ [_ ^PreparedStatement ps ^Integer i t]
+ ;; same rationale as above
+ (set-time-param ps i t))
+
+(defn- ^LocalTime sql-time->local-time
+ "Converts the given instance of `java.sql.Time` into a `java.time.LocalTime`, including milliseconds. Needed for
+ similar reasons as `set-time-param` above."
+ [^Time sql-time]
+ ;; Java 11 adds a simpler `ofInstant` method, but since we need to run on JDK 8, we can't use it
+ ;; https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/time/LocalTime.html#ofInstant(java.time.Instant,java.time.ZoneId)
+ (let [^LocalTime lt (t/local-time sql-time)
+ ^Long millis (mod (.getTime sql-time) 1000)]
+ (.with lt ChronoField/MILLI_OF_SECOND millis)))
+
+(defmethod sql-jdbc.execute/read-column-thunk [:presto-jdbc Types/TIME]
+ [_ ^ResultSet rs ^ResultSetMetaData rs-meta ^Integer i]
+ (let [type-name (.getColumnTypeName rs-meta i)
+ base-type (presto-common/presto-type->base-type type-name)
+ with-tz? (isa? base-type :type/TimeWithTZ)]
+ (fn []
+ (let [local-time (-> (.getTime rs i)
+ sql-time->local-time)]
+ ;; for both `time` and `time with time zone`, the JDBC type reported by the driver is `Types/TIME`, hence
+ ;; we also need to check the column type name to differentiate between them here
+ (if with-tz?
+ ;; even though this value is a `LocalTime`, the base-type is time with time zone, so we need to shift it back to
+ ;; the UTC (0) offset
+ (t/offset-time
+ local-time
+ (t/zone-offset 0))
+ ;; else the base-type is time without time zone, so just return the local-time value
+ local-time)))))
+
+(defn- ^PrestoConnection rs->presto-conn
+ "Returns the `PrestoConnection` associated with the given `ResultSet` `rs`."
+ [^ResultSet rs]
+ (-> (.. rs getStatement getConnection)
+ pooled-conn->presto-conn))
+
+(defmethod sql-jdbc.execute/read-column-thunk [:presto-jdbc Types/TIMESTAMP]
+ [_ ^ResultSet rset ^ResultSetMetaData rsmeta ^Integer i]
+ (let [zone (.getTimeZoneId (rs->presto-conn rset))]
+ (fn []
+ (when-let [s (.getString rset i)]
+ (when-let [t (u.date/parse s)]
+ (cond
+ (or (instance? OffsetDateTime t)
+ (instance? ZonedDateTime t))
+ (-> (t/offset-date-time t)
+ ;; tests are expecting this to be in the UTC offset, so convert to that
+ (t/with-offset-same-instant (t/zone-offset 0)))
+
+ ;; presto "helpfully" returns local results already adjusted to session time zone offset for us, e.g.
+ ;; '2021-06-15T00:00:00' becomes '2021-06-15T07:00:00' if the session timezone is US/Pacific. Undo the
+ ;; madness and convert back to UTC
+ zone
+ (-> (t/zoned-date-time t zone)
+ (u.date/with-time-zone-same-instant "UTC")
+ t/local-date-time)
+ :else
+ t))))))
+
+;;; +----------------------------------------------------------------------------------------------------------------+
+;;; | Other Driver Method Impls |
+;;; +----------------------------------------------------------------------------------------------------------------+
+
+(prefer-method driver/supports? [:presto-common :set-timezone] [:sql-jdbc :set-timezone])
diff --git a/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj b/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..17ed31f617675edee87a8e0d64b0660b30f7bf5b
--- /dev/null
+++ b/modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
@@ -0,0 +1,192 @@
+(ns metabase.driver.presto-jdbc-test
+ (:require [clojure.java.jdbc :as jdbc]
+ [clojure.test :refer :all]
+ [honeysql.core :as hsql]
+ [honeysql.format :as hformat]
+ [java-time :as t]
+ [metabase.db.metadata-queries :as metadata-queries]
+ [metabase.driver :as driver]
+ [metabase.driver.presto-jdbc :as presto-jdbc]
+ [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql.query-processor :as sql.qp]
+ [metabase.models.database :refer [Database]]
+ [metabase.models.field :refer [Field]]
+ [metabase.models.table :as table :refer [Table]]
+ [metabase.query-processor :as qp]
+ [metabase.sync :as sync]
+ [metabase.test :as mt]
+ [metabase.test.fixtures :as fixtures]
+ [metabase.test.util :as tu]
+ [toucan.db :as db]))
+
+(use-fixtures :once (fixtures/initialize :db))
+
+(deftest describe-database-test
+ (mt/test-driver :presto-jdbc
+ (is (= {:tables #{{:name "categories" :schema "default"}
+ {:name "venues" :schema "default"}
+ {:name "checkins" :schema "default"}
+ {:name "users" :schema "default"}}}
+ (-> (driver/describe-database :presto-jdbc (mt/db))
+ (update :tables (comp set (partial filter (comp #{"categories"
+ "venues"
+ "checkins"
+ "users"}
+ :name)))))))))
+
+(deftest describe-table-test
+ (mt/test-driver :presto-jdbc
+ (is (= {:name "venues"
+ :schema "default"
+ :fields #{{:name "name",
+ ;; for HTTP based Presto driver, this is coming back as varchar(255)
+ ;; however, for whatever reason, the DESCRIBE statement results do not return the length
+ :database-type "varchar"
+ :base-type :type/Text
+ :database-position 1}
+ {:name "latitude"
+ :database-type "double"
+ :base-type :type/Float
+ :database-position 3}
+ {:name "longitude"
+ :database-type "double"
+ :base-type :type/Float
+ :database-position 4}
+ {:name "price"
+ :database-type "integer"
+ :base-type :type/Integer
+ :database-position 5}
+ {:name "category_id"
+ :database-type "integer"
+ :base-type :type/Integer
+ :database-position 2}
+ {:name "id"
+ :database-type "integer"
+ :base-type :type/Integer
+ :database-position 0}}}
+ (driver/describe-table :presto-jdbc (mt/db) (db/select-one 'Table :id (mt/id :venues)))))))
+
+(deftest table-rows-sample-test
+ (mt/test-driver :presto-jdbc
+ (is (= [[1 "Red Medicine"]
+ [2 "Stout Burgers & Beers"]
+ [3 "The Apple Pan"]
+ [4 "Wurstküche"]
+ [5 "Brite Spot Family Restaurant"]]
+ (->> (metadata-queries/table-rows-sample (Table (mt/id :venues))
+ [(Field (mt/id :venues :id))
+ (Field (mt/id :venues :name))]
+ (constantly conj))
+ (sort-by first)
+ (take 5))))))
+
+(deftest page-test
+ (testing ":page clause"
+ (is (= {:select ["name" "id"]
+ :from [{:select [[:default.categories.name "name"]
+ [:default.categories.id "id"]
+ [(hsql/raw "row_number() OVER (ORDER BY \"default\".\"categories\".\"id\" ASC)")
+ :__rownum__]]
+ :from [:default.categories]
+ :order-by [[:default.categories.id :asc]]}]
+ :where [:> :__rownum__ 5]
+ :limit 5}
+ (sql.qp/apply-top-level-clause :presto-jdbc :page
+ {:select [[:default.categories.name "name"] [:default.categories.id "id"]]
+ :from [:default.categories]
+ :order-by [[:default.categories.id :asc]]}
+ {:page {:page 2
+ :items 5}})))))
+
+(deftest db-default-timezone-test
+ (mt/test-driver :presto-jdbc
+ (is (= "UTC"
+ (tu/db-timezone-id)))))
+
+(deftest template-tag-timezone-test
+ (mt/test-driver :presto-jdbc
+ (testing "Make sure date params work correctly when report timezones are set (#10487)"
+ (mt/with-temporary-setting-values [report-timezone "Asia/Hong_Kong"]
+ ;; the `read-column-thunk` for `Types/TIMESTAMP` always returns an `OffsetDateTime`, not a `LocalDateTime`, as
+ ;; the original Presto version of this test expected; therefore, convert the `ZonedDateTime` corresponding to
+ ;; midnight on this date (at the report TZ) to `OffsetDateTime` for comparison's sake
+ (is (= [[(-> (t/zoned-date-time 2014 8 2 0 0 0 0 (t/zone-id "Asia/Hong_Kong"))
+ t/offset-date-time
+ (t/with-offset-same-instant (t/zone-offset 0)))
+ (t/local-date 2014 8 2)]]
+ (mt/rows
+ (qp/process-query
+ {:database (mt/id)
+ :type :native
+ :middleware {:format-rows? false} ; turn off formatting so we can check the raw local date objs
+ :native {:query "SELECT {{date}}, cast({{date}} AS date)"
+ :template-tags {:date {:name "date" :display_name "Date" :type "date"}}}
+ :parameters [{:type "date/single"
+ :target ["variable" ["template-tag" "date"]]
+ :value "2014-08-02"}]}))))))))
+
+(deftest splice-strings-test
+ (mt/test-driver :presto-jdbc
+ (let [query (mt/mbql-query venues
+ {:aggregation [[:count]]
+ :filter [:= $name "wow"]})]
+ (testing "The native query returned in query results should use user-friendly splicing"
+ (is (= (str "SELECT count(*) AS \"count\" "
+ "FROM \"default\".\"venues\" "
+ "WHERE \"default\".\"venues\".\"name\" = 'wow'")
+ (:query (qp/query->native-with-spliced-params query))
+ (-> (qp/process-query query) :data :native_form :query)))))))
+
+(deftest connection-tests
+ (testing "db-name is correct in all cases"
+ (doseq [[c s expected] [[nil nil ""]
+ ["" "" ""]
+ ["my_catalog" nil "my_catalog"]
+ ["my_catalog" "" "my_catalog"]
+ ["my_catalog" "my_schema" "my_catalog/my_schema"]]]
+ (is (= expected (#'presto-jdbc/db-name c s)))))
+ (testing "jdbc-spec is correct"
+ (is (= {:classname "com.facebook.presto.jdbc.PrestoDriver"
+ :subname "//my-presto-server:1234/my_catalog?Option1=Value1&Option2=Value2"
+ :subprotocol "presto"}
+ (#'presto-jdbc/jdbc-spec {:host "my-presto-server"
+ :port 1234
+ :catalog "my_catalog"
+ :schema nil
+ :additional-options "Option1=Value1&Option2=Value2"})))))
+
+(deftest honeysql-tests
+ (testing "Complex HoneySQL conversions work as expected"
+ (testing "unix-timestamp with microsecond precision"
+ (is (= [(str "date_add('millisecond', mod((1623963256123456 / 1000), 1000),"
+ " from_unixtime(((1623963256123456 / 1000) / 1000), 'UTC'))")]
+ (-> (sql.qp/unix-timestamp->honeysql :presto-jdbc :microseconds (hsql/raw 1623963256123456))
+ (hformat/format)))))))
+
+(defn- execute-ddl! [ddl-statements]
+ (mt/with-driver :presto-jdbc
+ (let [jdbc-spec (sql-jdbc.conn/connection-details->spec :presto-jdbc (:details (mt/db)))]
+ (with-open [conn (doto (jdbc/get-connection jdbc-spec))]
+ (doseq [ddl-stmt ddl-statements]
+ (with-open [stmt (.prepareStatement conn ddl-stmt)]
+ (.executeUpdate stmt)))))))
+
+(deftest specific-schema-sync-test
+ (mt/test-driver :presto-jdbc
+ (testing "When a specific schema is designated, only that one is synced"
+ (let [s "specific_schema"
+ t "specific_table"
+ db-details (:details (mt/db))
+ with-schema (assoc db-details :schema s)]
+ (execute-ddl! [(format "DROP TABLE IF EXISTS %s.%s" s t)
+ (format "DROP SCHEMA IF EXISTS %s" s)
+ (format "CREATE SCHEMA %s" s)
+ (format "CREATE TABLE %s.%s (pk INTEGER, val1 VARCHAR(512))" s t)])
+ (mt/with-temp Database [db {:engine :presto-jdbc, :name "Temp Presto JDBC Schema DB", :details with-schema}]
+ (mt/with-db db
+ ;; same as test_data, but with schema, so should NOT pick up venues, users, etc.
+ (sync/sync-database! db)
+ (is (= [{:name t, :schema s, :db_id (mt/id)}]
+ (map #(select-keys % [:name :schema :db_id]) (db/select Table :db_id (mt/id)))))))
+ (execute-ddl! [(format "DROP TABLE %s.%s" s t)
+ (format "DROP SCHEMA %s" s)])))))
diff --git a/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj b/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
new file mode 100644
index 0000000000000000000000000000000000000000..96c4fc9defcffa3725550f1a3881a95786015db8
--- /dev/null
+++ b/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
@@ -0,0 +1,136 @@
+(ns metabase.test.data.presto-jdbc
+ "Presto JDBC driver test extensions."
+ (:require [clojure.string :as str]
+ [metabase.config :as config]
+ [metabase.connection-pool :as connection-pool]
+ [metabase.driver :as driver]
+ [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
+ [metabase.test.data.interface :as tx]
+ [metabase.test.data.presto-common]
+ [metabase.test.data.sql :as sql.tx]
+ [metabase.test.data.sql-jdbc :as sql-jdbc.tx]
+ [metabase.test.data.sql-jdbc.execute :as execute]
+ [metabase.test.data.sql-jdbc.load-data :as load-data]
+ [metabase.test.data.sql.ddl :as ddl])
+ (:import [java.sql Connection DriverManager PreparedStatement]))
+
+(sql-jdbc.tx/add-test-extensions! :presto-jdbc)
+
+;; during unit tests don't treat presto as having FK support
+(defmethod driver/supports? [:presto-jdbc :foreign-keys] [_ _] (not config/is-test?))
+
+(doseq [[base-type db-type] {:type/BigInteger "BIGINT"
+ :type/Boolean "BOOLEAN"
+ :type/Date "DATE"
+ :type/DateTime "TIMESTAMP"
+ :type/DateTimeWithTZ "TIMESTAMP WITH TIME ZONE"
+ :type/DateTimeWithZoneID "TIMESTAMP WITH TIME ZONE"
+ :type/DateTimeWithZoneOffset "TIMESTAMP WITH TIME ZONE"
+ :type/Decimal "DECIMAL"
+ :type/Float "DOUBLE"
+ :type/Integer "INTEGER"
+ :type/Text "VARCHAR"
+ :type/Time "TIME"
+ :type/TimeWithTZ "TIME WITH TIME ZONE"}]
+ (defmethod sql.tx/field-base-type->sql-type [:presto-jdbc base-type] [_ _] db-type))
+
+;; in the past, we had to manually update our Docker image and add a new catalog for every new dataset definition we
+;; added. That's insane. Just use the `test-data` catalog and put everything in that, and use
+;; `db-qualified-table-name` like everyone else.
+(def ^:private test-catalog-name "test_data")
+
+(def ^:private test-schema-name "default")
+
+(defn- dash->underscore [nm]
+ (str/replace nm #"-" "_"))
+
+(defmethod tx/dbdef->connection-details :presto-jdbc
+ [_ _ {:keys [database-name]}]
+ {:host (tx/db-test-env-var-or-throw :presto-jdbc :host "localhost")
+ :port (tx/db-test-env-var :presto-jdbc :port "8080")
+ :user (tx/db-test-env-var-or-throw :presto-jdbc :user "metabase")
+ :additional-options (tx/db-test-env-var :presto-jdbc :additional-options nil)
+ :ssl (tx/db-test-env-var :presto-jdbc :ssl "false")
+ :catalog (dash->underscore database-name)
+ :schema (tx/db-test-env-var :presto-jdbc :schema nil)})
+
+(defmethod execute/execute-sql! :presto-jdbc
+ [& args]
+ (apply execute/sequentially-execute-sql! args))
+
+(defn- load-data [dbdef tabledef]
+ ;; the JDBC driver statements fail with a cryptic status 500 error if there are too many
+ ;; parameters being set in a single statement; these numbers were arrived at empirically
+ (let [chunk-size (case (:table-name tabledef)
+ "people" 30
+ "reviews" 40
+ "orders" 30
+ "venues" 50
+ "products" 50
+ "cities" 50
+ "sightings" 50
+ "incidents" 50
+ "checkins" 25
+ "airport" 50
+ 100)
+ load-fn (load-data/make-load-data-fn load-data/load-data-add-ids
+ (partial load-data/load-data-chunked pmap chunk-size))]
+ (load-fn :presto-jdbc dbdef tabledef)))
+
+(defmethod load-data/load-data! :presto-jdbc
+ [_ dbdef tabledef]
+ (load-data dbdef tabledef))
+
+(defn- jdbc-spec->connection
+ "This is to work around some weird interplay between clojure.java.jdbc caching behavior of connections based on URL,
+ combined with the fact that the Presto driver apparently closes the connection when it closes a prepare statement.
+ Therefore, create a fresh connection from the DriverManager."
+ ^Connection [jdbc-spec]
+ (DriverManager/getConnection (format "jdbc:%s:%s" (:subprotocol jdbc-spec) (:subname jdbc-spec))
+ (connection-pool/map->properties (select-keys jdbc-spec [:user :SSL]))))
+
+(defmethod load-data/do-insert! :presto-jdbc
+ [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 %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))))))))
+
+(defmethod sql.tx/drop-db-if-exists-sql :presto-jdbc [_ _] nil)
+(defmethod sql.tx/create-db-sql :presto-jdbc [_ _] nil)
+
+(defmethod sql.tx/qualified-name-components :presto-jdbc
+ ;; use the default schema from the in-memory connector
+ ([_ db-name] [(dash->underscore db-name) "default"])
+ ([_ db-name table-name] [(dash->underscore db-name) "default" (dash->underscore table-name)])
+ ([_ db-name table-name field-name] [(dash->underscore db-name) "default" (dash->underscore table-name) field-name]))
+
+(defmethod sql.tx/pk-sql-type :presto-jdbc
+ [_]
+ "INTEGER")
+
+(defmethod sql.tx/create-table-sql :presto-jdbc
+ [driver dbdef tabledef]
+ ;; strip out the PRIMARY KEY stuff from the CREATE TABLE statement
+ (let [sql ((get-method sql.tx/create-table-sql :sql/test-extensions) driver dbdef tabledef)]
+ (str/replace sql #", PRIMARY KEY \([^)]+\)" "")))
+
+(defmethod tx/format-name :presto-jdbc [_ table-or-field-name]
+ (dash->underscore table-or-field-name))
+
+;; Presto doesn't support FKs, at least not adding them via DDL
+(defmethod sql.tx/add-fk-sql :presto-jdbc
+ [_ _ _ _]
+ nil)
+
+;; FIXME Presto actually has very good timezone support
+#_(defmethod tx/has-questionable-timezone-support? :presto-jdbc [_] true)
diff --git a/modules/drivers/presto/parents b/modules/drivers/presto/parents
new file mode 100644
index 0000000000000000000000000000000000000000..1b4a8dec8e88c0a560332d2c956a0644eae955b0
--- /dev/null
+++ b/modules/drivers/presto/parents
@@ -0,0 +1 @@
+presto-common
diff --git a/modules/drivers/presto/project.clj b/modules/drivers/presto/project.clj
index 037b559f262457a51e139b5aa6dc62f0798f94c4..33c8f12c6277a8232fd4abbe1889d80e91901dd9 100644
--- a/modules/drivers/presto/project.clj
+++ b/modules/drivers/presto/project.clj
@@ -5,7 +5,8 @@
{:provided
{:dependencies
[[org.clojure/clojure "1.10.1"]
- [metabase-core "1.0.0-SNAPSHOT"]]}
+ [metabase-core "1.0.0-SNAPSHOT"]
+ [metabase/presto-common-driver "1.0.0-SNAPSHOT"]]}
:uberjar
{:auto-clean true
diff --git a/modules/drivers/presto/resources/metabase-plugin.yaml b/modules/drivers/presto/resources/metabase-plugin.yaml
index 1dee26628cbce3d9008b6eaaa4156f5f71f181a3..859cb2460f5d939f2627c306f31fcbeba50615fc 100644
--- a/modules/drivers/presto/resources/metabase-plugin.yaml
+++ b/modules/drivers/presto/resources/metabase-plugin.yaml
@@ -2,11 +2,13 @@ info:
name: Metabase Presto Driver
version: 1.0.0-SNAPSHOT
description: Allows Metabase to connect to Presto databases.
+dependencies:
+ - plugin: Presto Common Driver
driver:
name: presto
- display-name: Presto
+ display-name: Presto (Deprecated Driver)
lazy-load: true
- parent: sql
+ parent: presto-common
connection-properties:
- host
- merge:
@@ -21,5 +23,8 @@ driver:
- ssl
connection-properties-include-tunnel-config: true
init:
+ - step: load-namespace
+ namespace: metabase.driver.presto-common
- step: load-namespace
namespace: metabase.driver.presto
+superseded-by: presto-jdbc
diff --git a/modules/drivers/presto/src/metabase/driver/presto.clj b/modules/drivers/presto/src/metabase/driver/presto.clj
index b07a1ec4d0ed9532b3c77738d385f37a064576f9..96032e372837eed916beba4f8a77d746971251a1 100644
--- a/modules/drivers/presto/src/metabase/driver/presto.clj
+++ b/modules/drivers/presto/src/metabase/driver/presto.clj
@@ -1,19 +1,15 @@
(ns metabase.driver.presto
- "Presto driver. See https://prestodb.io/docs/current/ for complete dox."
- (:require [buddy.core.codecs :as codecs]
- [clj-http.client :as http]
+ "Presto driver. Executes queries via the REST API. See https://prestodb.io/docs/current/ for complete dox."
+ (:require [clj-http.client :as http]
[clojure.core.async :as a]
[clojure.set :as set]
[clojure.string :as str]
[clojure.tools.logging :as log]
- [honeysql.core :as hsql]
- [honeysql.helpers :as h]
- [java-time :as t]
[medley.core :as m]
[metabase.driver :as driver]
[metabase.driver.common :as driver.common]
+ [metabase.driver.presto-common :as presto-common]
[metabase.driver.sql-jdbc.sync.describe-database :as sql-jdbc.describe-database]
- [metabase.driver.sql.query-processor :as sql.qp]
[metabase.driver.sql.util :as sql.u]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.query-processor.context :as context]
@@ -22,15 +18,12 @@
[metabase.query-processor.util :as qputil]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
- [metabase.util.honeysql-extensions :as hx]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.schema :as su]
[metabase.util.ssh :as ssh]
- [schema.core :as s])
- (:import java.sql.Time
- [java.time OffsetDateTime ZonedDateTime]))
+ [schema.core :as s]))
-(driver/register! :presto, :parent :sql)
+(driver/register! :presto, :parent :presto-common)
;;; Presto API helpers
@@ -40,28 +33,6 @@
:catalog su/NonBlankString
s/Any s/Any})
-(defn- presto-type->base-type [field-type]
- (condp re-matches field-type
- #"boolean" :type/Boolean
- #"tinyint" :type/Integer
- #"smallint" :type/Integer
- #"integer" :type/Integer
- #"bigint" :type/BigInteger
- #"real" :type/Float
- #"double" :type/Float
- #"decimal.*" :type/Decimal
- #"varchar.*" :type/Text
- #"char.*" :type/Text
- #"varbinary.*" :type/*
- #"json" :type/Text ; TODO - this should probably be Dictionary or something
- #"date" :type/Date
- #"time" :type/Time
- #"time.+" :type/DateTime
- #"array" :type/Array
- #"map" :type/Dictionary
- #"row.*" :type/* ; TODO - again, but this time we supposedly have a schema
- #".*" :type/*))
-
(s/defn ^:private details->uri
[{:keys [ssl host port]} :- PrestoConnectionDetails, path]
(str (if ssl "https" "http") "://" host ":" port
@@ -124,7 +95,7 @@
(if (seq columns)
(for [{col-name :name, col-type :type} columns]
{:name col-name
- :base_type (presto-type->base-type col-type)})
+ :base_type (presto-common/presto-type->base-type col-type)})
(:cols @next-page)))})))
(defn- cancel-query-with-id! [details query-id info-uri]
@@ -181,7 +152,7 @@
(parse-row row))
cols (for [{col-name :name, col-type :type} columns]
{:name col-name
- :base_type (presto-type->base-type col-type)})
+ :base_type (presto-common/presto-type->base-type col-type)})
async-results (delay
(when nextUri
(fetch-results-async details canceled-chan id infoUri nextUri)))]
@@ -216,14 +187,10 @@
(sql.u/quote-name driver :database catalog)))]
(= v "information_schema")))
-(defmethod sql.qp/add-interval-honeysql-form :presto
- [_ hsql-form amount unit]
- (hsql/call :date_add (hx/literal unit) amount hsql-form))
-
(s/defn ^:private database->all-schemas :- #{su/NonBlankString}
"Return a set of all schema names in this `database`."
[driver {{:keys [catalog schema] :as details} :details :as database}]
- (let [sql (str "SHOW SCHEMAS FROM " (sql.u/quote-name driver :database catalog))
+ (let [sql (presto-common/describe-catalog-sql driver catalog)
{:keys [rows]} (execute-query-for-sync details sql)]
(set (map first rows))))
@@ -237,97 +204,30 @@
false)))
(defn- describe-schema [driver {{:keys [catalog user] :as details} :details :as db} {:keys [schema]}]
- (let [sql (str "SHOW TABLES FROM " (sql.u/quote-name driver :schema catalog schema))]
+ (let [sql (presto-common/describe-schema-sql driver catalog schema)]
(set (for [[table-name & _] (:rows (execute-query-for-sync details sql))
:when (have-select-privilege? driver details schema table-name)]
{:name table-name
:schema schema}))))
-(def ^:private excluded-schemas #{"information_schema"})
-
(defmethod driver/describe-database :presto
[driver database]
- (let [schemas (remove excluded-schemas (database->all-schemas driver database))]
+ (let [schemas (remove presto-common/excluded-schemas (database->all-schemas driver database))]
{:tables (reduce set/union (for [schema schemas]
(describe-schema driver database {:schema schema})))}))
(defmethod driver/describe-table :presto
[driver {{:keys [catalog] :as details} :details} {schema :schema, table-name :name}]
- (let [sql (str "DESCRIBE " (sql.u/quote-name driver :table catalog schema table-name))
+ (let [sql (presto-common/describe-table-sql driver catalog schema table-name)
{:keys [rows]} (execute-query-for-sync details sql)]
{:schema schema
:name table-name
:fields (set (for [[idx [name type]] (m/indexed rows)]
{:name name
:database-type type
- :base-type (presto-type->base-type type)
+ :base-type (presto-common/presto-type->base-type type)
:database-position idx}))}))
-(defmethod driver/db-start-of-week :presto
- [_]
- :monday)
-
-(defmethod sql.qp/cast-temporal-string [:presto :Coercion/YYYYMMDDHHMMSSString->Temporal]
- [driver _coercion-strategy expr]
- (hsql/call :date_parse expr (hx/literal "%Y%m%d%H%i%s")))
-
-(defmethod sql.qp/cast-temporal-byte [:presto :Coercion/YYYYMMDDHHMMSSBytes->Temporal]
- [driver _coercion-strategy expr]
- (sql.qp/cast-temporal-string driver :Coercion/YYYYMMDDHHMMSSString->Temporal
- (hsql/call :from_utf8 expr)))
-
-(defmethod sql.qp/->honeysql [:presto Boolean]
- [_ bool]
- (hsql/raw (if bool "TRUE" "FALSE")))
-
-(defmethod sql.qp/->honeysql [:presto :time]
- [_ [_ t]]
- (hx/cast :time (u.date/format-sql (t/local-time t))))
-
-(defmethod sql.qp/->float :presto
- [_ value]
- (hx/cast :double value))
-
-(defmethod sql.qp/->honeysql [:presto :regex-match-first]
- [driver [_ arg pattern]]
- (hsql/call :regexp_extract (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern)))
-
-(defmethod sql.qp/->honeysql [:presto :median]
- [driver [_ arg]]
- (hsql/call :approx_percentile (sql.qp/->honeysql driver arg) 0.5))
-
-(defmethod sql.qp/->honeysql [:presto :percentile]
- [driver [_ arg p]]
- (hsql/call :approx_percentile (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver p)))
-
-(def ^:private ^:dynamic *param-splice-style*
- "How we should splice params into SQL (i.e. 'unprepare' the SQL). Either `:friendly` (the default) or `:paranoid`.
- `:friendly` makes a best-effort attempt to escape strings and generate SQL that is nice to look at, but should not
- be considered safe against all SQL injection -- use this for 'convert to SQL' functionality. `:paranoid` hex-encodes
- strings so SQL injection is impossible; this isn't nice to look at, so use this for actually running a query."
- :friendly)
-
-(defmethod unprepare/unprepare-value [:presto String]
- [_ ^String s]
- (case *param-splice-style*
- :friendly (str \' (sql.u/escape-sql s :ansi) \')
- :paranoid (format "from_utf8(from_hex('%s'))" (codecs/bytes->hex (.getBytes s "UTF-8")))))
-
-;; See https://prestodb.io/docs/current/functions/datetime.html
-
-;; This is only needed for test purposes, because some of the sample data still uses legacy types
-(defmethod unprepare/unprepare-value [:presto Time]
- [driver t]
- (unprepare/unprepare-value driver (t/local-time t)))
-
-(defmethod unprepare/unprepare-value [:presto OffsetDateTime]
- [_ t]
- (format "timestamp '%s %s %s'" (t/local-date t) (t/local-time t) (t/zone-offset t)))
-
-(defmethod unprepare/unprepare-value [:presto ZonedDateTime]
- [_ t]
- (format "timestamp '%s %s %s'" (t/local-date t) (t/local-time t) (t/zone-id t)))
-
(defmethod driver/execute-reducible-query :presto
[driver
{database-id :database
@@ -339,7 +239,7 @@
respond]
(let [sql (str "-- "
(qputil/query->remark :presto outer-query) "\n"
- (binding [*param-splice-style* :paranoid]
+ (binding [presto-common/*param-splice-style* :paranoid]
(unprepare/unprepare driver (cons sql params))))
details (merge (:details (qp.store/database))
settings)]
@@ -359,70 +259,3 @@
#".*" ; default
message))
-
-;;; `:sql-driver` methods
-
-(defmethod sql.qp/apply-top-level-clause [:presto :page]
- [_ _ honeysql-query {{:keys [items page]} :page}]
- (let [offset (* (dec page) items)]
- (if (zero? offset)
- ;; if there's no offset we can simply use limit
- (h/limit honeysql-query items)
- ;; if we need to do an offset we have to do nesting to generate a row number and where on that
- (let [over-clause (format "row_number() OVER (%s)"
- (first (hsql/format (select-keys honeysql-query [:order-by])
- :allow-dashed-names? true
- :quoting :ansi)))]
- (-> (apply h/select (map last (:select honeysql-query)))
- (h/from (h/merge-select honeysql-query [(hsql/raw over-clause) :__rownum__]))
- (h/where [:> :__rownum__ offset])
- (h/limit items))))))
-
-(defmethod sql.qp/date [:presto :default] [_ _ expr] expr)
-(defmethod sql.qp/date [:presto :minute] [_ _ expr] (hsql/call :date_trunc (hx/literal :minute) expr))
-(defmethod sql.qp/date [:presto :minute-of-hour] [_ _ expr] (hsql/call :minute expr))
-(defmethod sql.qp/date [:presto :hour] [_ _ expr] (hsql/call :date_trunc (hx/literal :hour) expr))
-(defmethod sql.qp/date [:presto :hour-of-day] [_ _ expr] (hsql/call :hour expr))
-(defmethod sql.qp/date [:presto :day] [_ _ expr] (hsql/call :date_trunc (hx/literal :day) expr))
-(defmethod sql.qp/date [:presto :day-of-month] [_ _ expr] (hsql/call :day expr))
-(defmethod sql.qp/date [:presto :day-of-year] [_ _ expr] (hsql/call :day_of_year expr))
-
-(defmethod sql.qp/date [:presto :day-of-week]
- [_ _ expr]
- (sql.qp/adjust-day-of-week :presto (hsql/call :day_of_week expr)))
-
-(defmethod sql.qp/date [:presto :week]
- [_ _ expr]
- (sql.qp/adjust-start-of-week :presto (partial hsql/call :date_trunc (hx/literal :week)) expr))
-
-(defmethod sql.qp/date [:presto :month] [_ _ expr] (hsql/call :date_trunc (hx/literal :month) expr))
-(defmethod sql.qp/date [:presto :month-of-year] [_ _ expr] (hsql/call :month expr))
-(defmethod sql.qp/date [:presto :quarter] [_ _ expr] (hsql/call :date_trunc (hx/literal :quarter) expr))
-(defmethod sql.qp/date [:presto :quarter-of-year] [_ _ expr] (hsql/call :quarter expr))
-(defmethod sql.qp/date [:presto :year] [_ _ expr] (hsql/call :date_trunc (hx/literal :year) expr))
-
-(defmethod sql.qp/unix-timestamp->honeysql [:presto :seconds]
- [_ _ expr]
- (hsql/call :from_unixtime expr))
-
-(defmethod driver.common/current-db-time-date-formatters :presto
- [_]
- (driver.common/create-db-time-formatters "yyyy-MM-dd'T'HH:mm:ss.SSSZ"))
-
-(defmethod driver.common/current-db-time-native-query :presto
- [_]
- "select to_iso8601(current_timestamp)")
-
-(defmethod driver/current-db-time :presto
- [& args]
- (apply driver.common/current-db-time args))
-
-(doseq [[feature supported?] {:set-timezone true
- :basic-aggregations true
- :standard-deviation-aggregations true
- :expressions true
- :native-parameters true
- :expression-aggregations true
- :binning true
- :foreign-keys true}]
- (defmethod driver/supports? [:presto feature] [_ _] supported?))
diff --git a/modules/drivers/presto/test/metabase/test/data/presto.clj b/modules/drivers/presto/test/metabase/test/data/presto.clj
index 6596ff2d35aa9ddc1a11c82dc5309eaa3856e512..42e32f2dd01e56805257744524c6fbe3adaaf63e 100644
--- a/modules/drivers/presto/test/metabase/test/data/presto.clj
+++ b/modules/drivers/presto/test/metabase/test/data/presto.clj
@@ -7,9 +7,11 @@
[metabase.config :as config]
[metabase.driver :as driver]
[metabase.driver.presto :as presto]
+ [metabase.driver.presto-common :as presto-common]
[metabase.driver.sql.util :as sql.u]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.test.data.interface :as tx]
+ [metabase.test.data.presto-common]
[metabase.test.data.sql :as sql.tx]))
(sql.tx/add-test-extensions! :presto)
@@ -42,19 +44,20 @@
;; we need a dummy value for every base-type to make a properly typed SELECT statement
(if (keyword? field-type)
(case field-type
- :type/Boolean "TRUE"
- :type/Integer "1"
- :type/BigInteger "cast(1 AS bigint)"
- :type/Float "1.0"
- :type/Decimal "DECIMAL '1.0'"
- :type/Text "cast('' AS VARCHAR)"
- :type/Date "current_timestamp" ; this should probably be a date type, but the test data begs to differ
- :type/DateTime "current_timestamp"
- :type/DateTimeWithTZ "current_timestamp"
- :type/Time "cast(current_time as TIME)"
+ :type/Boolean "TRUE"
+ :type/Integer "1"
+ :type/BigInteger "cast(1 AS bigint)"
+ :type/Float "1.0"
+ :type/Decimal "DECIMAL '1.0'"
+ :type/Text "cast('' AS VARCHAR)"
+ :type/Date "current_timestamp" ; this should be a date type, but the test data begs to differ
+ :type/DateTime "current_timestamp"
+ :type/DateTimeWithTZ "current_timestamp"
+ :type/DateTimeWithZoneOffset "current_timestamp" ; needed for office-checkins
+ :type/Time "cast(current_time as TIME)"
"from_hex('00')") ; this might not be the best default ever
;; we were given a native type, map it back to a base-type and try again
- (field-base-type->dummy-value (#'presto/presto-type->base-type field-type))))
+ (field-base-type->dummy-value (presto-common/presto-type->base-type field-type))))
(defmethod sql.tx/create-table-sql :presto
[driver {:keys [database-name]} {:keys [table-name], :as tabledef}]
@@ -129,15 +132,5 @@
[_ s]
(str/lower-case s))
-(defmethod tx/aggregate-column-info :presto
- ([driver ag-type]
- ((get-method tx/aggregate-column-info ::tx/test-extensions) driver ag-type))
-
- ([driver ag-type field]
- (merge
- ((get-method tx/aggregate-column-info ::tx/test-extensions) driver ag-type field)
- (when (= ag-type :sum)
- {:base_type :type/BigInteger}))))
-
;; FIXME Presto actually has very good timezone support
(defmethod tx/has-questionable-timezone-support? :presto [_] true)
diff --git a/src/metabase/db/spec.clj b/src/metabase/db/spec.clj
index 5fe60ecbc023d13a0c0524ac86ce4a4108bb40b5..a7e9377a93e40a8e787855feb85337f43f96e9cb 100644
--- a/src/metabase/db/spec.clj
+++ b/src/metabase/db/spec.clj
@@ -2,7 +2,8 @@
"Functions for creating JDBC DB specs for a given driver.
Only databases that are supported as application DBs should have functions in this namespace;
otherwise, similar functions are only needed by drivers, and belong in those namespaces."
- (:require [metabase.config :as config]))
+ (:require [clojure.string :as str]
+ [metabase.config :as config]))
(defn h2
"Create a Clojure JDBC database specification for a H2 database."
@@ -14,8 +15,12 @@
:subname db}
(dissoc opts :db)))
-(defn- make-subname [host port db]
- (str "//" host ":" port "/" db))
+(defn make-subname
+ "Make a subname for the given `host`, `port`, and `db` params. Iff `db` is not blank, then a slash will
+ precede it in the subname."
+ {:arglists '([host port db]), :added "0.39.0"}
+ [host port db]
+ (str "//" host ":" port (if-not (str/blank? db) (str "/" db) "/")))
(defn postgres
"Create a Clojure JDBC database specification for a Postgres database."
diff --git a/src/metabase/driver/sql/query_processor.clj b/src/metabase/driver/sql/query_processor.clj
index ef867bc2c7a9c04ffeabd5def305bab584f49110..15bc2f6ca41d57884bab653bb5003b6d52b721ff 100644
--- a/src/metabase/driver/sql/query_processor.clj
+++ b/src/metabase/driver/sql/query_processor.clj
@@ -100,7 +100,8 @@
:hierarchy #'driver/hierarchy)
(defmulti current-datetime-honeysql-form
- "HoneySQL form that should be used to get the current `datetime` (or equivalent). Defaults to `:%now`."
+ "HoneySQL form that should be used to get the current `datetime` (or equivalent). Defaults to `:%now`. Should ideally
+ include the database type info on the form (ex: via `hx/with-type-info`)."
{:arglists '([driver])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
diff --git a/src/metabase/plugins/dependencies.clj b/src/metabase/plugins/dependencies.clj
index 05c930e9ff5c42cafcaddb8500430c6d5587e7ca..42ba20da936b56c504bf1c5bc1bfb246be6060d5 100644
--- a/src/metabase/plugins/dependencies.clj
+++ b/src/metabase/plugins/dependencies.clj
@@ -1,5 +1,7 @@
(ns metabase.plugins.dependencies
- (:require [clojure.tools.logging :as log]
+ (:require [clojure.string :as str]
+ [clojure.tools.logging :as log]
+ [environ.core :as env]
[metabase.plugins.classloader :as classloader]
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]))
@@ -7,10 +9,11 @@
(def ^:private plugins-with-unsatisfied-deps
(atom #{}))
-(defn- dependency-type [{classname :class, plugin :plugin}]
+(defn- dependency-type [{classname :class, plugin :plugin, env-var :env-var}]
(cond
classname :class
plugin :plugin
+ env-var :env-var
:else :unknown))
(defmulti ^:private dependency-satisfied?
@@ -59,6 +62,16 @@
(log-once plugin-name (trs "Plugin ''{0}'' depends on plugin ''{1}''" plugin-name dep-plugin-name))
((set initialized-plugin-names) dep-plugin-name))
+(defmethod dependency-satisfied? :env-var
+ [_ {{plugin-name :name} :info, :as info} {env-var-name :env-var}]
+ (if (str/blank? (env/env (keyword env-var-name)))
+ (do
+ (log-once plugin-name (trs "Plugin ''{0}'' depends on environment variable ''{1}'' being set to something"
+ plugin-name
+ env-var-name))
+ false)
+ true))
+
(defn- all-dependencies-satisfied?*
[initialized-plugin-names {:keys [dependencies], {plugin-name :name} :info, :as info}]
(let [dep-satisfied? (fn [dep]
diff --git a/src/metabase/util/date_2.clj b/src/metabase/util/date_2.clj
index bb51354f12890559dbf25fae1cf87adaf7293d56..0e5304f8fb58f358880be4794392e385d1ed11df 100644
--- a/src/metabase/util/date_2.clj
+++ b/src/metabase/util/date_2.clj
@@ -467,23 +467,15 @@
converts it to the corresponding offset/zoned type; for offset/zoned types, this applies an appropriate timezone
shift."))
-;; We don't know what zone offset to shift this to, since the offset for a zone-id can vary depending on the date
-;; part of a temporal value (e.g. DST vs non-DST). So just adjust to the non-DST "standard" offset for the zone in
-;; question.
-(defn- standard-offset
- "Standard (non-DST) offset for a time zone, for cases when we don't have date information."
- ^java.time.ZoneOffset [^java.time.ZoneId zone-id]
- (.. zone-id getRules (getStandardOffset (t/instant 0))))
-
(extend-protocol WithTimeZoneSameInstant
;; convert to a OffsetTime with no offset (UTC); the OffsetTime method impl will apply the zone shift.
LocalTime
(with-time-zone-same-instant [t zone-id]
- (t/offset-time t (standard-offset zone-id)))
+ (t/offset-time t (common/standard-offset zone-id)))
OffsetTime
(with-time-zone-same-instant [t ^java.time.ZoneId zone-id]
- (t/with-offset-same-instant t (standard-offset zone-id)))
+ (t/with-offset-same-instant t (common/standard-offset zone-id)))
LocalDate
(with-time-zone-same-instant [t zone-id]
diff --git a/src/metabase/util/date_2/common.clj b/src/metabase/util/date_2/common.clj
index 17095312a1f65de16645231582af66fed609f98e..8836ee9e85c10798fc5aea28bd10645540ff91fd 100644
--- a/src/metabase/util/date_2/common.clj
+++ b/src/metabase/util/date_2/common.clj
@@ -1,7 +1,9 @@
(ns metabase.util.date-2.common
(:require [clojure.string :as str]
+ [java-time :as t]
[metabase.util :as u])
- (:import [java.time.temporal ChronoField IsoFields TemporalField WeekFields]))
+ (:import [java.time ZoneId ZoneOffset]
+ [java.time.temporal ChronoField IsoFields TemporalField WeekFields]))
;; TODO - not sure this belongs here, it seems to be a bit more general than just `date-2`.
@@ -33,3 +35,12 @@
:week-fields/week-of-month (.weekOfMonth WeekFields/SUNDAY_START)
:week-fields/week-of-week-based-year (.weekOfWeekBasedYear WeekFields/SUNDAY_START)
:week-fields/week-of-year (.weekOfYear WeekFields/SUNDAY_START)}))
+
+;; We don't know what zone offset to shift this to, since the offset for a zone-id can vary depending on the date
+;; part of a temporal value (e.g. DST vs non-DST). So just adjust to the non-DST "standard" offset for the zone in
+;; question.
+(defn standard-offset
+ "Standard (non-DST) offset for a time zone, for cases when we don't have date information. Gets the offset for the
+ given `zone-id` at January 1 of the current year (since that is the best we can do in this situation)."
+ ^ZoneOffset [^ZoneId zone-id]
+ (.. zone-id getRules (getStandardOffset (t/instant (t/offset-date-time (-> (t/zoned-date-time) t/year t/value) 1 1)))))
diff --git a/src/metabase/util/date_2/parse.clj b/src/metabase/util/date_2/parse.clj
index 518312d1d81440fce4233f9528853c3aaa206c13..65e85c94746ab6e27d920cb2c1829261d7088366 100644
--- a/src/metabase/util/date_2/parse.clj
+++ b/src/metabase/util/date_2/parse.clj
@@ -67,7 +67,7 @@
[:zone :date] (ZonedDateTime/of local-date (t/local-time 0) zone-id)
[:offset :date] (OffsetDateTime/of local-date (t/local-time 0) zone-offset)
[:local :date] local-date
- [:zone :time] (OffsetTime/of local-time zone-offset)
+ [:zone :time] (OffsetTime/of local-time (or zone-offset (common/standard-offset zone-id)))
[:offset :time] (OffsetTime/of local-time zone-offset)
[:local :time] local-time
(throw (ex-info (tru "Don''t know how to parse {0} using format {1}" (pr-str s) (pr-str formattr))
diff --git a/src/metabase/util/honeysql_extensions.clj b/src/metabase/util/honeysql_extensions.clj
index 6b04ce1361ed77f7e20103e38ffe87b3e46a4e95..aa940041c52b515dd83412e076626495c0f77791 100644
--- a/src/metabase/util/honeysql_extensions.clj
+++ b/src/metabase/util/honeysql_extensions.clj
@@ -204,12 +204,18 @@
(unwrap-typed-honeysql-form [this]
(:form this)))
+(defn type-info->db-type
+ "For a given type-info, returns the `database-type`."
+ [type-info]
+ {:added "0.39.0"}
+ (::database-type type-info))
+
(defn is-of-type?
"Is `honeysql-form` a typed form with `database-type`?
(is-of-type? expr \"datetime\") ; -> true"
[honeysql-form database-type]
- (= (::database-type (type-info honeysql-form))
+ (= (type-info->db-type (type-info honeysql-form))
(some-> database-type name str/lower-case)))
(s/defn with-database-type-info
diff --git a/test/metabase/query_processor_test/breakout_test.clj b/test/metabase/query_processor_test/breakout_test.clj
index 2347486dac29d4f65c290788cb950b2b2ec11e9b..0eeb1f52f23d23e125a515839a70b5d09e275861 100644
--- a/test/metabase/query_processor_test/breakout_test.clj
+++ b/test/metabase/query_processor_test/breakout_test.clj
@@ -244,7 +244,8 @@
{:source-query
{:source-table $$venues
:aggregation [[:count]]
- :breakout [[:field %latitude {:binning {:strategy :default}}]]}}))))))
+ :breakout [[:field %latitude {:binning {:strategy :default}}]]}
+ :order-by [[:asc $latitude]]}))))))
(testing "Binning is not supported when there is no fingerprint to determine boundaries"
;; Unfortunately our new `add-source-metadata` middleware is just too good at what it does and will pull in
diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
index 36132b145a444333753eb2e895b2adecfe5a3b6e..412a20ed4019204a482c65147d72cf61391b0c6a 100644
--- a/test/metabase/query_processor_test/date_bucketing_test.clj
+++ b/test/metabase/query_processor_test/date_bucketing_test.clj
@@ -16,11 +16,13 @@
their results."
(:require [clj-time.core :as time]
[clj-time.format :as tformat]
+ [clojure.string :as str]
[clojure.test :refer :all]
[java-time :as t]
[metabase.driver :as driver]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.driver.sql.query-processor-test :as sql.qp.test]
+ [metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.models.database :refer [Database]]
[metabase.query-processor :as qp]
[metabase.query-processor-test :as qp.test]
@@ -28,6 +30,7 @@
[metabase.test :as mt]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
+ [metabase.util.honeysql-extensions :as hx]
[potemkin.types :as p.types]
[pretty.core :as pretty]
[toucan.db :as db])
@@ -848,13 +851,24 @@
(pretty [_]
(list 'TimestampDatasetDef. intervalSeconds)))
+(defn- driver->current-datetime-base-type
+ "Returns the :base-type of the \"current timestamp\" HoneySQL form defined by the driver `d`. Relies upon the driver
+ implementation having set that explicitly via `hx/with-type-info`. Returns `nil` if it can't be determined."
+ [d]
+ (when (isa? driver/hierarchy driver/*driver* :sql)
+ (let [db-type (-> (sql.qp/current-datetime-honeysql-form d)
+ hx/type-info
+ hx/type-info->db-type)]
+ (when-not (str/blank? db-type)
+ (sql-jdbc.sync/database-type->base-type d db-type)))))
+
(defmethod mt/get-dataset-definition TimestampDatasetDef
[^TimestampDatasetDef this]
(let [interval-seconds (.intervalSeconds this)]
(mt/dataset-definition (str "checkins_interval_" interval-seconds)
["checkins"
[{:field-name "timestamp"
- :base-type :type/DateTime}]
+ :base-type (or (driver->current-datetime-base-type driver/*driver*) :type/DateTime)}]
(vec (for [i (range -15 15)]
;; TIMESTAMP FIXME — not sure if still needed
;;
@@ -1077,9 +1091,15 @@
(testing "Additional tests for filtering against various datetime bucketing units that aren't tested above"
(mt/test-drivers (mt/normal-drivers)
(doseq [[expected-count unit filter-value] addition-unit-filtering-vals]
- (testing (format "\nunit = %s" unit)
- (is (= expected-count (count-of-checkins unit filter-value))
- (format "count of rows where (= (%s date) %s) should be %d" (name unit) filter-value expected-count)))))))
+ (doseq [tz [nil "UTC"]] ;iterate on at least two report time zones to suss out bugs related to that
+ (mt/with-temporary-setting-values [report-timezone tz]
+ (testing (format "\nunit = %s" unit)
+ (is (= expected-count (count-of-checkins unit filter-value))
+ (format
+ "count of rows where (= (%s date) %s) should be %d"
+ (name unit)
+ filter-value
+ expected-count)))))))))
(deftest legacy-default-datetime-bucketing-test
(testing (str ":type/Date or :type/DateTime fields that don't have `:temporal-unit` clauses should get default `:day` "
diff --git a/test/metabase/query_processor_test/expressions_test.clj b/test/metabase/query_processor_test/expressions_test.clj
index cff90843b1d1c97b74abffd64c0b3e9165f7dbc5..3957181e3e8ee96d54e9b21ecb3ea2a554f7edb0 100644
--- a/test/metabase/query_processor_test/expressions_test.clj
+++ b/test/metabase/query_processor_test/expressions_test.clj
@@ -353,11 +353,11 @@
{:source-query {:source-table (mt/id :venues)
:aggregation [[:min (mt/id :venues :price)]
[:max (mt/id :venues :price)]]
- :breakout [[:field (mt/id :venues :name) nil]]}
+ :breakout [[:field (mt/id :venues :name) nil]]
+ :limit 3}
:expressions {:price-range [:-
[:field "max" {:base-type :type/Number}]
- [:field "min" {:base-type :type/Number}]]}
- :limit 3})))))))
+ [:field "min" {:base-type :type/Number}]]}})))))))
(deftest fk-field-and-duplicate-names-test
;; Redshift hangs on sample-dataset -- See #14784
diff --git a/test/metabase/query_processor_test/filter_test.clj b/test/metabase/query_processor_test/filter_test.clj
index 96fc24acda66cf42af0533138c6e55c73294cbe5..067aa12d5dded842c8781107d85fe1fdbf3e8fa9 100644
--- a/test/metabase/query_processor_test/filter_test.clj
+++ b/test/metabase/query_processor_test/filter_test.clj
@@ -445,8 +445,8 @@
(mt/test-drivers (mt/normal-drivers)
(testing "The QP should automatically parse String parameters in filter clauses to the correct type"
(testing "String parameter to an Integer Field"
- (is (= (mt/rows (mt/run-mbql-query venues {:filter [:= $price 4]}))
- (mt/rows (mt/run-mbql-query venues {:filter [:= $price "4"]}))))))))
+ (is (= (mt/rows (mt/run-mbql-query venues {:filter [:= $price 4] :order-by [[:asc $id]]}))
+ (mt/rows (mt/run-mbql-query venues {:filter [:= $price "4"] :order-by [[:asc $id]]}))))))))
;; For the tests below:
;;
diff --git a/test/metabase/query_processor_test/parameters_test.clj b/test/metabase/query_processor_test/parameters_test.clj
index ada1b027bee728be4ce868fe8c93e3d8e4b6fbd5..f1de41d12e931f9358c80ea786e379b6ca62dac6 100644
--- a/test/metabase/query_processor_test/parameters_test.clj
+++ b/test/metabase/query_processor_test/parameters_test.clj
@@ -86,6 +86,22 @@
:target [:dimension [:template-tag (name field)]]
:value value}]})
+;; TODO: fix this test for Presto JDBC (detailed explanation follows)
+;; Spent a few hours and need to move on. Here is the query being generated for the failing case
+;; SELECT count(*) AS "count" FROM "default"."attempts"
+;; WHERE date_trunc('day', "default"."attempts"."datetime_tz") = date '2019-11-12';
+;; And here is what it *SHOULD* be to pass the test
+;; SELECT count(*) AS "count" FROM "default"."attempts"
+;; WHERE date_trunc('day', "default"."attempts"."datetime_tz" AT TIME ZONE 'UTC') = date '2019-11-12';
+;; Notice the AT TIME ZONE 'UTC' part. In this case, the test does not set a report timezone, so a fallback of UTC
+;; should (evidently) be applied.
+;; We need the type information, that the datetime_tz is `timestamp with time zone`, to be available to
+;; (defmethod sql.qp/date [:presto-jdbc :day]
+;; However, it is not available there. The expression's HSQL type-info and db-type are both nil. Somehow need to tell
+;; the query processor (or something else?) to *include* that type information when running this test, because it's
+;; clearly known (i.e. if you sync the DB and then query the `metabase_field`, it is there and is correct.
+;; Tried manually syncing the DB (with attempted-murders dataset), and storing it to an initialized QP, to no avail.
+
;; this isn't a complete test for all possible field filter types, but it covers mostly everything
(deftest field-filter-param-test
(letfn [(is-count-= [expected-count table field value-type value]
diff --git a/test/metabase/query_processor_test/timezones_test.clj b/test/metabase/query_processor_test/timezones_test.clj
index 17857873d4e0d40c5c0f295a43d678f77e46b0de..0eb62eed080871a94c1677cd301455b1810a8627 100644
--- a/test/metabase/query_processor_test/timezones_test.clj
+++ b/test/metabase/query_processor_test/timezones_test.clj
@@ -119,8 +119,8 @@
(hsql/raw "{{date1}}")
(hsql/raw "{{date2}}")]
:order-by [[(field-identifier :users :id) :asc]]})
- :template-tags {:date1 {:name "date1" :display_name "Date1" :type "date" }
- :date2 {:name "date2" :display_name "Date2" :type "date" }}}
+ :template-tags {:date1 {:name "date1" :display_name "Date1" :type "date"}
+ :date2 {:name "date2" :display_name "Date2" :type "date"}}}
:parameters [{:type "date/single"
:target ["variable" ["template-tag" "date1"]]
:value "2014-08-02T02:00:00.000000"}
@@ -224,6 +224,6 @@
(mt/dataset attempted-murders
(doseq [timezone [nil "US/Pacific" "US/Eastern" "Asia/Hong_Kong"]]
(mt/with-temporary-setting-values [report-timezone timezone]
- (let [expected (expected-attempts)]
- (is (= expected
- (select-keys (attempts) (keys expected))))))))))
+ (let [expected (expected-attempts)
+ actual (select-keys (attempts) (keys expected))]
+ (is (= expected actual))))))))
diff --git a/test/metabase/test/data/dataset_definitions.clj b/test/metabase/test/data/dataset_definitions.clj
index 6eb9851b68ee6f2700c77764bae91a2bd5418b26..df74a3751cc5c2f002a57fb43d093bca148c1d63 100644
--- a/test/metabase/test/data/dataset_definitions.clj
+++ b/test/metabase/test/data/dataset_definitions.clj
@@ -209,5 +209,4 @@
(t/local-time t) ; time
(t/offset-time t) ; time-ltz
(t/offset-time t) ; time-tz
- cnt ; num-crows
- ])]])
+ cnt])]]) ; num-crows
diff --git a/test/metabase/test/data/dataset_definitions/office-checkins.edn b/test/metabase/test/data/dataset_definitions/office-checkins.edn
index 92c42085a468a81c097039a796c48d30ec2da71a..55ff4a78384e5c5842701ee662f86fdf93cde502 100644
--- a/test/metabase/test/data/dataset_definitions/office-checkins.edn
+++ b/test/metabase/test/data/dataset_definitions/office-checkins.edn
@@ -1,5 +1,5 @@
[["checkins" [{:field-name "person", :base-type :type/Text}
- {:field-name "timestamp", :base-type :type/DateTime}]
+ {:field-name "timestamp", :base-type :type/DateTimeWithZoneOffset}]
[["Cam", #t "2019-01-02T05:30:00.000-07:00"]
["Cam", #t "2019-01-09T05:30:00.000-07:00"]
["Kyle", #t "2019-01-06T08:30:00.000-07:00"]
diff --git a/test/metabase/util/date_2_test.clj b/test/metabase/util/date_2_test.clj
index d84aa4dc9326aaf78a347d7880bc42ce4ebd29b7..69696a8f4b48ed84e98d3e4907ccda8d206ac632 100644
--- a/test/metabase/util/date_2_test.clj
+++ b/test/metabase/util/date_2_test.clj
@@ -586,3 +586,11 @@
(testing "can handle infinity dates (#12761)"
(is (u.date/with-time-zone-same-instant java.time.OffsetDateTime/MAX (t/zone-id "UTC")))
(is (u.date/with-time-zone-same-instant java.time.OffsetDateTime/MIN (t/zone-id "UTC")))))
+
+(deftest standard-offset-test
+ (testing "standard-offset works correctly, regardless of system clock timezone"
+ (doseq [clk [#t "2021-01-01T00:00-06:00[US/Central]" ; one in CST
+ #t "2021-07-01T00:00-05:00[US/Central]"]] ; one in CDT
+ (mt/with-clock clk
+ (is (= (t/zone-offset "-06:00") (u.date.common/standard-offset (t/zone-id "America/Chicago"))))
+ (is (= (t/zone-offset "Z") (u.date.common/standard-offset (t/zone-id "UTC"))))))))