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

Fix zero-datetime in statement result raises ArgumentError #1054

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gentlawk
Copy link

A zero-datetime value ('0000-00-00 00:00:00') is returned nil by Client#query,
but raises ArgumentError by Statement#execute.

client.prepare("SELECT CAST('0000-00-00 00:00:00' AS DATETIME) as test").execute.first
#=>
        6: from /home/gentlawk/work/ruby/mysql2/lib/mysql2/statement.rb:6:in `execute'
        5: from /home/gentlawk/work/ruby/mysql2/lib/mysql2/statement.rb:6:in `handle_interrupt'
        4: from /home/gentlawk/work/ruby/mysql2/lib/mysql2/statement.rb:7:in `block in execute'
        3: from /home/gentlawk/work/ruby/mysql2/lib/mysql2/statement.rb:7:in `_execute'
        2: from /home/gentlawk/work/ruby/mysql2/lib/mysql2/statement.rb:7:in `each'
        1: from /home/gentlawk/work/ruby/mysql2/lib/mysql2/statement.rb:7:in `civil'
ArgumentError (invalid date)

I fixed Statement#execute to return nil correctly for zero-datetime.

if (seconds == 0) {
val = Qnil;
} else {
if (seconds < MYSQL2_MIN_TIME || seconds > MYSQL2_MAX_TIME) { // use DateTime instead
Copy link
Collaborator

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.

@sodabrew
Copy link
Collaborator

sodabrew commented Dec 1, 2019

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?

@gentlawk
Copy link
Author

gentlawk commented Dec 6, 2019

Thanks for your advice!
I fixed the indent of if block.

@kamipo
Copy link
Contributor

kamipo commented Dec 6, 2019

Looks like there is no reason to change the permission 644 to 755.

image

@gentlawk
Copy link
Author

gentlawk commented Dec 6, 2019

Hmm... you are exactly.
I fixed these.

@sodabrew
Copy link
Collaborator

sodabrew commented Jan 8, 2020

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:

mysql> SELECT CAST('0000-00-00 00:00:00' AS DATETIME);
+-----------------------------------------+
| CAST('0000-00-00 00:00:00' AS DATETIME) |
+-----------------------------------------+
| NULL                                    |
+-----------------------------------------+
1 row in set, 1 warning (0.00 sec)

mysql> SELECT CAST('0000-00-00 00:00:01' AS DATETIME);
+-----------------------------------------+
| CAST('0000-00-00 00:00:01' AS DATETIME) |
+-----------------------------------------+
| 0000-00-00 00:00:01                     |
+-----------------------------------------+
1 row in set (0.00 sec)

mysql> PREPARE t FROM 'SELECT CAST(? AS DATETIME)';
Query OK, 0 rows affected (0.01 sec)
Statement prepared

mysql> SET @a = '0000-00-00 00:00:00';
Query OK, 0 rows affected (0.01 sec)

mysql> SET @b = '0000-00-00 00:00:01';
Query OK, 0 rows affected (0.00 sec)

mysql> EXECUTE t USING @a;
+---------------------+
| CAST(? AS DATETIME) |
+---------------------+
| NULL                |
+---------------------+
1 row in set, 1 warning (0.01 sec)

mysql> EXECUTE t USING @b;
+---------------------+
| CAST(? AS DATETIME) |
+---------------------+
| 0000-00-00 00:00:01 |
+---------------------+
1 row in set (0.00 sec)

@sodabrew sodabrew added this to the 0.5.4 milestone Jan 8, 2020
@sodabrew
Copy link
Collaborator

sodabrew commented Jan 8, 2020

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.

@gentlawk
Copy link
Author

gentlawk commented Jan 8, 2020

Possibly, would your MySQL be running with 'NO_ZERO_DATE' mode?
If sql_mode variable is 'NO_ZERO_DATE' MySQL returns NULL for '0000-00-00 00:00:00'.

mysql> SET @@sql_mode = 'NO_ZERO_DATE';
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> SELECT CAST('0000-00-00 00:00:00' AS DATETIME);
+-----------------------------------------+
| CAST('0000-00-00 00:00:00' AS DATETIME) |
+-----------------------------------------+
| NULL                                    |
+-----------------------------------------+
1 row in set, 1 warning (0.00 sec)

mysql> SELECT CAST('0000-00-00 00:00:01' AS DATETIME);
+-----------------------------------------+
| CAST('0000-00-00 00:00:01' AS DATETIME) |
+-----------------------------------------+
| 0000-00-00 00:00:01                     |
+-----------------------------------------+
1 row in set (0.00 sec)

mysql> SET @@sql_mode = '';
Query OK, 0 rows affected (0.00 sec)

mysql> SELECT CAST('0000-00-00 00:00:00' AS DATETIME);
+-----------------------------------------+
| CAST('0000-00-00 00:00:00' AS DATETIME) |
+-----------------------------------------+
| 0000-00-00 00:00:00                     |
+-----------------------------------------+
1 row in set (0.00 sec)

mysql> SELECT CAST('0000-00-00 00:00:01' AS DATETIME);
+-----------------------------------------+
| CAST('0000-00-00 00:00:01' AS DATETIME) |
+-----------------------------------------+
| 0000-00-00 00:00:01                     |
+-----------------------------------------+
1 row in set (0.00 sec)

@sodabrew sodabrew modified the milestones: 0.5.4, 0.6.0 Jun 3, 2022
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

Successfully merging this pull request may close these issues.

3 participants