From 7d1bf1c0a6316f3b7335b103e89bf0a3719cd443 Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Tue, 3 Dec 2024 10:07:58 +0100
Subject: [PATCH] Improve prepared statement ergonomics

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.
---
 Gemfile                       |  3 ++-
 ext/mysql2/statement.c        | 27 +++++++++++++++++++++++----
 spec/mysql2/statement_spec.rb | 16 +++++++++++++---
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/Gemfile b/Gemfile
index 7b4e1b8a7..f7a0900a0 100644
--- a/Gemfile
+++ b/Gemfile
@@ -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
diff --git a/ext/mysql2/statement.c b/ext/mysql2/statement.c
index 5506526ee..fa3b660cd 100644
--- a/ext/mysql2/statement.c
+++ b/ext/mysql2/statement.c
@@ -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"); }
 
@@ -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);
@@ -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"));
 
diff --git a/spec/mysql2/statement_spec.rb b/spec/mysql2/statement_spec.rb
index c3ebf9bb3..8c4d97e68 100644
--- a/spec/mysql2/statement_spec.rb
+++ b/spec/mysql2/statement_spec.rb
@@ -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
@@ -319,8 +319,8 @@ def stmt_count
     it "should throw an exception if we try to iterate twice when streaming is enabled" do
       result = @client.prepare("SELECT 1 UNION SELECT 2").execute(stream: true, cache_rows: false)
       expect do
-        result.each {}
-        result.each {}
+        result.to_a
+        result.to_a
       end.to raise_exception(Mysql2::Error)
     end
   end
@@ -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