Skip to content

Commit b641c0d

Browse files
authored
Merge pull request #5232 from sysown/fix/issue-4855
Fix issue 4855: Incorrect affected rows reporting for DDL queries
2 parents 0f719d3 + d188715 commit b641c0d

File tree

4 files changed

+212
-4
lines changed

4 files changed

+212
-4
lines changed

include/sqlite3db.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ extern void (*proxy_sqlite3_free)(void*);
4444
extern int (*proxy_sqlite3_status)(int op, int *pCurrent, int *pHighwater, int resetFlag);
4545
extern int (*proxy_sqlite3_status64)(int op, long long *pCurrent, long long *pHighwater, int resetFlag);
4646
extern int (*proxy_sqlite3_changes)(sqlite3*);
47+
extern long long (*proxy_sqlite3_total_changes64)(sqlite3*);
4748
extern int (*proxy_sqlite3_step)(sqlite3_stmt*);
4849
extern int (*proxy_sqlite3_config)(int, ...);
4950
extern int (*proxy_sqlite3_shutdown)(void);
@@ -93,6 +94,7 @@ int (*proxy_sqlite3_status)(int op, int *pCurrent, int *pHighwater, int resetFla
9394
int (*proxy_sqlite3_status64)(int op, long long *pCurrent, long long *pHighwater, int resetFlag);
9495

9596
int (*proxy_sqlite3_changes)(sqlite3*);
97+
long long (*proxy_sqlite3_total_changes64)(sqlite3*);
9698
int (*proxy_sqlite3_step)(sqlite3_stmt*);
9799
int (*proxy_sqlite3_config)(int, ...);
98100
int (*proxy_sqlite3_shutdown)(void);

lib/sqlite3db.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
#define USLEEP_SQLITE_LOCKED 100
1919

20-
2120
/**
2221
* @brief Constructor for the SQLite3_column class.
2322
*
@@ -342,6 +341,8 @@ bool SQLite3DB::execute_statement(const char *str, char **error, int *cols, int
342341
*cols = (*proxy_sqlite3_column_count)(statement);
343342
if (*cols==0) { // not a SELECT
344343
*resultset=NULL;
344+
// Get total changes before executing the statement
345+
long long total_changes_before = (*proxy_sqlite3_total_changes64)(db);
345346
do {
346347
rc=(*proxy_sqlite3_step)(statement);
347348
if (rc==SQLITE_LOCKED || rc==SQLITE_BUSY) { // the execution of the prepared statement failed because locked
@@ -353,7 +354,9 @@ bool SQLite3DB::execute_statement(const char *str, char **error, int *cols, int
353354
}
354355
} while (rc==SQLITE_LOCKED || rc==SQLITE_BUSY);
355356
if (rc==SQLITE_DONE) {
356-
*affected_rows=(*proxy_sqlite3_changes)(db);
357+
// Calculate affected rows as the difference in total changes
358+
long long total_changes_after = (*proxy_sqlite3_total_changes64)(db);
359+
*affected_rows = (int)(total_changes_after - total_changes_before);
357360
ret=true;
358361
} else {
359362
*error=strdup((*proxy_sqlite3_errmsg)(db));
@@ -372,7 +375,7 @@ bool SQLite3DB::execute_statement(const char *str, char **error, int *cols, int
372375

373376
/**
374377
* @brief Executes a SQL statement and returns the result set without parsing it.
375-
*
378+
*
376379
* @param str The SQL statement to execute.
377380
* @param error Pointer to a variable to store the error message.
378381
* @param cols Pointer to a variable to store the number of columns.
@@ -394,14 +397,18 @@ bool SQLite3DB::execute_statement_raw(const char *str, char **error, int *cols,
394397
*cols = (*proxy_sqlite3_column_count)(*statement);
395398
if (*cols==0) { // not a SELECT
396399
//*resultset=NULL;
400+
// Get total changes before executing the statement
401+
long long total_changes_before = (*proxy_sqlite3_total_changes64)(db);
397402
do {
398403
rc=(*proxy_sqlite3_step)(*statement);
399404
if (rc==SQLITE_LOCKED || rc==SQLITE_BUSY) { // the execution of the prepared statement failed because locked
400405
usleep(USLEEP_SQLITE_LOCKED);
401406
}
402407
} while (rc==SQLITE_LOCKED || rc==SQLITE_BUSY);
403408
if (rc==SQLITE_DONE) {
404-
*affected_rows=(*proxy_sqlite3_changes)(db);
409+
// Calculate affected rows as the difference in total changes
410+
long long total_changes_after = (*proxy_sqlite3_total_changes64)(db);
411+
*affected_rows = (int)(total_changes_after - total_changes_before);
405412
ret=true;
406413
} else {
407414
*error=strdup((*proxy_sqlite3_errmsg)(db));
@@ -1010,6 +1017,7 @@ void SQLite3DB::LoadPlugin(const char *plugin_name) {
10101017
proxy_sqlite3_status = NULL;
10111018
proxy_sqlite3_status64 = NULL;
10121019
proxy_sqlite3_changes = NULL;
1020+
proxy_sqlite3_total_changes64 = NULL;
10131021
proxy_sqlite3_step = NULL;
10141022
proxy_sqlite3_shutdown = NULL;
10151023
proxy_sqlite3_prepare_v2 = NULL;
@@ -1089,6 +1097,7 @@ void SQLite3DB::LoadPlugin(const char *plugin_name) {
10891097
proxy_sqlite3_status = sqlite3_status;
10901098
proxy_sqlite3_status64 = sqlite3_status64;
10911099
proxy_sqlite3_changes = sqlite3_changes;
1100+
proxy_sqlite3_total_changes64 = sqlite3_total_changes64;
10921101
proxy_sqlite3_step = sqlite3_step;
10931102
proxy_sqlite3_shutdown = sqlite3_shutdown;
10941103
proxy_sqlite3_prepare_v2 = sqlite3_prepare_v2;
@@ -1118,6 +1127,7 @@ void SQLite3DB::LoadPlugin(const char *plugin_name) {
11181127
assert(proxy_sqlite3_status);
11191128
assert(proxy_sqlite3_status64);
11201129
assert(proxy_sqlite3_changes);
1130+
assert(proxy_sqlite3_total_changes64);
11211131
assert(proxy_sqlite3_step);
11221132
assert(proxy_sqlite3_shutdown);
11231133
assert(proxy_sqlite3_prepare_v2);

test/tap/groups/groups.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"mysql-reg_test_4716_single_semicolon-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ],
3232
"mysql-reg_test_4723_query_cache_stores_empty_result-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ],
3333
"mysql-reg_test_4867_query_rules-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ],
34+
"reg_test_4855_affected_rows_ddl-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ],
3435
"reg_test_5212_tcp_keepalive_warnings-t" : [ "default-g1" ],
3536
"mysql-set_transaction-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ],
3637
"mysql-sql_log_bin-error-t" : [ "default-g1","mysql-auto_increment_delay_multiplex=0-g1","mysql-multiplexing=false-g1","mysql-query_digests=0-g1","mysql-query_digests_keep_comment=1-g1" ],
Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
/*
2+
Copyright (c) 2025 ProxySQL
3+
4+
This program is free software; you can redistribute it and/or modify
5+
it under the terms of the GNU General Public License as published by
6+
the Free Software Foundation; version 2 of the License.
7+
8+
This program is distributed in the hope that it will be useful,
9+
but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+
GNU General Public License for more details.
12+
13+
You should have received a copy of the GNU General Public License
14+
along with this program; if not, write to the Free Software
15+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335 USA */
16+
17+
#include <stdlib.h>
18+
#include <stdio.h>
19+
#include <string.h>
20+
#include <vector>
21+
#include <string>
22+
#include "tap.h"
23+
#include "command_line.h"
24+
#include "utils.h"
25+
#include "mysql.h"
26+
27+
int main(int argc, char** argv) {
28+
plan(14);
29+
30+
CommandLine cl;
31+
if (cl.getEnv()) {
32+
diag("Failed to get the required environmental variables.");
33+
return exit_status();
34+
}
35+
36+
MYSQL *admin = mysql_init(NULL);
37+
ok(admin != NULL, "mysql_init() succeeded");
38+
if (!admin) {
39+
return exit_status();
40+
}
41+
42+
// Connect to ProxySQL Admin using command line arguments
43+
if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) {
44+
diag("Failed to connect to ProxySQL Admin: %s", mysql_error(admin));
45+
mysql_close(admin);
46+
return exit_status();
47+
}
48+
ok(true, "Connected to ProxySQL Admin");
49+
50+
// Clean up any existing test tables
51+
mysql_query(admin, "DROP TABLE IF EXISTS test_table_4855");
52+
mysql_query(admin, "DROP TABLE IF EXISTS test_table_4855_2");
53+
54+
// Test 1: Run a DDL query - should return 0 affected rows (this was the bug)
55+
if (mysql_query(admin, "CREATE TABLE test_table_4855 (id INT PRIMARY KEY, name VARCHAR(255))")) {
56+
diag("Failed to execute CREATE TABLE query: %s", mysql_error(admin));
57+
mysql_close(admin);
58+
return exit_status();
59+
}
60+
61+
my_ulonglong affected_rows = mysql_affected_rows(admin);
62+
ok(affected_rows == 0, "CREATE TABLE returns 0 affected rows (bug fix verified): %llu", affected_rows);
63+
diag("CREATE TABLE query executed successfully");
64+
65+
// Test 2: Run a DML query that affects rows
66+
if (mysql_query(admin, "INSERT INTO test_table_4855 (id, name) VALUES (1, 'test1')")) {
67+
diag("Failed to execute INSERT query: %s", mysql_error(admin));
68+
mysql_close(admin);
69+
return exit_status();
70+
}
71+
72+
affected_rows = mysql_affected_rows(admin);
73+
ok(affected_rows == 1, "INSERT query returns 1 affected row: %llu", affected_rows);
74+
diag("INSERT query executed successfully");
75+
76+
// Test 3: Run another DML query
77+
if (mysql_query(admin, "INSERT INTO test_table_4855 (id, name) VALUES (2, 'test2')")) {
78+
diag("Failed to execute second INSERT query: %s", mysql_error(admin));
79+
mysql_close(admin);
80+
return exit_status();
81+
}
82+
83+
affected_rows = mysql_affected_rows(admin);
84+
ok(affected_rows == 1, "Second INSERT query returns 1 affected row: %llu", affected_rows);
85+
diag("Second INSERT query executed successfully");
86+
87+
// Test 4: Run another DDL query - should return 0 affected rows
88+
if (mysql_query(admin, "CREATE TABLE test_table_4855_2 (id INT PRIMARY KEY, value TEXT)")) {
89+
diag("Failed to execute second CREATE TABLE query: %s", mysql_error(admin));
90+
mysql_close(admin);
91+
return exit_status();
92+
}
93+
94+
affected_rows = mysql_affected_rows(admin);
95+
ok(affected_rows == 0, "Second CREATE TABLE returns 0 affected rows: %llu", affected_rows);
96+
diag("Second CREATE TABLE query executed successfully");
97+
98+
// Test 5: Run an UPDATE query - should return correct affected rows
99+
if (mysql_query(admin, "UPDATE test_table_4855 SET name = 'updated' WHERE id = 1")) {
100+
diag("Failed to execute UPDATE query: %s", mysql_error(admin));
101+
mysql_close(admin);
102+
return exit_status();
103+
}
104+
105+
affected_rows = mysql_affected_rows(admin);
106+
ok(affected_rows == 1, "UPDATE query returns 1 affected row: %llu", affected_rows);
107+
diag("UPDATE query executed successfully");
108+
109+
// Test 6: Run a DELETE query - should return correct affected rows
110+
if (mysql_query(admin, "DELETE FROM test_table_4855 WHERE id IN (1, 2)")) {
111+
diag("Failed to execute DELETE query: %s", mysql_error(admin));
112+
mysql_close(admin);
113+
return exit_status();
114+
}
115+
116+
affected_rows = mysql_affected_rows(admin);
117+
ok(affected_rows == 2, "DELETE query returns 2 affected rows: %llu", affected_rows);
118+
diag("DELETE query executed successfully");
119+
120+
// Test 7: Run DROP TABLE - should return 0 affected rows
121+
if (mysql_query(admin, "DROP TABLE test_table_4855")) {
122+
diag("Failed to execute DROP TABLE query: %s", mysql_error(admin));
123+
mysql_close(admin);
124+
return exit_status();
125+
}
126+
127+
affected_rows = mysql_affected_rows(admin);
128+
ok(affected_rows == 0, "DROP TABLE returns 0 affected rows: %llu", affected_rows);
129+
diag("DROP TABLE query executed successfully");
130+
131+
// Test 8: Run another DDL to verify the fix again (ALTER TABLE)
132+
if (mysql_query(admin, "ALTER TABLE test_table_4855_2 ADD COLUMN extra INTEGER")) {
133+
diag("Failed to execute ALTER TABLE query: %s", mysql_error(admin));
134+
mysql_close(admin);
135+
return exit_status();
136+
}
137+
138+
affected_rows = mysql_affected_rows(admin);
139+
ok(affected_rows == 0, "ALTER TABLE returns 0 affected rows: %llu", affected_rows);
140+
diag("ALTER TABLE query executed successfully");
141+
142+
// Test 9: Test with comments followed by DDL (this was mentioned in the issue)
143+
if (mysql_query(admin, "/* This is a comment */ CREATE TABLE test_table_4855 (id INT PRIMARY KEY)")) {
144+
diag("Failed to execute CREATE TABLE with comment: %s", mysql_error(admin));
145+
mysql_close(admin);
146+
return exit_status();
147+
}
148+
149+
affected_rows = mysql_affected_rows(admin);
150+
ok(affected_rows == 0, "CREATE TABLE with comment returns 0 affected rows: %llu", affected_rows);
151+
diag("CREATE TABLE with comment executed successfully");
152+
153+
// Test 10: Insert data and run SELECT to verify normal operation
154+
if (mysql_query(admin, "INSERT INTO test_table_4855 (id) VALUES (1), (2), (3)")) {
155+
diag("Failed to execute INSERT with multiple values: %s", mysql_error(admin));
156+
mysql_close(admin);
157+
return exit_status();
158+
}
159+
160+
affected_rows = mysql_affected_rows(admin);
161+
ok(affected_rows == 3, "INSERT with multiple values returns 3 affected rows: %llu", affected_rows);
162+
diag("INSERT with multiple values executed successfully");
163+
164+
// Test 11: Run VACUUM - should return 0 affected rows
165+
if (mysql_query(admin, "VACUUM")) {
166+
diag("Failed to execute VACUUM query: %s", mysql_error(admin));
167+
mysql_close(admin);
168+
return exit_status();
169+
}
170+
171+
affected_rows = mysql_affected_rows(admin);
172+
ok(affected_rows == 0, "VACUUM returns 0 affected rows: %llu", affected_rows);
173+
diag("VACUUM query executed successfully");
174+
175+
// Test 12: Clean up - DROP remaining tables individually
176+
if (mysql_query(admin, "DROP TABLE IF EXISTS test_table_4855")) {
177+
diag("Failed to execute final DROP TABLE: %s", mysql_error(admin));
178+
mysql_close(admin);
179+
return exit_status();
180+
}
181+
182+
affected_rows = mysql_affected_rows(admin);
183+
ok(affected_rows == 0, "Final DROP TABLE returns 0 affected rows: %llu", affected_rows);
184+
diag("Final DROP TABLE query executed successfully");
185+
186+
// Additional cleanup without test assertion
187+
if (mysql_query(admin, "DROP TABLE IF EXISTS test_table_4855_2")) {
188+
diag("Failed to execute second DROP TABLE: %s", mysql_error(admin));
189+
mysql_close(admin);
190+
return exit_status();
191+
}
192+
diag("Additional cleanup DROP TABLE executed");
193+
mysql_close(admin);
194+
return exit_status();
195+
}

0 commit comments

Comments
 (0)