Set default timezone to 'Z' fixes #360

[grncdr]

No description provided.

[dresende]

Default timezone should keep being local. People used to the previous behavior won't notice anything and people really wanting UTC in the database just have to set that option.

[dresende]

Regarding #360, if the test fails I have to check it.

[grncdr]

I understand wanting to remain backwards-compatible, but the current behaviour seems broken to me and this is an alpha release series.

When it comes to DATE columns with local timezones, the issue stems from limited timezone support in Javascript and MySQL,* and I wasn't able to find a simple workaround at the driver level. The closest I came was appending an empty time to dates before parsing when timezone == 'local' (e.g. the date '2012-05-11' becomes new Date('2012-05-11 00:00:00')), but that still causes issues because you end up with a timestamp that is 24 - utcOffsetInHours different than where you started, and depending on the timezone used to ultimately display the date, you can end up off by a day again.

Another workaround to consider is for the driver to use it's own Javascript constructor for dates (not datetimes or timestamps), creating objects that have no time part and ignore timezones completely, but that seems like a lot of incidental complexity to take on.

On the other hand, defaulting to UTC prevents the issue, and users who are willing to manage the quirks in their application can still opt-in to using local.

* which is why most people encourage storing only UTC in MySQL

[dresende]

In DATE columns, the module already should append 00:00:00 to the date, at least when fetching. Isn't this working? Or it's just the other way around?

I understand your point and agree to some degree. I first merged timezone support as defaulting to UTC but then changed as @felixge requested.

[grncdr]

The driver only does it if the timezone isn't local. https://github.com/felixge/node-mysql/blob/master/lib/protocol/packets/RowDataPacket.js#L42

[dresende]

You're saying that if the code always adds '00:00:00' to DATE columns regardless of the timezone, it will work?

[dresende]

No, it doesn't work. The library must work with local, Z or any other timezone. If you find the solution for the problem (using any timezone) please tell me.

[grncdr]

If I had been able to find a solution this would be a different pull request 😉, but I'm fairly certain there is no driver-level workaround other than defining a new "DateWithoutTime" type (which I don't like for a number of reasons). The issue is that dates in MySQL are really just dates, and ignore times/timezones completely, whereas Javascript has no such concept natively.

[dresende]

Give me a few days before pulling this. I'm going to check your problem. You said you happened to get this error when your timezone is negative using local timezone, right?

[grncdr]

Yes. My local timezone is GMT-8 (PST).
On Dec 18, 2012 2:22 AM, "Diogo Resende" [email protected] wrote:

Give me a few days before pulling this. I'm going to check your problem.
You said you happened to get this error when your timezone is negative
using local timezone, right?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/361#issuecomment-11480767.

[dougwilson]

IMO the default timezone should say local, as the default timezone for JavaScript itself is local as well as MySQL has the default timezone as local by default as well. Yes, DATETIME columns do not store a timezone (and as such MySQL assumes they are only in the time zone set in the MySQL configuration) but TIMESTAMP columns do store the timezone (by the way of the server converting the time using the current timezone on storage and retrieval, so changing the time zone configuration will change the values of your TIMESTAMP columns). The default timezone should be the same as the default of MySQL which is local. This means if you don't touch any timezone configuration they are all using the same timezone, but if you changed your MySQL configuration to use UTC, then you should be manually changing this driver's timezone as well.

[pixelfreak]

+1 for defaulting to "Z". I always set all my machines in different environment/location to UTC to avoid timezone headache.

[nanek]

My local timezone is EST, and I can confirm the issue as @grncdr describes still exists. +1 for defaulting default timezone to "Z", or at least updating the readme that the default "local" version is broken.

[0:00:00 0 0/1 0.0% node test/integration/connection/test-type-casting.js]

assert.js:102
throw new assert.AssertionError({
^
AssertionError: got: "Thu May 10 2012 20:00:00 GMT-0400 (EDT)" (string) expected: "Fri May 11 2012 20:00:00 GMT-0400 (EDT)" (string) test: date
at /Users/kenan/projects/node-mysql/test/integration/connection/test-type-casting.js:115:14
at Array.forEach (native)
at process. (/Users/kenan/projects/node-mysql/test/integration/connection/test-type-casting.js:91:9)
at process.EventEmitter.emit (events.js:93:17)

[0:00:00 1 0/1 100.0% node test/integration/connection/test-type-casting.js]

[dougwilson]

Closing this. Until the MySQL server starts defaulting the timezone to UTC, this module will not.