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

Prevent command out of sync errors with Prepared Statements #958

Conversation

sodabrew
Copy link
Collaborator

@sodabrew sodabrew commented Apr 4, 2018

Thanks to @kamipo for diagnosing the problem and drafting the first PR #957

This fixes a regression caused by #912 due to calling rb_funcall between mysql_stmt_execute and mysql_stmt_store_result, it will cause mysql_stmt_close to be called in wrong order.

In further testing, rb_hash_aref can also call rb_funcall if the query_options hash has a default_proc set, so adding a stronger comment to remind future maintainers.

Fixes #956.

@sodabrew
Copy link
Collaborator Author

sodabrew commented Apr 4, 2018

From the land of you must be f kidding me, prepared statement cursors disable microsecond timestamps. https://bugs.mysql.com/bug.php?id=86818

This is fixed in MariaDB and broken in MySQL 5.5, 5.6. Meanwhile, MySQL 5.7 and 8.0 just hang when cursors are enabled.


  1) Mysql2::Statement should prepare DateTime values with microseconds
     Failure/Error: expect(result.first['a'].strftime('%F %T.%5N %z')).to eql(now.strftime('%F %T.%5N %z'))

       expected: "2018-04-04 06:54:50.60401 -0700"
            got: "2018-04-04 06:54:51.00000 -0700"

       (compared using eql?)
     # ./spec/mysql2/statement_spec.rb:172:in `block (2 levels) in <top (required)>'

  2) Mysql2::Statement should prepare Time values with microseconds
     Failure/Error: expect(result.first['a'].strftime('%F %T.%5N %z')).to eql(now.strftime('%F %T.%5N %z'))

       expected: "2018-04-04 06:54:50.63292 -0700"
            got: "2018-04-04 06:54:51.00000 -0700"

       (compared using eql?)
     # ./spec/mysql2/statement_spec.rb:164:in `block (2 levels) in <top (required)>'

@sodabrew sodabrew force-pushed the avoid_rb_funcall_between_mysql_api_calls branch from 67c1795 to 216c92e Compare April 7, 2018 01:44
@sodabrew sodabrew changed the title Avoid GC run between mysql_stmt_execute and mysql_stmt_store_result Prevent command out of sync errors with Prepared Statements Apr 7, 2018
@sodabrew sodabrew force-pushed the avoid_rb_funcall_between_mysql_api_calls branch from 216c92e to aaac1e5 Compare April 7, 2018 02:07
This commit fixes two error scenarios. The first is to avoid GC runs between
`mysql_stmt_execute` and `mysql_stmt_store_result`. This fixes a regression
caused by brianmario#912 due to calling `rb_funcall` between `mysql_stmt_execute` and
`mysql_stmt_store_result`. The error in this case is:
  Commands out of sync; you can't run this command now

Thanks to @kamipo for diagnosing the problem and drafting the first PR.

The second problem is that in streaming mode, rows are returned to Ruby space
one at a time, so garbage collection will naturally occur at any time. By
requesting a cursor into the result set, other MySQL commands can be sent on
the wire between row fetches. The error in this case is:
  Row retrieval was canceled by mysql_stmt_close

Fixes brianmario#956, updates brianmario#957.
@sodabrew sodabrew force-pushed the avoid_rb_funcall_between_mysql_api_calls branch from aaac1e5 to 515ee31 Compare April 7, 2018 02:13
@sodabrew sodabrew merged commit ed9aa84 into brianmario:master Apr 7, 2018
@sodabrew sodabrew deleted the avoid_rb_funcall_between_mysql_api_calls branch April 7, 2018 03:00
@sodabrew sodabrew added this to the 0.5.1 milestone Apr 7, 2018
jeremy added a commit to jeremy/mysql2 that referenced this pull request Mar 5, 2019
* upstream/master:
  Expose windows client authentication (brianmario#1018)
  Fix code snippet (brianmario#1002)
  Add CentOS on Travis CI. (brianmario#989)
  Bump version to 0.5.2
  Travis apt-get update for MySQL 5.5 install
  Updating the mysql2_mysql_enc_to_rb conversion table to 8.0 List (brianmario#976)
  Add default-libmysqlclient-dev to the likely packages list
  Bump version to 0.5.1
  Use the prepared statement performance schema if available (brianmario#960)
  README mysql2 0.5.x works with Rails 5.0.7, 5.1.6, and higher
  README be sure to read about the known limitations of prepared statements
  Add missing FREE_BINDS to prepared statement streaming error case (brianmario#958)
  Fix with --with-mysql-dir (brianmario#952)
  Prevent command out of sync errors with Prepared Statements (brianmario#958)
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.

Random failures using prepared statements
1 participant