Skip to content

Commit

Permalink
Improve prepared statement ergonomics
Browse files Browse the repository at this point in the history
While working on Mysql2 prepared statement support in Rails,
I found them very hard to use. Most notably we sometimes need to
close them eagerly, but it's complicated by the fact that you can
only call `close` once, any subsequent call raises an error instead
of just nooping.

There's also no way to check if a statement was closed or not.
  • Loading branch information
byroot committed Dec 3, 2024
1 parent f0c5d11 commit 7b85b44
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 6 deletions.
3 changes: 2 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ group :test do
gem 'rspec', '~> 3.2'

# https://github.com/bbatsov/rubocop/pull/4789
gem 'rubocop', '~> 1.30', '>= 1.30.1' if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.6')
# 1.51 is the last version supporting Ruby 2.6
gem 'rubocop', '>= 1.30.1', '< 1.51' if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.6')
end

group :benchmarks, optional: true do
Expand Down
27 changes: 23 additions & 4 deletions ext/mysql2/statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ static VALUE intern_sec_fraction, intern_usec, intern_sec, intern_min, intern_ho
#define TypedData_Get_Struct(obj, type, ignore, sval) Data_Get_Struct(obj, type, sval)
#endif

#define GET_STATEMENT(self) \
#define RAW_GET_STATEMENT(self) \
mysql_stmt_wrapper *stmt_wrapper; \
TypedData_Get_Struct(self, mysql_stmt_wrapper, &rb_mysql_statement_type, stmt_wrapper); \

#define GET_STATEMENT(self) \
RAW_GET_STATEMENT(self) \
if (!stmt_wrapper->stmt) { rb_raise(cMysql2Error, "Invalid statement handle"); } \
if (stmt_wrapper->closed) { rb_raise(cMysql2Error, "Statement handle already closed"); }

Expand Down Expand Up @@ -603,12 +606,27 @@ static VALUE rb_mysql_stmt_affected_rows(VALUE self) {
* own prepared statement cache.
*/
static VALUE rb_mysql_stmt_close(VALUE self) {
GET_STATEMENT(self);
stmt_wrapper->closed = 1;
rb_thread_call_without_gvl(nogvl_stmt_close, stmt_wrapper, RUBY_UBF_IO, 0);
RAW_GET_STATEMENT(self);

if (!stmt_wrapper->closed) {
stmt_wrapper->closed = 1;
rb_thread_call_without_gvl(nogvl_stmt_close, stmt_wrapper, RUBY_UBF_IO, 0);
}

return Qnil;
}

/* call-seq:
* stmt.closed?
*
* Returns wheter or not the statement have been closed.
*/
static VALUE rb_mysql_stmt_closed_p(VALUE self) {
RAW_GET_STATEMENT(self);

return stmt_wrapper->closed ? Qtrue : Qfalse;
}

void init_mysql2_statement() {
cDate = rb_const_get(rb_cObject, rb_intern("Date"));
rb_global_variable(&cDate);
Expand All @@ -630,6 +648,7 @@ void init_mysql2_statement() {
rb_define_method(cMysql2Statement, "last_id", rb_mysql_stmt_last_id, 0);
rb_define_method(cMysql2Statement, "affected_rows", rb_mysql_stmt_affected_rows, 0);
rb_define_method(cMysql2Statement, "close", rb_mysql_stmt_close, 0);
rb_define_method(cMysql2Statement, "closed?", rb_mysql_stmt_closed_p, 0);

sym_stream = ID2SYM(rb_intern("stream"));

Expand Down
12 changes: 11 additions & 1 deletion spec/mysql2/statement_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require './spec/spec_helper'

RSpec.describe Mysql2::Statement do
RSpec.describe Mysql2::Statement do # rubocop:disable Metrics/BlockLength
before(:example) do
@client = new_client(encoding: "utf8")
end
Expand Down Expand Up @@ -720,5 +720,15 @@ def stmt_count
stmt.close
expect { stmt.execute }.to raise_error(Mysql2::Error, /Invalid statement handle/)
end

it 'should not raise if called multiple times' do
stmt = @client.prepare 'SELECT 1'
expect(stmt).to_not be_closed

3.times do
expect { stmt.close }.to_not raise_error
expect(stmt).to be_closed
end
end
end
end

0 comments on commit 7b85b44

Please sign in to comment.