From b5ea59aab57422949c23650518c3233af0d9180f Mon Sep 17 00:00:00 2001
From: dpsutton <dan@dpsutton.com>
Date: Tue, 2 Aug 2022 15:01:55 -0500
Subject: [PATCH] bump snowflake (#24480)
* bump snowflake
* Handle Types/TIMESTAMP_WITH_TIMEZONE for snowflake
Snowflake used to return columns of this type as `Types/TIMESTAMP` =
93.
The driver is registered with the legacy stuff
```clojure
(driver/register! :snowflake,
:parent #{:sql-jdbc
::sql-jdbc.legacy/use-legacy-classes-for-read-and-set})
```
causing it to hit the following codepath:
```clojure
(defmethod sql-jdbc.execute/read-column-thunk [::use-legacy-classes-for-read-and-set Types/TIMESTAMP]
[_ ^ResultSet rs _ ^Integer i]
(fn []
(when-let [s (.getString rs i)]
(let [t (u.date/parse s)]
(log/tracef "(.getString rs i) [TIMESTAMP] -> %s -> %s" (pr-str s) (pr-str t))
t))))
```
But snowflake now reports the column as `Types/TIMESTAMP_WITH_TIMEZONE`
= 2014 in https://github.com/snowflakedb/snowflake-jdbc/pull/934/files
we no longer hit this string based path for the timestamp with timezone
pathway. Instead it hits
```clojure
(defn- get-object-of-class-thunk [^ResultSet rs, ^long i, ^Class klass]
^{:name (format "(.getObject rs %d %s)" i (.getCanonicalName klass))}
(fn []
(.getObject rs i klass)))
,,,
(defmethod read-column-thunk [:sql-jdbc Types/TIMESTAMP_WITH_TIMEZONE]
[_ rs _ i]
(get-object-of-class-thunk rs i java.time.OffsetDateTime))
```
And `(.getObject ...)` blows up with an unsupported exception.
```java
// @Override
public <T> T getObject(int columnIndex, Class<T> type) throws SQLException {
logger.debug("public <T> T getObject(int columnIndex,Class<T> type)", false);
throw new SnowflakeLoggedFeatureNotSupportedException(session);
}
```
There seem to be some `getTimetamp` methods on the
`SnowflakeBaseResultSet` that we could call but for now I'm just
grabbing the string and we'll parse on our side.
One style note, it is not quite clear to me if this should follow the
pattern established in `snowflake.clj` driver of setting `defmethod
sql-jdbc.execute/read-column-thunk [:snowflake
Types/TIMESTAMP_WITH_TIMEZONE]` or if it should follow the legacy
pattern of `defmethod sql-jdbc.execute/read-column-thunk
[::use-legacy-classes-for-read-and-set Types/TIMESTAMP]`. It seems like
almost either would be fine. Just depends on whether other legacy
drivers might like this fix which seems possible?
---
modules/drivers/snowflake/deps.edn | 2 +-
.../drivers/snowflake/src/metabase/driver/snowflake.clj | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/modules/drivers/snowflake/deps.edn b/modules/drivers/snowflake/deps.edn
index 08c54c71b57..8ad82597fd8 100644
--- a/modules/drivers/snowflake/deps.edn
+++ b/modules/drivers/snowflake/deps.edn
@@ -2,4 +2,4 @@
["src" "resources"]
:deps
- {net.snowflake/snowflake-jdbc {:mvn/version "3.13.14"}}}
+ {net.snowflake/snowflake-jdbc {:mvn/version "3.13.21"}}}
diff --git a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
index 75c2d1d67e1..5b533ce6590 100644
--- a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
+++ b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj
@@ -335,6 +335,15 @@
;; Like Vertica, Snowflake doesn't seem to be able to return a LocalTime/OffsetTime like everyone else, but it can
;; return a String that we can parse
+
+(defmethod sql-jdbc.execute/read-column-thunk [:snowflake Types/TIMESTAMP_WITH_TIMEZONE]
+ [_ ^ResultSet rs _ ^Integer i]
+ (fn []
+ (when-let [s (.getString rs i)]
+ (let [t (u.date/parse s)]
+ (log/tracef "(.getString rs %d) [TIMESTAMP_WITH_TIMEZONE] -> %s -> %s" i (pr-str s) (pr-str t))
+ t))))
+
(defmethod sql-jdbc.execute/read-column-thunk [:snowflake Types/TIME]
[_ ^ResultSet rs _ ^Integer i]
(fn []
--
GitLab