Fix support for H2/Postgres CLOB types (#12029)

* Fix support for H2/Postgres CLOB types [ci h2] [ci postgres]

* Fix circular references

src/metabase/db/jdbc_protocols.clj

@@ -66,23 +66,29 @@
   (set-parameter [t stmt i]
     (jdbc/set-parameter (t/offset-date-time t) stmt i)))
 
-(extend-protocol jdbc/IResultSetReadColumn
-  org.postgresql.util.PGobject
-  (result-set-read-column [clob _ _]
-    (.getValue clob))
-
-  org.h2.jdbc.JdbcClob
-  (result-set-read-column [clob _ _]
-    (letfn [(clob->str [^BufferedReader buffered-reader]
+(defn clob->str
+  "Convert an H2 clob to a String."
+  ^String [^org.h2.jdbc.JdbcClob clob]
+  (when clob
+    (letfn [(->str [^BufferedReader buffered-reader]
               (loop [acc []]
                 (if-let [line (.readLine buffered-reader)]
                   (recur (conj acc line))
                   (str/join "\n" acc))))]
       (with-open [reader (.getCharacterStream clob)]
         (if (instance? BufferedReader reader)
-          (clob->str reader)
+          (->str reader)
           (with-open [buffered-reader (BufferedReader. reader)]
-            (clob->str buffered-reader)))))))
+            (->str buffered-reader)))))))
+
+(extend-protocol jdbc/IResultSetReadColumn
+  org.postgresql.util.PGobject
+  (result-set-read-column [clob _ _]
+    (.getValue clob))
+
+  org.h2.jdbc.JdbcClob
+  (result-set-read-column [clob _ _]
+    (clob->str clob)))
 
 (defmulti ^:private read-column
   {:arglists '([rs rsmeta i])}

src/metabase/driver/h2.clj

@@ -6,20 +6,24 @@
              [db :as mdb]
              [driver :as driver]
              [util :as u]]
-            [metabase.db.spec :as dbspec]
+            [metabase.db
+             [jdbc-protocols :as jdbc-protocols]
+             [spec :as dbspec]]
             [metabase.driver.common :as driver.common]
             [metabase.driver.sql-jdbc
              [connection :as sql-jdbc.conn]
              [execute :as sql-jdbc.execute]
              [sync :as sql-jdbc.sync]]
             [metabase.driver.sql.query-processor :as sql.qp]
+            [metabase.plugins.classloader :as classloader]
             [metabase.query-processor
              [error-type :as error-type]
              [store :as qp.store]]
             [metabase.util
              [honeysql-extensions :as hx]
              [i18n :refer [deferred-tru tru]]])
-  (:import java.time.OffsetTime))
+  (:import [java.sql Clob ResultSet ResultSetMetaData]
+           java.time.OffsetTime))
 
 (driver/register! :h2, :parent :sql-jdbc)
 
@@ -289,6 +293,16 @@
         (.close conn)
         (throw e)))))
 
+;; de-CLOB any CLOB values that come back
+(defmethod sql-jdbc.execute/read-column-thunk :h2
+  [_ ^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i]
+  (let [classname (Class/forName (.getColumnClassName rsmeta i) true (classloader/the-classloader))]
+    (if (isa? classname Clob)
+      (fn []
+        (jdbc-protocols/clob->str (.getObject rs i)))
+      (fn []
+        (.getObject rs i)))))
+
 (defmethod sql-jdbc.execute/set-parameter [:h2 OffsetTime]
   [driver prepared-statement i t]
   (let [local-time (t/local-time (t/with-offset-same-instant t (t/zone-offset 0)))]

src/metabase/driver/postgres.clj

@@ -341,6 +341,15 @@
     (fn []
       (.getObject rs i))))
 
