Skip to content

Accept query options on Statement#execute - #912

Merged
sodabrew merged 1 commit into
brianmario:masterfrom
sodabrew:stmt_execute_options
Nov 30, 2017
Merged

Accept query options on Statement#execute#912
sodabrew merged 1 commit into
brianmario:masterfrom
sodabrew:stmt_execute_options

Conversation

@sodabrew

@sodabrew sodabrew commented Nov 28, 2017

Copy link
Copy Markdown
Collaborator

Instead of #640

@sodabrew sodabrew mentioned this pull request Nov 28, 2017
Closed
@sodabrew
sodabrew force-pushed the stmt_execute_options branch from 076bec1 to 6709b2b Compare November 28, 2017 05:21
@sodabrew

sodabrew commented Nov 28, 2017

Copy link
Copy Markdown
Collaborator Author

Turns out this is Ruby 2.0 syntax. Supporting the extra arguments in Ruby 1.9.3 requires a different approach to parsing out a trailing Hash in the argument list.

@sodabrew sodabrew added this to the 0.5.0 milestone Nov 28, 2017
tamird
tamird approved these changes Nov 29, 2017
View reviewed changes

@tamird tamird left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks better than #640!

Comment thread ext/mysql2/statement.c Outdated
return Qnil;
}

// Important to duplicate the hash, will receive merge! if extra opts

@tamird tamird Nov 29, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be possible to duplicate only conditionally, or use merge instead?

@sodabrew sodabrew Nov 29, 2017

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was duplicate previously - I should actually check if it gets duplicated again in rb_mysql_result_to_obj or if that method keeps the same options hash argument that was passed to it.

@sodabrew sodabrew Nov 29, 2017

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that rb_mysql_result_to_obj uses what is passed to it, rather than duplicating its own query options hash, so the comment here (and in client.c) should reflect that.

Comment thread lib/mysql2/statement.rb Outdated
def execute(*args)
Thread.handle_interrupt(::Mysql2::Util::TIMEOUT_ERROR_CLASS => :never) do
_execute(*args)
options = args.last.is_a?(Hash) ? args.pop : {}

@tamird tamird Nov 29, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider leaving a TODO that this should change when 1.9 is dropped.

@sodabrew sodabrew mentioned this pull request Nov 29, 2017
Closed
@sodabrew
sodabrew force-pushed the stmt_execute_options branch from 8627a1e to b65c3a2 Compare November 30, 2017 01:40
@sodabrew
sodabrew force-pushed the stmt_execute_options branch from b65c3a2 to 45cc574 Compare November 30, 2017 01:41
@sodabrew
sodabrew merged commit d203086 into brianmario:master Nov 30, 2017
@sodabrew
sodabrew deleted the stmt_execute_options branch November 30, 2017 01:53
jeremy added a commit to jeremy/mysql2 that referenced this pull request Dec 11, 2017
@jeremy
Merge branch 'master' into force_latin1_to_utf8
04d7daa 
* master:
  GitHub is HTTPS by default (brianmario#922)
  Fix wrong value of type YEAR on big endian environment. (brianmario#921)
  Avoid 0 byte allocation call when statement takes no parameters
  Small spec improvement for RSpec style
  Travis CI for Mac OS X builds use the same steps again
  More specific exception classes (brianmario#911)
  Travis CI matrix add Ruby 2.5 and switch Mac OS X to Ruby 2.3 (ala High Sierra)
  README text for query options on Statement#execute
  Accept query options on Statement#execute (brianmario#912)
  Drop 1.9.3 support (brianmario#913)
This was referenced Mar 21, 2018
Closed
Merged
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
Closed
Closed
This was referenced Mar 21, 2018
Merged
Closed
kamipo added a commit to kamipo/mysql2 that referenced this pull request Apr 4, 2018
@kamipo
Avoid GC run between mysql_stmt_execute and mysql_stmt_store_result
8ddfe12 
This fixes a regression caused by brianmario#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.

Fixes brianmario#956.
@kamipo kamipo mentioned this pull request Apr 4, 2018
Closed
sodabrew added a commit to sodabrew/mysql2 that referenced this pull request Apr 4, 2018
@sodabrew
Avoid GC run between mysql_stmt_execute and mysql_stmt_store_result
21d308f 
Thanks to @kamipo for diagnosing the problem and drafting the first PR.

This fixes a regression caused by brianmario#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 brianmario#956.
@sodabrew sodabrew mentioned this pull request Apr 4, 2018
Merged
sodabrew added a commit to sodabrew/mysql2 that referenced this pull request Apr 7, 2018
@sodabrew
Prevent command out of sync errors with Prepared Statements
216c92e 
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 added a commit to sodabrew/mysql2 that referenced this pull request Apr 7, 2018
@sodabrew
Prevent command out of sync errors with Prepared Statements
aaac1e5 
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 added a commit to sodabrew/mysql2 that referenced this pull request Apr 7, 2018
@sodabrew
Prevent command out of sync errors with Prepared Statements
515ee31 
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 added a commit that referenced this pull request Apr 7, 2018
@sodabrew
Prevent command out of sync errors with Prepared Statements (#958)
ed9aa84 
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 #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 #956, updates #957.
This was referenced Apr 11, 2018
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@tamird tamird tamird approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.5.0

Development

Successfully merging this pull request may close these issues.

2 participants

@sodabrew @tamird