diff --git a/modules/drivers/sqlite/deps.edn b/modules/drivers/sqlite/deps.edn index e4fd0f63911e02b2000b2ead1c35be758412dfff..8064aff04ae4b75d053242e123dfe3ec64260379 100644 --- a/modules/drivers/sqlite/deps.edn +++ b/modules/drivers/sqlite/deps.edn @@ -2,4 +2,4 @@ ["src" "resources"] :deps - {org.xerial/sqlite-jdbc {:mvn/version "3.25.2"}}} + {org.xerial/sqlite-jdbc {:mvn/version "3.36.0.1"}}} diff --git a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj index 88d23de96edafb2fe99a83af7561599b63c2fcf2..92bed2e8b41e7c3332474a776ad4c21c64c2cf4f 100644 --- a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj +++ b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj @@ -376,14 +376,30 @@ (.close stmt) (throw e))))) -;; (.getObject rs i LocalDate) doesn't seem to work, nor does `(.getDate)`; and it seems to be the case that -;; timestamps come back as `Types/DATE` as well? Fetch them as a String and then parse them +;; SQLite has no intrinsic date/time type. The sqlite-jdbc driver provides the following de-facto mappings: +;; DATE or DATETIME => Types/DATE (only if type is int or string) +;; TIMESTAMP => Types/TIMESTAMP (only if type is int) +;; The data itself can be stored either as +;; 1) integer (unix epoch) - this is "point in time", so no confusion about timezone +;; 2) float (julian days) - this is "point in time", so no confusion about timezone +;; 3) string (ISO8601) - zoned or unzoned depending on content, sqlite-jdbc always treat it as local time +;; Note that it is possible to store other invalid data in the column as SQLite does not perform any validation. +(defn- sqlite-handle-timestamp + [^ResultSet rs ^Integer i] + (let [obj (.getObject rs i)] + (cond + ;; For strings, use our own parser which is more flexible than sqlite-jdbc's and handles timezones correctly + (instance? String obj) (u.date/parse obj) + ;; For other types, fallback to sqlite-jdbc's parser + ;; Even in DATE column, it is possible to put DATETIME, so always treat as DATETIME + (some? obj) (t/local-date-time (.getTimestamp rs i))))) + (defmethod sql-jdbc.execute/read-column-thunk [:sqlite Types/DATE] [_ ^ResultSet rs _ ^Integer i] (fn [] - (try - (when-let [t (.getDate rs i)] - (t/local-date t)) - (catch Throwable _ - (when-let [s (.getString rs i)] - (u.date/parse s)))))) + (sqlite-handle-timestamp rs i))) + +(defmethod sql-jdbc.execute/read-column-thunk [:sqlite Types/TIMESTAMP] + [_ ^ResultSet rs _ ^Integer i] + (fn [] + (sqlite-handle-timestamp rs i))) diff --git a/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj b/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj index 6f71fe21da5148edd1258761ae7488072deb1a25..93b79643033a12c4a05b35b19bd866b2e65b8054 100644 --- a/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj +++ b/modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj @@ -142,3 +142,64 @@ :base-type :type/DateTime :database-position 0}}} (driver/describe-table driver db (Table (mt/id :timestamp_table))))))))))))) + +(deftest select-query-datetime + (mt/test-driver :sqlite + (let [db-name "datetime_test" + details (mt/dbdef->connection-details :sqlite :db {:database-name db-name})] + (doseq [stmt ["DROP TABLE IF EXISTS datetime_table;" + "CREATE TABLE datetime_table ( + test_case varchar, + col_timestamp timestamp, + col_date date, + col_datetime datetime);" + "INSERT INTO datetime_table + (test_case, col_timestamp, col_date, col_datetime) VALUES + ('epoch', 1629865104000, 1629849600000, 1629865104000), + ('iso8601-ms', '2021-08-25 04:18:24.111', null, '2021-08-25 04:18:24.111'), + ('iso8601-no-ms', '2021-08-25 04:18:24', null, '2021-08-25 04:18:24'), + ('iso8601-no-time', null, '2021-08-25', null), + ('null', null, null, null);"]] + (jdbc/execute! (sql-jdbc.conn/connection-details->spec :sqlite details) + [stmt])) + (mt/with-temp Database [db {:engine :sqlite :details (assoc details :dbname db-name)}] + (sync/sync-database! db) + ;; In SQLite, you can actually store any value in any date/timestamp column, + ;; let's test only values we'd reasonably run into. + ;; Caveat: TIMESTAMP stored as string doesn't get parsed and is returned as-is by the driver, + ;; some upper layer will handle it. + (mt/with-db db + (testing "select datetime stored as unix epoch" + (is (= [["2021-08-25T04:18:24Z" ; TIMESTAMP + "2021-08-25T00:00:00Z" ; DATE + "2021-08-25T04:18:24Z"]] ; DATETIME + (qp.test/rows + (mt/run-mbql-query :datetime_table + {:fields [$col_timestamp $col_date $col_datetime] + :filter [:= $test_case "epoch"]}))))) + (testing "select datetime stored as string with milliseconds" + (is (= [["2021-08-25 04:18:24.111" ; TIMESTAMP (raw string) + "2021-08-25T04:18:24.111Z"]] ; DATETIME + (qp.test/rows + (mt/run-mbql-query :datetime_table + {:fields [$col_timestamp $col_datetime] + :filter [:= $test_case "iso8601-ms"]}))))) + (testing "select datetime stored as string without milliseconds" + (is (= [["2021-08-25 04:18:24" ; TIMESTAMP (raw string) + "2021-08-25T04:18:24Z"]] ; DATETIME + (qp.test/rows + (mt/run-mbql-query :datetime_table + {:fields [$col_timestamp $col_datetime] + :filter [:= $test_case "iso8601-no-ms"]}))))) + (testing "select date stored as string without time" + (is (= [["2021-08-25T00:00:00Z"]] ; DATE + (qp.test/rows + (mt/run-mbql-query :datetime_table + {:fields [$col_date] + :filter [:= $test_case "iso8601-no-time"]}))))) + (testing "select NULL" + (is (= [[nil nil nil]] + (qp.test/rows + (mt/run-mbql-query :datetime_table + {:fields [$col_timestamp $col_date $col_datetime] + :filter [:= $test_case "null"]}))))))))))