+;; de-CLOB any CLOB values that come back
+(defmethod sql-jdbc.execute/read-column-thunk :postgres
+  [_ ^ResultSet rs _ ^Integer i]
+  (fn []
+    (let [obj (.getObject rs i)]
+      (if (instance? org.postgresql.util.PGobject obj)
+        (.getValue ^org.postgresql.util.PGobject obj)
+        obj))))
+
 ;; Postgres doesn't support OffsetTime
 (defmethod sql-jdbc.execute/set-parameter [:postgres OffsetTime]
   [driver prepared-statement i t]

src/metabase/driver/sql_jdbc/execute.clj

@@ -89,7 +89,12 @@
   :hierarchy #'driver/hierarchy)
 
 (defmulti read-column-thunk
-  "Return a zero-arg function that, when called, will fetch the value of the column from the current row."
+  "Return a zero-arg function that, when called, will fetch the value of the column from the current row. This also
+  supports defaults for the entire driver:
+
+    ;; default method for Postgres not covered by any [driver jdbc-type] methods
+    (defmethod read-column-thunk :postgres
+      ...)"
   {:added "0.35.0", :arglists '([driver rs rsmeta i])}
   (fn [driver _ ^ResultSetMetaData rsmeta ^long col-idx]
     [(driver/dispatch-on-initialized-driver driver) (.getColumnType rsmeta col-idx)])
@@ -262,10 +267,12 @@
   (.executeQuery stmt))
 
 (defmethod read-column-thunk :default
-  [_ ^ResultSet rs _ ^long i]
-  ^{:name (format "(.getObject rs %d)" i)}
-  (fn []
-    (.getObject rs i)))
+  [driver ^ResultSet rs rsmeta ^long i]
+  (let [driver-default-method (get-method read-column-thunk driver)]
+    (if-not (= driver-default-method (get-method read-column-thunk :default))
+      ^{:name (format "(read-column-thunk %s)" driver)} (driver-default-method driver rs rsmeta i)
+      ^{:name (format "(.getObject rs %d)" i)} (fn []
+                                                 (.getObject rs i)))))
 
 (defn- get-object-of-class-thunk [^ResultSet rs, ^long i, ^Class klass]
   ^{:name (format "(.getObject rs %d %s)" i (.getCanonicalName klass))}
@@ -356,7 +363,7 @@
         #_:jdbc_type #_ (u/ignore-exceptions
                           (.getName (JDBCType/valueOf (.getColumnType rsmeta i))))
         #_:db_type   #_db-type-name
-        :base_type   base-type}))
+        :base_type   (or base-type :type/*)}))
    (column-range rsmeta)))
 
 (defn reducible-rows

src/metabase/query_processor/context/default.clj

@@ -63,8 +63,7 @@
                                   (context/raisef (ex-info (tru "Error reducing result rows")
                                                            {:type error-type/qp}
                                                            e)
-                                                  context)
-                                  nil))]
+                                                  context)))]
         (context/reducedf metadata reduced-rows context)))))
 
 (defn- default-runf [query rf context]

test/metabase/driver/h2_test.clj

@@ -82,3 +82,18 @@
   (testing "Non-fractional seconds should remain seconds, but be cast to longs"
     (is (= (hsql/call :dateadd (hx/literal "second") 100 :%now)
            (sql.qp/add-interval-honeysql-form :h2 :%now 100.0 :second)))))
+
+(deftest clob-test
+  (mt/test-driver :h2
+    (testing "Make sure we properly handle rows that come back as `org.h2.jdbc.JdbcClob`"
+      (let [results (qp/process-query (mt/native-query {:query "SELECT cast('Conchúr Tihomir' AS clob) AS name;"}))]
+        (testing "rows"
+          (is (= [["Conchúr Tihomir"]]
+                 (mt/rows results))))
+        (testing "cols"
+          (is (= [{:display_name "NAME"
+                   :base_type    :type/Text
+                   :source       :native
+                   :field_ref    [:field-literal "NAME" :type/Text]
+                   :name         "NAME"}]
+                 (mt/cols results))))))))

test/metabase/driver/postgres_test.clj

@@ -476,3 +476,18 @@
             {:database (mt/id)
              :type     :native
              :native   {:query "SELECT adsasdasd;"}}))))))
+
+(deftest pgobject-test
+  (mt/test-driver :postgres
+    (testing "Make sure PGobjects are decoded correctly"
+      (let [results (qp/process-query (mt/native-query {:query "SELECT pg_sleep(0.1) AS sleep;"}))]
+        (testing "rows"
+          (is (= [[""]]
+                 (mt/rows results))))
+        (testing "cols"
+          (is (= [{:display_name "sleep"
+                   :base_type    :type/*
+                   :source       :native
+                   :field_ref    [:field-literal "sleep" :type/*]
+                   :name         "sleep"}]
+                 (mt/cols results))))))))