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

Add `quoted_time` for truncating the date part of a time column value #24542

Merged
merged 1 commit into from Apr 14, 2016

Conversation

Projects
None yet
3 participants
@kamipo
Member

kamipo commented Apr 14, 2016

Context #24522.

TIME column on MariaDB doesn't ignore the date part of the string when
it coerces to time.

root@localhost [test] > CREATE TABLE `foos` (`id` int AUTO_INCREMENT PRIMARY KEY, `start` time(0), `finish` time(4)) ENGINE=InnoDB;
Query OK, 0 rows affected (0.02 sec)

root@localhost [test] > INSERT INTO `foos` (`start`, `finish`) VALUES ('2000-01-01 12:30:00', '2000-01-01 12:30:00.999900');
Query OK, 1 row affected, 2 warnings (0.00 sec)

Note (Code 1265): Data truncated for column 'start' at row 1
Note (Code 1265): Data truncated for column 'finish' at row 1
root@localhost [test] > SELECT `foos`.* FROM `foos`;
+----+----------+---------------+
| id | start    | finish        |
+----+----------+---------------+
|  1 | 12:30:00 | 12:30:00.9999 |
+----+----------+---------------+
1 row in set (0.00 sec)

root@localhost [test] > SELECT  `foos`.* FROM `foos` WHERE `foos`.`start` = '2000-01-01 12:30:00' LIMIT 1;
Empty set (0.00 sec)

root@localhost [test] > SELECT  `foos`.* FROM `foos` WHERE `foos`.`start` = '12:30:00' LIMIT 1;
+----+----------+---------------+
| id | start    | finish        |
+----+----------+---------------+
|  1 | 12:30:00 | 12:30:00.9999 |
+----+----------+---------------+
1 row in set (0.00 sec)

r? @vipulnsward

@vipulnsward

This comment has been minimized.

Member

vipulnsward commented Apr 14, 2016

r? @jeremy

@rails-bot rails-bot assigned jeremy and unassigned vipulnsward Apr 14, 2016

@@ -166,6 +170,7 @@ def _quote(value)
# BigDecimals need to be put in a non-normalized form and quoted.
when BigDecimal then value.to_s('F')
when Numeric, ActiveSupport::Duration then value.to_s
when Type::Time::Value then "'#{quoted_time(value)}'"
when Date, Time then "'#{quoted_date(value)}'"

This comment has been minimized.

@vipulnsward

vipulnsward Apr 14, 2016

Member

does when Time case happen now, on the next line?

This comment has been minimized.

@kamipo

kamipo Apr 14, 2016

Member

Yes, Type::DateTime#serialize returns a Time instance.

Add `quoted_time` for truncating the date part of a time column value
Context #24522.

TIME column on MariaDB doesn't ignore the date part of the string when
it coerces to time.

```
root@localhost [test] > CREATE TABLE `foos` (`id` int AUTO_INCREMENT PRIMARY KEY, `start` time(0), `finish` time(4)) ENGINE=InnoDB;
Query OK, 0 rows affected (0.02 sec)

root@localhost [test] > INSERT INTO `foos` (`start`, `finish`) VALUES ('2000-01-01 12:30:00', '2000-01-01 12:30:00.999900');
Query OK, 1 row affected, 2 warnings (0.00 sec)

Note (Code 1265): Data truncated for column 'start' at row 1
Note (Code 1265): Data truncated for column 'finish' at row 1
root@localhost [test] > SELECT `foos`.* FROM `foos`;
+----+----------+---------------+
| id | start    | finish        |
+----+----------+---------------+
|  1 | 12:30:00 | 12:30:00.9999 |
+----+----------+---------------+
1 row in set (0.00 sec)

root@localhost [test] > SELECT  `foos`.* FROM `foos` WHERE `foos`.`start` = '2000-01-01 12:30:00' LIMIT 1;
Empty set (0.00 sec)

root@localhost [test] > SELECT  `foos`.* FROM `foos` WHERE `foos`.`start` = '12:30:00' LIMIT 1;
+----+----------+---------------+
| id | start    | finish        |
+----+----------+---------------+
|  1 | 12:30:00 | 12:30:00.9999 |
+----+----------+---------------+
1 row in set (0.00 sec)
```

@kamipo kamipo force-pushed the kamipo:add_quoted_time branch to 28ec8c4 Apr 14, 2016

else
value
end
end

This comment has been minimized.

@jeremy

jeremy Apr 14, 2016

Member

Nice approach

This comment has been minimized.

@kamipo

@jeremy jeremy merged commit 53ab1ee into rails:master Apr 14, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@kamipo kamipo deleted the kamipo:add_quoted_time branch Apr 14, 2016

kamipo added a commit to kamipo/rails that referenced this pull request Apr 14, 2016

Should keep quoting behaivor of a time column value in sqlite3 adapter
Follow up to rails#24542.

In MySQL and PostgreSQL, a time column value is saved as ignored the
date part of it. But in SQLite3, a time column value is saved as a string.

We should keep previous quoting behavior in sqlite3 adapter.

```
sqlite> CREATE TABLE "foos" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "start" time(0), "finish" time(4));
sqlite> INSERT INTO "foos" ("start", "finish") VALUES ('2000-01-01 12:30:00', '2000-01-01 12:30:00.999900');
sqlite> SELECT "foos".* FROM "foos";
1|2000-01-01 12:30:00|2000-01-01 12:30:00.999900
sqlite> SELECT  "foos".* FROM "foos" WHERE "foos"."start" = '2000-01-01 12:30:00' LIMIT 1;
1|2000-01-01 12:30:00|2000-01-01 12:30:00.999900
sqlite> SELECT  "foos".* FROM "foos" WHERE "foos"."start" = '12:30:00' LIMIT 1;
sqlite>
```

vipulnsward added a commit to vipulnsward/rails that referenced this pull request Apr 15, 2016

jeremy added a commit that referenced this pull request Apr 15, 2016

kamipo added a commit to kamipo/rails that referenced this pull request Jul 18, 2017

Fix type casting a time for MariaDB
Context rails#24542.

Since 8ebe1f2, it has lost stripping date part for a time value. But I
confirmed it is still needed even if MariaDB 10.2.6 GA.

MariaDB 10.2.6, `prepared_statements: true`:

```
% ARCONN=mysql2 be ruby -w -Itest test/cases/time_precision_test.rb -n test_formatting_time_according_to_precision
Using mysql2
Run options: -n test_formatting_time_according_to_precision --seed 37614

F

Failure:
TimePrecisionTest#test_formatting_time_according_to_precision [test/cases/time_precision_test.rb:53]:
Failed assertion, no message given.

bin/rails test test/cases/time_precision_test.rb:46

Finished in 0.040279s, 24.8268 runs/s, 24.8268 assertions/s.

1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
```

pixeltrix added a commit that referenced this pull request Mar 11, 2018

Ensure that leading date is stripped by quoted_time
In #24542, quoted_time was introduced to strip the leading date
component for time columns because it was having a significant
effect in mariadb. However, it assumed that the date component
was always 2000-01-01 which isn't the case, especially if the
source wasn't another time column.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment