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

Default date/time/timezone handling #33

Closed
magnusjt opened this issue Dec 18, 2018 · 8 comments
Closed

Default date/time/timezone handling #33

magnusjt opened this issue Dec 18, 2018 · 8 comments

Comments

@magnusjt
Copy link

Thanks for your work on this library! I haven't used it yet, but it looks interesting.

I'm wondering how dates, times, and timezones are handled by default in this project. In my experience, people tend to stick with the defaults (at least until the bugs start to show up), so it's important that they are sane. The existing mysql libs don't do a good job here. In my opinion, the defaults should be:

  1. Dates and datetimes are returned as string (as opposed to Date objects) since they do not have timezone info
  2. Timestamps should be parsed as UTC and returned as Date objects. This works because of the next point:
  3. The mysql session timezone should be set to UTC by default ("SET time_zone='+00:00';")

What do you think?

@knoxcard
Copy link
Contributor

knoxcard commented Apr 8, 2019

@magnusjt This issue has been fixed in Sequelize
(Recommend using Sequelize as your ORM moving forward)

sequelize/sequelize#10705

@rusher
Copy link
Collaborator

rusher commented May 7, 2019

@magnusjt
The current solution seems the most suitable in terms of ease of use but is based on having client and server configured according to the same timezone.

Handling client and server following different timezone is complex, the solution you describe would work well. The problem then would be letting user parsing timestamps and date string themselves, opening the door to mistakes.
I don't see any excellent solution, particulary because javascript doesn't have DATE / TIMESTAMP distinction, only Date object

@magnusjt
Copy link
Author

magnusjt commented May 7, 2019

@knoxcard I personally don't use sequelize, or any other orm's, so can't benefit from their fix

@rusher "The problem then would be letting user parsing timestamps and date string themselves, opening the door to mistakes." - The mistake would be to parse them using Date, imo..

The ease of use argument is fair, but will inevitably lead to bugs for many people. Maybe it's just a question of making this clear in the documentation, and easy to configure for different time zone on client and server?

@rusher
Copy link
Collaborator

rusher commented May 7, 2019

Documentation never hurt.
I'll do that shortly, then post here reference for review/ideas.

@knoxcard
Copy link
Contributor

knoxcard commented May 9, 2019

Guess this issue can be closed now, after the most recent release?

@rusher
Copy link
Collaborator

rusher commented May 9, 2019

better to keep it open as a reminder !

@knoxcard
Copy link
Contributor

knoxcard commented Jun 8, 2019

should we carry this over to JIRA and merge it with the other (similar) issue?

(Think this has been resolved already?)

@rusher
Copy link
Collaborator

rusher commented Jun 14, 2019

closing, not an issue, but task, created as https://jira.mariadb.org/browse/CONJS-82

@rusher rusher closed this as completed Jun 14, 2019
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

No branches or pull requests

3 participants