search

dacort / metabase-athena-driver

An Amazon Athena driver for Metabase 0.32 and later
Apache License 2.0
225 stars 32 forks source link

Added unix-timestamp->timestamp function. #21

Closed nicolasterral closed 4 years ago

nicolasterral commented 4 years ago

Following the implementation of relative-date querying, we had issues running queries built with the editor when the DB model was set to Unix Timestamp.

Issue encountered : "No method in multimethod 'unix-timestamp->timestamp' for dispatch value: [:athena :seconds]"

I've just added the the unix-timestamp->timestamp function that we can find in many others drivers to resolve the problem.

dacort commented 4 years ago

Thanks, @nicolasterral and apologies for the inconvenience. Do you know if we need to make a similar method for milliseconds? I know Unix Timestamp values can be set to either in the Metabase admin.

dacort commented 4 years ago

A quick lein run driver-methods shows that we may need to? Will validate on my end.

unix-timestamp->timestamp [driver seconds-or-milliseconds field-or-value]
dacort commented 4 years ago

Ah, ok, after digging in a little more it looks like we don't need millisecond-specific function as well.

Per query_processor:

There is a default implementation for :milliseconds the recursively calls with :seconds and (expr / 1000)."

nicolasterral commented 4 years ago

Thanks, @nicolasterral and apologies for the inconvenience. Do you know if we need to make a similar method for milliseconds? I know Unix Timestamp values can be set to either in the Metabase admin.

No problem, we are actually doing tests before migrating our production env. from Metabase 0.28.6 (ricardoekm PR) to 0.33.4 with this driver version. I should have specified that the function addition was also working with milliseconds.

Thanks for the reactivity and your awesome work on the driver.