Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added unix-timestamp->timestamp function. #21

Merged
merged 1 commit into from
Oct 31, 2019
Merged

Added unix-timestamp->timestamp function. #21

merged 1 commit into from
Oct 31, 2019

Conversation

nicolasterral
Copy link

@nicolasterral nicolasterral commented Oct 29, 2019

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
Copy link
Owner

dacort commented Oct 31, 2019

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
Copy link
Owner

dacort commented Oct 31, 2019

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
Copy link
Owner

dacort commented Oct 31, 2019

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)."

@dacort dacort merged commit 17d4b0d into dacort:master Oct 31, 2019
@nicolasterral
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants