Skip to content

Commit

Permalink
Save affected_rows on the wrapper when reading results (#1383)
Browse files Browse the repository at this point in the history
Between fetching the result and accessing the affected_rows property, GC
might have been triggered and might have freed some Mysql2::Statement
objects. This calls mysql_stmt_close which resets the connection
affected_rows to -1, which in turn is treated as an error when calling
mysql_affected_rows.

```
client.query("SELECT 1")
client.affected_rows # raises Mysql2::Error
```

Note that the data type of mysql_affected_rows changed from my_ulonglong
to uint64_t starting with MySQL 8.0. Older versions should still work, though.
  • Loading branch information
byroot authored Dec 2, 2024
1 parent f6a9b68 commit 2583661
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 11 deletions.
8 changes: 5 additions & 3 deletions ext/mysql2/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ static VALUE allocate(VALUE klass) {
wrapper->initialized = 0; /* will be set true after calling mysql_init */
wrapper->closed = 1; /* will be set false after calling mysql_real_connect */
wrapper->refcount = 1;
wrapper->affected_rows = -1;
wrapper->client = (MYSQL*)xmalloc(sizeof(MYSQL));

return obj;
Expand Down Expand Up @@ -669,6 +670,7 @@ static VALUE rb_mysql_client_async_result(VALUE self) {
wrapper->active_fiber = Qnil;
rb_raise_mysql2_error(wrapper);
}
wrapper->affected_rows = mysql_affected_rows(wrapper->client);

is_streaming = rb_hash_aref(rb_ivar_get(self, intern_current_query_options), sym_stream);
if (is_streaming == Qtrue) {
Expand Down Expand Up @@ -1155,12 +1157,12 @@ static VALUE rb_mysql_client_session_track(VALUE self, VALUE type) {
* if it was an UPDATE, DELETE, or INSERT.
*/
static VALUE rb_mysql_client_affected_rows(VALUE self) {
my_ulonglong retVal;
uint64_t retVal;
GET_CLIENT(self);

REQUIRE_CONNECTED(wrapper);
retVal = mysql_affected_rows(wrapper->client);
if (retVal == (my_ulonglong)-1) {
retVal = wrapper->affected_rows;
if (retVal == (uint64_t)-1) {
rb_raise_mysql2_error(wrapper);
}
return ULL2NUM(retVal);
Expand Down
1 change: 1 addition & 0 deletions ext/mysql2/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ typedef struct {
int initialized;
int refcount;
int closed;
uint64_t affected_rows;
MYSQL *client;
} mysql_client_wrapper;

Expand Down
38 changes: 30 additions & 8 deletions spec/mysql2/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ def run_gc
expect(@client).to respond_to(:last_id)
end

it "#last_id should return a Fixnum, the from the last INSERT/UPDATE" do
it "#last_id should return a Fixnum, from the last INSERT/UPDATE" do
expect(@client.last_id).to eql(0)
@client.query "INSERT INTO lastIdTest (blah) VALUES (1234)"
expect(@client.last_id).to eql(1)
Expand All @@ -1043,13 +1043,6 @@ def run_gc
expect(@client).to respond_to(:last_id)
end

it "#last_id should return a Fixnum, the from the last INSERT/UPDATE" do
@client.query "INSERT INTO lastIdTest (blah) VALUES (1234)"
expect(@client.affected_rows).to eql(1)
@client.query "UPDATE lastIdTest SET blah=4321 WHERE id=1"
expect(@client.affected_rows).to eql(1)
end

it "#last_id should handle BIGINT auto-increment ids above 32 bits" do
# The id column type must be BIGINT. Surprise: INT(x) is limited to 32-bits for all values of x.
# Insert a row with a given ID, this should raise the auto-increment state
Expand All @@ -1058,6 +1051,35 @@ def run_gc
@client.query "INSERT INTO lastIdTest (blah) VALUES (5001)"
expect(@client.last_id).to eql(5000000001)
end

it "#last_id isn't cleared by Statement#close" do
stmt = @client.prepare("INSERT INTO lastIdTest (blah) VALUES (1234)")

@client.query "INSERT INTO lastIdTest (blah) VALUES (1234)"
expect(@client.last_id).to eql(1)

stmt.close

expect(@client.last_id).to eql(1)
end

it "#affected_rows should return a Fixnum, from the last INSERT/UPDATE" do
@client.query "INSERT INTO lastIdTest (blah) VALUES (1234)"
expect(@client.affected_rows).to eql(1)
@client.query "UPDATE lastIdTest SET blah=4321 WHERE id=1"
expect(@client.affected_rows).to eql(1)
end

it "#affected_rows isn't cleared by Statement#close" do
stmt = @client.prepare("INSERT INTO lastIdTest (blah) VALUES (1234)")

@client.query "INSERT INTO lastIdTest (blah) VALUES (1234)"
expect(@client.affected_rows).to eql(1)

stmt.close

expect(@client.affected_rows).to eql(1)
end
end

it "should respond to #thread_id" do
Expand Down

0 comments on commit 2583661

Please sign in to comment.