Changes date parsing to return String if not a valid JS Date #430

Merged
merged 1 commit into from Mar 28, 2013

Conversation

Projects
None yet
4 participants
@dresende
Collaborator

dresende commented Mar 28, 2013

Related to #429

@felixge

This comment has been minimized.

Show comment Hide comment
@felixge

felixge Mar 28, 2013

Collaborator

This is a good idea for the use case in #429, and we should support it since MySQL supports these partial date values. But IMO this should be disabled by default, as it could easily cause exceptions / other issues in programs that always expect to be given a date object. So I'm +1 on this, but behind some option that is disabled by default.

Collaborator

felixge commented Mar 28, 2013

This is a good idea for the use case in #429, and we should support it since MySQL supports these partial date values. But IMO this should be disabled by default, as it could easily cause exceptions / other issues in programs that always expect to be given a date object. So I'm +1 on this, but behind some option that is disabled by default.

@0b10011

This comment has been minimized.

Show comment Hide comment
@0b10011

0b10011 Mar 28, 2013

I'm against this being disabled by default, since MySQL has this option enabled by default. And, in cases where the date is invalid, it already returns a string and not a date---albeit "Invalid Date", rather than the actual date. However, not sending the date would be rather confusing to new users who expect the MySQL defaults to work out of the box (like I did). Also, those who expect an actual date are probably only allowing valid dates to be entered into the database, so they won't come across this edge-case.

0b10011 commented Mar 28, 2013

I'm against this being disabled by default, since MySQL has this option enabled by default. And, in cases where the date is invalid, it already returns a string and not a date---albeit "Invalid Date", rather than the actual date. However, not sending the date would be rather confusing to new users who expect the MySQL defaults to work out of the box (like I did). Also, those who expect an actual date are probably only allowing valid dates to be entered into the database, so they won't come across this edge-case.

@dresende

This comment has been minimized.

Show comment Hide comment
@dresende

dresende Mar 28, 2013

Collaborator

I agree with @bfrohs, it makes sense.

Collaborator

dresende commented Mar 28, 2013

I agree with @bfrohs, it makes sense.

@felixge

This comment has been minimized.

Show comment Hide comment
@felixge

felixge Mar 28, 2013

Collaborator

The older I get, the more I prefer static typing. That being said, JS and MySql both seem to be in the "loose is better" camp ... so I guess it makes sense. So ignore me on this and go ahead with the patch. But don't come yelling at me when your apps blow up / end up being insecure : ).

Collaborator

felixge commented Mar 28, 2013

The older I get, the more I prefer static typing. That being said, JS and MySql both seem to be in the "loose is better" camp ... so I guess it makes sense. So ignore me on this and go ahead with the patch. But don't come yelling at me when your apps blow up / end up being insecure : ).

@dresende

This comment has been minimized.

Show comment Hide comment
@dresende

dresende Mar 28, 2013

Collaborator

No worries. We're here to help people learn to define their column types and know the limitations and pitfalls :)

Collaborator

dresende commented Mar 28, 2013

No worries. We're here to help people learn to define their column types and know the limitations and pitfalls :)

dresende added a commit that referenced this pull request Mar 28, 2013

Merge pull request #430 from felixge/dates_as_strings
Changes date parsing to return String if not a valid JS Date

@dresende dresende merged commit 8ba067f into master Mar 28, 2013

1 check passed

default The Travis build passed
Details

@dresende dresende deleted the dates_as_strings branch Mar 28, 2013

@chriso

This comment has been minimized.

Show comment Hide comment
@chriso

chriso May 2, 2013

This is breaking lots of code for us, we now have to check if each date column actually returns a date object or not :(

This is breaking lots of code for us, we now have to check if each date column actually returns a date object or not :(

This comment has been minimized.

Show comment Hide comment
@dresende

dresende May 3, 2013

Collaborator

This should only happen on invalid dates. How did you treat invalid dates before?

Collaborator

dresende replied May 3, 2013

This should only happen on invalid dates. How did you treat invalid dates before?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment