-
Notifications
You must be signed in to change notification settings - Fork 550
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
Fix zero-datetime in statement result raises ArgumentError #1054
base: master
Are you sure you want to change the base?
Conversation
ext/mysql2/result.c
Outdated
if (seconds == 0) { | ||
val = Qnil; | ||
} else { | ||
if (seconds < MYSQL2_MIN_TIME || seconds > MYSQL2_MAX_TIME) { // use DateTime instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be equivalent to move this up as } else if (seconds...) {
, no need to indent the entire if block.
I didn't get this one into the 0.5.3 release, but I want to plan for whether to include it in the next release or not. If you'd be so kind as to make the requested change to the if block? |
Thanks for your advice! |
d4740d3
to
946532d
Compare
Hmm... you are exactly. |
I am confused why MySQL doesn't return NULL as the value type in this case, and is instead giving back an all-zeroes date value. It seems like the fix is papering over some quirk here. That said, it does appear that there's "no such thing" as an all-zeroes DATETIME, so mapping that to NULL appears consistent with MySQL's own behavior. Maybe the command-line client is actually doing this same paper-over? Here's my local session output:
|
Thanks for taking the review feedback, the changes look good. I just want to make sure I understand this under the hood before landing the PR. |
Possibly, would your MySQL be running with 'NO_ZERO_DATE' mode?
|
A zero-datetime value ('0000-00-00 00:00:00') is returned nil by Client#query,
but raises ArgumentError by Statement#execute.
I fixed Statement#execute to return nil correctly for zero-datetime.