This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 4721373fd1 fix(c/driver/sqlite): only count rows_affected when
applicable (#4826)
4721373fd1 is described below
commit 4721373fd1626c23a67302b3472529b52e8e86bf
Author: David Li <[email protected]>
AuthorDate: Wed Sep 30 16:18:55 2026 -0700
fix(c/driver/sqlite): only count rows_affected when applicable (#4826)
sqlite3_total_changes isn't updated by certain DDL statements,
so don't count it to avoid reporting a stale count.
Depends on #4825.
Closes #4820.
Generated-by: GPT-6 Astra
---
c/driver/sqlite/sqlite.cc | 19 +++++-
c/driver/sqlite/sqlite_test.cc | 95 ++++++++++++++++++++++++++++++
c/driver/sqlite/validation/tests/sqlite.py | 1 +
3 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/c/driver/sqlite/sqlite.cc b/c/driver/sqlite/sqlite.cc
index 8d98a5f3aa..b24cff80f3 100644
--- a/c/driver/sqlite/sqlite.cc
+++ b/c/driver/sqlite/sqlite.cc
@@ -33,6 +33,17 @@
#include "driver/framework/status.h"
#include "driver/sqlite/statement_reader.h"
+#if SQLITE_VERSION_NUMBER < 3037000
+// Polyfill for older versions
+sqlite3_int64 sqlite3_total_changes64(sqlite3* db) {
+ return static_cast<sqlite3_int64>(sqlite3_total_changes(db));
+}
+
+sqlite3_int64 sqlite3_changes64(sqlite3* db) {
+ return static_cast<sqlite3_int64>(sqlite3_changes(db));
+}
+#endif
+
namespace adbc::sqlite {
using driver::Result;
@@ -1138,12 +1149,16 @@ class SqliteStatement : public
driver::Statement<SqliteStatement> {
}
}
+ const sqlite3_int64 total_changes = sqlite3_total_changes64(conn_);
while (sqlite3_step(stmt_) == SQLITE_ROW) {
output_rows++;
}
- if (sqlite3_column_count(stmt_) == 0) {
- changed_rows += sqlite3_changes(conn_);
+ // NOTE(apache/arrow-adbc#4820): only count sqlite3_total_changes when
+ // it actually changed from before
+ if (sqlite3_column_count(stmt_) == 0 &&
+ sqlite3_total_changes64(conn_) != total_changes) {
+ changed_rows += sqlite3_changes64(conn_);
}
if (!binder_.schema.release) break;
diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc
index d1ba96533b..32e96296a2 100644
--- a/c/driver/sqlite/sqlite_test.cc
+++ b/c/driver/sqlite/sqlite_test.cc
@@ -680,6 +680,101 @@ TEST_F(SqliteStatementTest, SqlIngestNameEscaping) {
ASSERT_EQ(3, rows_affected);
}
+TEST_F(SqliteStatementTest, RowsAffectedDdlAndDml) {
+ ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error),
+ adbc_validation::IsOkStatus(&error));
+
+ const std::vector<std::pair<const char*, int64_t>> queries = {
+ {"CREATE TABLE affected_rows (value INTEGER)", 0},
+ {"INSERT INTO affected_rows VALUES (1), (2), (3)", 3},
+ {"CREATE TABLE affected_rows_other (value INTEGER)", 0},
+ {"CREATE INDEX affected_rows_idx ON affected_rows(value)", 0},
+ {"DROP TABLE affected_rows_other", 0},
+ {"UPDATE affected_rows SET value = value + 10 WHERE value = 1", 1},
+ {"UPDATE affected_rows SET value = value + 10 WHERE value = 99", 0},
+ {"DELETE FROM affected_rows WHERE value = 2", 1},
+ {"DELETE FROM affected_rows WHERE value = 99", 0},
+ {"CREATE TABLE affected_rows_audit (value INTEGER)", 0},
+ {"CREATE TRIGGER affected_rows_trigger AFTER INSERT ON affected_rows "
+ "BEGIN INSERT INTO affected_rows_audit VALUES (new.value); END",
+ 0},
+ {"INSERT INTO affected_rows VALUES (4), (5), (6)", 3},
+ {"SELECT * FROM affected_rows_audit", 3},
+ {"DELETE FROM affected_rows", 5},
+ {"DROP INDEX affected_rows_idx", 0},
+ {"DROP TABLE affected_rows", 0},
+ {"DROP TABLE affected_rows_audit", 0},
+ };
+ for (const auto& [query, expected_rows] : queries) {
+ SCOPED_TRACE(query);
+ ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, query, &error),
+ adbc_validation::IsOkStatus(&error));
+ int64_t rows_affected = -1;
+ ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected,
&error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_EQ(expected_rows, rows_affected);
+ }
+}
+
+TEST_F(SqliteStatementTest, RowsAffectedBoundParameters) {
+ ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementSetSqlQuery(
+ &statement, "CREATE TABLE affected_rows_bound (value
INTEGER)", &error),
+ adbc_validation::IsOkStatus(&error));
+ int64_t rows_affected = -1;
+ ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected,
&error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_EQ(0, rows_affected);
+
+ auto bind_values = [&](std::vector<std::optional<int64_t>> values) {
+ adbc_validation::Handle<struct ArrowSchema> schema;
+ adbc_validation::Handle<struct ArrowArray> array;
+ struct ArrowError na_error = {};
+ ASSERT_THAT(
+ adbc_validation::MakeSchema(&schema.value, {{"value",
NANOARROW_TYPE_INT64}}),
+ adbc_validation::IsOkErrno());
+ ASSERT_THAT(adbc_validation::MakeBatch<int64_t>(&schema.value,
&array.value,
+ &na_error, values),
+ adbc_validation::IsOkErrno(&na_error));
+ ASSERT_THAT(AdbcStatementBind(&statement, &array.value, &schema.value,
&error),
+ adbc_validation::IsOkStatus(&error));
+ };
+
+ ASSERT_THAT(AdbcStatementSetSqlQuery(
+ &statement, "INSERT INTO affected_rows_bound VALUES (?)",
&error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementPrepare(&statement, &error),
+ adbc_validation::IsOkStatus(&error));
+ for (int execution = 0; execution < 2; execution++) {
+ ASSERT_NO_FATAL_FAILURE(bind_values({1, 2, 3}));
+ ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected,
&error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_EQ(3, rows_affected);
+ }
+
+ ASSERT_THAT(
+ AdbcStatementSetSqlQuery(
+ &statement, "UPDATE affected_rows_bound SET value = value + 10 WHERE
value = ?",
+ &error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(bind_values({1, 99, 3}));
+ ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected,
&error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_EQ(4, rows_affected);
+
+ ASSERT_THAT(AdbcStatementSetSqlQuery(
+ &statement, "DELETE FROM affected_rows_bound WHERE value =
?", &error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(bind_values({11, 99, 13}));
+ ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected,
&error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_EQ(4, rows_affected);
+
+ ASSERT_THAT(quirks()->DropTable(&connection, "affected_rows_bound", &error),
+ adbc_validation::IsOkStatus(&error));
+}
+
// -- SQLite Specific Tests ------------------------------------------
constexpr size_t kInferRows = 16;
diff --git a/c/driver/sqlite/validation/tests/sqlite.py
b/c/driver/sqlite/validation/tests/sqlite.py
index ae8a49eee4..24b40aa1e6 100644
--- a/c/driver/sqlite/validation/tests/sqlite.py
+++ b/c/driver/sqlite/validation/tests/sqlite.py
@@ -42,6 +42,7 @@ class SQLiteQuirks(model.DriverQuirks):
statement_get_parameter_schema=True,
statement_prepare=True,
statement_rows_affected=True,
+ statement_rows_affected_ddl=True,
current_catalog="main",
current_schema="",
secondary_catalog="secondary",
