Fix querying `TIMESTAMP WITH LOCAL TIME ZONE` for oracle (#44456)

modules/drivers/oracle/src/metabase/driver/oracle.clj

@@ -23,6 +23,7 @@
    [metabase.driver.sql.util.unprepare :as unprepare]
    [metabase.models.secret :as secret]
    [metabase.query-processor.timezone :as qp.timezone]
+   [metabase.util.date-2 :as u.date]
    [metabase.util.honey-sql-2 :as h2x]
    [metabase.util.log :as log]
    [metabase.util.ssh :as ssh])
@@ -77,7 +78,8 @@
     ;; Spatial types -- see http://docs.oracle.com/cd/B28359_01/server.111/b28286/sql_elements001.htm#i107588
     [#"^SDO_"       :type/*]
     [#"STRUCT"      :type/*]
-    [#"TIMESTAMP(\(\d\))? WITH TIME ZONE" :type/DateTimeWithTZ]
+    [#"TIMESTAMP(\(\d\))? WITH TIME ZONE"       :type/DateTimeWithTZ]
+    [#"TIMESTAMP(\(\d\))? WITH LOCAL TIME ZONE" :type/DateTimeWithLocalTZ]
     [#"TIMESTAMP"   :type/DateTime]
     [#"URI"         :type/Text]
     [#"XML"         :type/*]]))
@@ -610,12 +612,16 @@
     (when-let [^TIMESTAMPTZ t (.getObject rs i TIMESTAMPTZ)]
       (let [^C3P0ProxyConnection proxy-conn (.. rs getStatement getConnection)
             conn                            (.unwrap proxy-conn OracleConnection)]
-        ;; TIMEZONE FIXME - we need to warn if the Oracle JDBC driver is `ojdbc7.jar`, which probably won't have this
-        ;; method
-        ;;
-        ;; I think we can call `(oracle.jdbc.OracleDriver/getJDBCVersion)` and check whether it returns 4.2+
         (.offsetDateTimeValue t conn)))))
 
+(defmethod sql-jdbc.execute/read-column-thunk [:oracle OracleTypes/TIMESTAMPLTZ]
+  [_driver ^ResultSet rs _rsmeta ^Integer i]
+  ;; `.offsetDateTimeValue` with TIMESTAMPLTZ returns the incorrect value with daylight savings time, so instead of
+  ;; trusting Oracle to get time zones right we assume the string value from `.getNString` is correct and is in a format
+  ;; we can parse.
+  (fn []
+    (u.date/parse (.getNString rs i))))
+
 (defmethod unprepare/unprepare-value [:oracle OffsetDateTime]
   [_ t]
   ;; Oracle doesn't like `Z` to mean UTC

modules/drivers/oracle/test/metabase/driver/oracle_test.clj

@@ -30,6 +30,7 @@
    [metabase.test.data.sql :as sql.tx]
    [metabase.test.data.sql.ddl :as ddl]
    [metabase.util :as u]
+   [metabase.util.date-2 :as u.date]
    [metabase.util.honey-sql-2 :as h2x]
    [metabase.util.log :as log]
    [toucan2.core :as t2]
@@ -461,3 +462,41 @@
   (testing "Oracle should strip double quotes and null characters from identifiers"
     (is (= "ABC_D_E__FG_H"
            (driver/escape-alias :oracle "ABC\"D\"E\"\u0000FG\u0000H")))))
+
+(deftest read-timestamp-with-tz-test
+  (mt/test-driver :oracle
+    (testing "We should return TIMESTAMP WITH TIME ZONE columns correctly"
+      (let [test-date "2024-12-20 16:05:00 US/Eastern"
+            sql       (format "SELECT TIMESTAMP '%s' AS timestamp_tz FROM dual" test-date)
+            query     (-> (mt/native-query {:query sql})
+                          (assoc-in [:middleware :format-rows?] false))]
+        (doseq [report-tz ["UTC" "US/Pacific"]]
+          (mt/with-temporary-setting-values [report-timezone report-tz]
+            (mt/with-native-query-testing-context query
+              (testing "The value should come back from driver with original zone info, regardless of report timezone"
+                (= (t/offset-date-time (u.date/parse test-date) "US/Eastern")
+                   (ffirst (mt/rows (qp/process-query query))))))))))))
+
+(deftest read-timestamp-with-local-tz-test
+  (mt/test-driver :oracle
+    (testing "We should return TIMESTAMP WITH LOCAL TIME ZONE columns correctly"
+      (let [test-date  "2024-12-20 16:05:00 US/Pacific"
+            sql        (format "SELECT CAST(TIMESTAMP '%s' AS TIMESTAMP WITH LOCAL TIME ZONE) AS timestamp_tz FROM dual" test-date)
+            query      (-> (mt/native-query {:query sql})
+                           (assoc-in [:middleware :format-rows?] false))
+            spec       #(sql-jdbc.conn/db->pooled-connection-spec (mt/db))
+            old-format (-> (jdbc/query (spec) "SELECT value FROM NLS_SESSION_PARAMETERS WHERE PARAMETER = 'NLS_TIMESTAMP_TZ_FORMAT'")
+                           ffirst
+                           val)
+            do-with-temp-tz-format (fn [thunk]
+                                     ;; verify that the value is independent of NLS_TIMESTAMP_TZ_FORMAT
+                                     (jdbc/execute! (spec) "ALTER SESSION SET NLS_TIMESTAMP_TZ_FORMAT = 'YYYY-MM-DD HH:MI'")
+                                     (thunk)
+                                     (jdbc/execute! (spec) (format "ALTER SESSION SET NLS_TIMESTAMP_TZ_FORMAT = '%s'" old-format)))]
+        (do-with-temp-tz-format
+         (fn []
+           (doseq [report-tz ["UTC" "US/Pacific"]]
+             (mt/with-temporary-setting-values [report-timezone report-tz]
+               (mt/with-native-query-testing-context query
+                 (is (= (t/zoned-date-time (u.date/parse test-date) report-tz)
+                        (ffirst (mt/rows (qp/process-query query))))))))))))))

modules/drivers/oracle/test/metabase/test/data/oracle.clj

@@ -225,7 +225,7 @@
 
 (defn drop-user! [username]
   (u/ignore-exceptions
-   (execute! "DROP USER %s CASCADE" username)))
+   (execute! "DROP USER \"%s\" CASCADE" username)))
 
 (defmethod tx/before-run :oracle
   [_]