From 80b46b1f75382ca3e806334f6fc01f67ed484d68 Mon Sep 17 00:00:00 2001
From: Jeff Evans <jeff303@users.noreply.github.com>
Date: Fri, 23 Jul 2021 17:03:06 -0500
Subject: [PATCH] Implement JDBC based Presto driver (#16194)
Implement JDBC based Presto driver
Adding new Presto JDBC driver using the PrestoDB JDBC driver from `https://github.com/prestodb/presto`
Marking the old Presto driver as being `superseded-by` the new one
Pulling out common Presto code into new presto-common driver (modeled after the relationship between, ex: `googleanalytics` and `google)`
Putting common QP/HoneySQL logic into the new (abstract) :presto-common driver
Updating :presto driver to extend from the new common driver and only adding HTTP/REST API related methods
Adding implementation of Presto JDBC driver, named :presto-jdbc, extending from :presto-common and :sql-jdbc
Using com.facebook.presto/presto-jdbc as underlying JDBC driver dependency (since this is explicitly for Presto clusters, as opposed to Trino)
Adapting code from the existing Presto driver where appropriate
Adding new dependency-satisfied? implementation for :env-var, to allow for a plugin to require an env var be set, and making the Presto JDBC driver depend on that (specifically: `mb-enable-presto-jdbc-driver`)
Adding CircleCI configuration to run against the newer Presto (0.254) Docker image
Adding explicit ordering in a few tests where it was missing
Fixing presto-type->base-type (timestamps were being synced as :type/Time because the regex pattern was wrong)
Add tx/format-name test implementation for :presto-jdbc to lowercase table name
Make modified test Oracle friendly
Fixing bug parsing the `[:zone :time]` case within `metabase.util.date-2.parse/parse-with-formatter`; the offset is nil so it can't be passed directly in this case, so use the `standard-offset` fn (which was moved from `date-2` to `common` to get a standard offset for that zone
Fixing more test failures by adding explicit ordering
Changing sync to check whether the driver supports foreign keys before attempting to sync those (since drivers might throw an exception if attempting to check)
Moving some common test dataset functionality between :presto and :presto-jdbc to a new test.data ns for :presto-common
Adding HoneySQL form for :count-where, since we have to explicitly give a higher precision Decimal in order for Presto to not reduce the precision in results
Put limit within subquery for `expression-using-aggregation-test` (since ordering from subquery is not guaranteed in Presto)
Adding impls for ->prepared-substitution to handle substitutions for native query params
Adding HoneySQL impls for `mod` (to do as "mod(x,y)" and `timestamp` (since it's a function with no parens to invoke) functions
Adding various `sql.qp/date` impls that use the `AT TIME ZONE` operator to account for report tz, and make bucketing tests happy
---
.circleci/config.yml | 74 ++-
.../row_level_restrictions_test.clj | 24 +-
.../serialization/load_test.clj | 2 +-
modules/drivers/presto-common/project.clj | 24 +
.../resources/metabase-plugin.yaml | 11 +
.../src/metabase/driver/presto_common.clj | 212 ++++++++
.../test/metabase/test/data/presto_common.clj | 15 +
modules/drivers/presto-jdbc/parents | 1 +
modules/drivers/presto-jdbc/project.clj | 19 +
.../resources/metabase-plugin.yaml | 37 ++
.../src/metabase/driver/presto_jdbc.clj | 477 ++++++++++++++++++
.../test/metabase/driver/presto_jdbc_test.clj | 192 +++++++
.../test/metabase/test/data/presto_jdbc.clj | 136 +++++
modules/drivers/presto/parents | 1 +
modules/drivers/presto/project.clj | 3 +-
.../presto/resources/metabase-plugin.yaml | 9 +-
.../presto/src/metabase/driver/presto.clj | 193 +------
.../presto/test/metabase/test/data/presto.clj | 35 +-
src/metabase/db/spec.clj | 11 +-
src/metabase/driver/sql/query_processor.clj | 3 +-
src/metabase/plugins/dependencies.clj | 17 +-
src/metabase/util/date_2.clj | 12 +-
src/metabase/util/date_2/common.clj | 13 +-
src/metabase/util/date_2/parse.clj | 2 +-
src/metabase/util/honeysql_extensions.clj | 8 +-
.../query_processor_test/breakout_test.clj | 3 +-
.../date_bucketing_test.clj | 28 +-
.../query_processor_test/expressions_test.clj | 6 +-
.../query_processor_test/filter_test.clj | 4 +-
.../query_processor_test/parameters_test.clj | 16 +
.../query_processor_test/timezones_test.clj | 10 +-
.../test/data/dataset_definitions.clj | 3 +-
.../dataset_definitions/office-checkins.edn | 2 +-
test/metabase/util/date_2_test.clj | 8 +
34 files changed, 1359 insertions(+), 252 deletions(-)
create mode 100644 modules/drivers/presto-common/project.clj
create mode 100644 modules/drivers/presto-common/resources/metabase-plugin.yaml
create mode 100644 modules/drivers/presto-common/src/metabase/driver/presto_common.clj
create mode 100644 modules/drivers/presto-common/test/metabase/test/data/presto_common.clj
create mode 100644 modules/drivers/presto-jdbc/parents
create mode 100644 modules/drivers/presto-jdbc/project.clj
create mode 100644 modules/drivers/presto-jdbc/resources/metabase-plugin.yaml
create mode 100644 modules/drivers/presto-jdbc/src/metabase/driver/presto_jdbc.clj
create mode 100644 modules/drivers/presto-jdbc/test/metabase/driver/presto_jdbc_test.clj
create mode 100644 modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
create mode 100644 modules/drivers/presto/parents
diff --git a/.circleci/config.yml b/.circleci/config.yml
index dba1264c59e..915d1bd9ab2 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 f3b223cc40f..088d893eee1 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 fa14d3c3850..8b7234ed735 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 00000000000..6fd0ab15bb3
--- /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 00000000000..9d6c8c96c48
--- /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 00000000000..498a424e369
--- /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 00000000000..182d3278046
--- /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 00000000000..1b4a8dec8e8
--- /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 00000000000..9bd5b358a66
--- /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 00000000000..973edfb617e
--- /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 00000000000..3a9cd285c19
--- /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 00000000000..17ed31f6176
--- /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 00000000000..96c4fc9defc
--- /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 00000000000..1b4a8dec8e8
--- /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 037b559f262..33c8f12c627 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 1dee26628cb..859cb2460f5 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 b07a1ec4d0e..96032e37283 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 6596ff2d35a..42e32f2dd01 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 5fe60ecbc02..a7e9377a93e 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 ef867bc2c7a..15bc2f6ca41 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 05c930e9ff5..42ba20da936 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 bb51354f128..0e5304f8fb5 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 17095312a1f..8836ee9e85c 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 518312d1d81..65e85c94746 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 6b04ce1361e..aa940041c52 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 2347486dac2..0eeb1f52f23 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 36132b145a4..412a20ed401 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 cff90843b1d..3957181e3e8 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 96fc24acda6..067aa12d5dd 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 ada1b027bee..f1de41d12e9 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 17857873d4e..0eb62eed080 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 6eb9851b68e..df74a3751cc 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 92c42085a46..55ff4a78384 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 d84aa4dc932..69696a8f4b4 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"))))))))
--
GitLab