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

stop rounding times #1172

Merged
merged 4 commits into from Feb 2, 2021
Merged

Conversation

shogo82148
Copy link
Contributor

Description

fixes #1121

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

@methane
Copy link
Member

methane commented Nov 27, 2020

https://dev.mysql.com/doc/refman/8.0/en/fractional-seconds.html

MySQL supports only milliseconds. Do we need to send nanoseconds?

@shogo82148
Copy link
Contributor Author

MySQL supports only milliseconds.

You mean microseconds?

At least, we need to send 100 nanoseconds (7 digits) precision.
because the DATETIME converter can handle them.

Some examples:

mysql> SELECT VERSION();
+-----------+
| VERSION() |
+-----------+
| 8.0.22    |
+-----------+
1 row in set (0.00 sec)

mysql> SELECT CONVERT('2020-11-27 10:00:00.1234564', DATETIME(6));
+-----------------------------------------------------+
| CONVERT('2020-11-27 10:00:00.1234564', DATETIME(6)) |
+-----------------------------------------------------+
| 2020-11-27 10:00:00.123456                          |
+-----------------------------------------------------+
1 row in set (0.01 sec)

mysql> SELECT CONVERT('2020-11-27 10:00:00.1234565', DATETIME(6));
+-----------------------------------------------------+
| CONVERT('2020-11-27 10:00:00.1234565', DATETIME(6)) |
+-----------------------------------------------------+
| 2020-11-27 10:00:00.123457                          |
+-----------------------------------------------------+
1 row in set (0.01 sec)

mysql> SET @@sql_mode = sys.list_add(@@sql_mode, 'TIME_TRUNCATE_FRACTIONAL');
Query OK, 0 rows affected (0.02 sec)

mysql> SELECT CONVERT('2020-11-27 10:00:00.1234564', DATETIME(6));
+-----------------------------------------------------+
| CONVERT('2020-11-27 10:00:00.1234564', DATETIME(6)) |
+-----------------------------------------------------+
| 2020-11-27 10:00:00.123456                          |
+-----------------------------------------------------+
1 row in set (0.01 sec)

mysql> SELECT CONVERT('2020-11-27 10:00:00.1234565', DATETIME(6));
+-----------------------------------------------------+
| CONVERT('2020-11-27 10:00:00.1234565', DATETIME(6)) |
+-----------------------------------------------------+
| 2020-11-27 10:00:00.123456                          |
+-----------------------------------------------------+
1 row in set (0.01 sec)

As you say, currently MySQL only supports up to microseconds.
However, there is no guarantee that it will not support nanoseconds in the future.
So, I think that we should send all digits we know.

@methane
Copy link
Member

methane commented Nov 28, 2020

I think you are right, but still feel it is a bit too much.

#1121 is really a bug. Round off 1.4449995 to 1.45 doesn't make sense at all.
Compared to it, behavior consistency between round off and round down is very minor. Should we send another 3 digits only for this minor consistency?

Let's check another connectors behavior.

@shogo82148
Copy link
Contributor Author

OK. I'll check another connectors behavior, too.

There is the issue about this, so we should discuss in #1121.
I'll convert this pull request to draft.
Let's continue discussion in #1121.

@shogo82148 shogo82148 marked this pull request as draft November 28, 2020 02:40
@methane
Copy link
Member

methane commented Jan 6, 2021

I think this pull request is good for v1.6. @shogo82148 would you merge master branch to run CI?

@shogo82148 shogo82148 marked this pull request as ready for review January 31, 2021 13:26
@shogo82148
Copy link
Contributor Author

@methane I did!

@methane
Copy link
Member

methane commented Feb 1, 2021

Is it safe to trim trailing 0s?

@shogo82148
Copy link
Contributor Author

Maybe, it is.

@shogo82148
Copy link
Contributor Author

It looks that starting MySQL server fails on macOS. it is not due to my pull request.

https://github.com/go-sql-driver/mysql/pull/1172/checks?check_run_id=1806566232

To have launchd start mysql now and restart at login:
  brew services start mysql
Or, if you don't want/need a background service you can just run:
  mysql.server start
Starting MySQL
/usr/local/Cellar/mysql/8.0.23/bin/mysqld_safe: line 653: /usr/local/var/mysql/Mac-1612182949676.local.err: No such file or directory
Logging to '/usr/local/var/mysql/Mac-1612182949676.local.err'.
/usr/local/Cellar/mysql/8.0.23/bin/mysqld_safe: line 144: /usr/local/var/mysql/Mac-1612182949676.local.err: No such file or directory
/usr/local/Cellar/mysql/8.0.23/bin/mysqld_safe: line 199: /usr/local/var/mysql/Mac-1612182949676.local.err: No such file or directory
/usr/local/Cellar/mysql/8.0.23/bin/mysqld_safe: line 916: /usr/local/var/mysql/Mac-1612182949676.local.err: No such file or directory
/usr/local/Cellar/mysql/8.0.23/bin/mysqld_safe: line 144: /usr/local/var/mysql/Mac-1612182949676.local.err: No such file or directory
. ERROR! The server quit without updating PID file (/usr/local/var/mysql/Mac-1612182949676.local.pid).
Error: Process completed with exit code 1.

@shogo82148
Copy link
Contributor Author

Thank you for your review!

@shogo82148 shogo82148 merged commit fe2230a into go-sql-driver:master Feb 2, 2021
@shogo82148 shogo82148 deleted the stop-rounding-time branch February 2, 2021 04:30
andygrunwald added a commit to andygrunwald/mysql that referenced this pull request Jul 31, 2021
* master: (93 commits)
  return unsigned in database type name when necessary (go-sql-driver#1238)
  add an invalid DSN test case (go-sql-driver#1235)
  refactoring (*textRows).readRow in a more clear way (go-sql-driver#1230)
  use utf8mb4 instead of utf8 in TestCharset (go-sql-driver#1228)
  improve readability follows go-staticcheck (go-sql-driver#1227)
  support Is comparison on MySQLError (go-sql-driver#1210)
  Wording correction in README (go-sql-driver#1218)
  noCopy implements sync.Locker (go-sql-driver#1216)
  Fix readme: MaxIdle is same or less than MaxOpen (go-sql-driver#1215)
  Drop support of Go 1.12 (go-sql-driver#1211)
  Release v1.6.0 (go-sql-driver#1197)
  add Go 1.16 to the build matrix (go-sql-driver#1198)
  Implement driver.Validator interface (go-sql-driver#1174)
  handling decoding pem error (go-sql-driver#1192)
  stop rounding times (go-sql-driver#1172)
  improve GitHub Actions workflows (go-sql-driver#1190)
  Move tests from Travis to Actions (go-sql-driver#1183)
  Fix go vet error (go-sql-driver#1173)
  README: Make usage code more friendly (go-sql-driver#1170)
  Fix a broken link to cleartext client side plugin (go-sql-driver#1165)
  ...
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.

Driver should not round time
2 participants