Skip to content
Snippets Groups Projects
Unverified Commit 6b869c4b authored by dpsutton's avatar dpsutton Committed by GitHub
Browse files

Fix misspelling and duplication in h2 unix timestamp conversions (#16631)

No tests caught this because of compounding errors on top of a generic
solution that already worked:

for h2, defined unix-timestamp->honeysql on [:h2 :millisecond] but
:millisecond is not a valid dispatch key, it is always
milliseconds (plural). And it was duplicated forgetting to use
:microseconds.

But this all worked because there are defaults in the
sql/query_processor

```clojure
(defmethod unix-timestamp->honeysql [:sql :milliseconds]
  [driver _ expr]
  (unix-timestamp->honeysql driver :seconds (hx// expr 1000)))

(defmethod unix-timestamp->honeysql [:sql :microseconds]
  [driver _ expr]
  (unix-timestamp->honeysql driver :seconds (hx// expr 1000000)))

```

so h2 was always using these fallbacks with the division:

```sql
"SELECT \"PUBLIC\".\"INCIDENTS\".\"ID\" AS \"ID\",
        \"PUBLIC\".\"INCIDENTS\".\"SEVERITY\" AS \"SEVERITY\",
        timestampadd('second', (\"PUBLIC\".\"INCIDENTS\".\"TIMESTAMP\"/1000000),--note division
                     timestamp '1970-01-01T00:00:00Z') AS \"TIMESTAMP\"
 FROM \"PUBLIC\".\"INCIDENTS\" LIMIT 1048575"
 ```

rather than the h2 optimized form:

```sql
"SELECT \"PUBLIC\".\"INCIDENTS\".\"ID\" AS \"ID\",
        \"PUBLIC\".\"INCIDENTS\".\"SEVERITY\" AS \"SEVERITY\",
        timestampadd('microsecond', --note microseconds
                     \"PUBLIC\".\"INCIDENTS\".\"TIMESTAMP\",
                     timestamp '1970-01-01T00:00:00Z') AS
                     \"TIMESTAMP\"
FROM \"PUBLIC\".\"INCIDENTS\" LIMIT 1048575"
```

The end result is that the defmethods were never called and the
default was used. So a bug, but not one that was visible. Good catch
jeff
parent e253d10c
No related branches found
No related tags found
No related merge requests found
......@@ -140,10 +140,10 @@
(defmethod sql.qp/unix-timestamp->honeysql [:h2 :seconds] [_ _ expr]
(add-to-1970 expr "second"))
(defmethod sql.qp/unix-timestamp->honeysql [:h2 :millisecond] [_ _ expr]
(defmethod sql.qp/unix-timestamp->honeysql [:h2 :milliseconds] [_ _ expr]
(add-to-1970 expr "millisecond"))
(defmethod sql.qp/unix-timestamp->honeysql [:h2 :millisecond] [_ _ expr]
(defmethod sql.qp/unix-timestamp->honeysql [:h2 :microseconds] [_ _ expr]
(add-to-1970 expr "microsecond"))
(defmethod sql.qp/cast-temporal-string [:h2 :Coercion/YYYYMMDDHHMMSSString->Temporal]
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment