search

dresende / node-sql-query

NodeJS SQL query builder
122 stars 103 forks source link

Store all dates as UTC for SQLite #36

Open apoco opened 9 years ago

apoco commented 9 years ago

Dates are stored in SQLite as Unix timestamps. Dates are represented in JavaScript as Unix timestamps. Thus, there need not be any sort of conversions. Client-side timezones are irrelevant; they should only affect the client-side display of dates. The existing code is incorrect because it's storing timezone-offset dates as UTC dates (with the "Z" suffix).

This change simplifies the type conversion; a Date is stored as an ISO-8601 date string, which is understood by both JavaScript's Date constructor and SQLite.

dxg commented 9 years ago

Looks good, I'll just have to run this against ORM

apoco commented 9 years ago

Great. I have some changes locally that work in conjunction with this, but I hesitated creating a PR since it won't pass without this package's updates. Would it be helpful for me to put a PR together, or is this pretty straightforward?

Another option that occurred to me is maybe having a different data type for this, like isodate; that way, if there is anyone that is using the current behavior would not be affected by the change. What are your thoughts? I'm not in a big rush because I went with just using strings for now.

dxg commented 9 years ago

I've continued discussion in dresende/node-orm2#568 Feel free to open the pull request and write a comment about why the tests fail inside.