From 17b773246b3379a56dd8084748ebcf960103e9a9 Mon Sep 17 00:00:00 2001 From: chrispader Date: Thu, 23 Jul 2026 16:36:56 +0200 Subject: [PATCH 1/2] fix: report zero rows affected for read-only commands --- .../specs/operations/executeBatch.spec.ts | 17 ++ .../cpp/importSqlFile.cpp | 10 +- .../cpp/operations.cpp | 253 ++++++++---------- .../cpp/operations.hpp | 3 +- .../cpp/sqliteExecuteBatch.cpp | 16 +- 5 files changed, 139 insertions(+), 160 deletions(-) diff --git a/example/tests/unit/specs/operations/executeBatch.spec.ts b/example/tests/unit/specs/operations/executeBatch.spec.ts index e20748c6..9ceca983 100644 --- a/example/tests/unit/specs/operations/executeBatch.spec.ts +++ b/example/tests/unit/specs/operations/executeBatch.spec.ts @@ -42,6 +42,23 @@ export default function registerExecuteBatchUnitTests() { ]) }) + it('reports zero rows affected for read-only commands', () => { + const id = chance.integer() + testDb.execute( + 'INSERT INTO "User" (id, name, age, networth) VALUES(?, ?, ?, ?)', + [id, chance.name(), chance.integer(), chance.floating()], + ) + + const result = testDb.executeBatch([ + { + query: 'SELECT * FROM User WHERE id = ?', + params: [id], + }, + ]) + + expect(result.rowsAffected).toBe(0) + }) + it('Async batch execute', async () => { const id1 = chance.integer() const name1 = chance.name() diff --git a/packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp b/packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp index 1ae63dd8..16857ad4 100644 --- a/packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp +++ b/packages/react-native-nitro-sqlite/cpp/importSqlFile.cpp @@ -17,15 +17,15 @@ SQLiteOperationResult importSqlFile(const std::string& dbName, const std::string try { int rowsAffected = 0; int commands = 0; - sqliteExecuteLiteral(dbName, "BEGIN EXCLUSIVE TRANSACTION"); + sqliteExecuteCommand(dbName, "BEGIN EXCLUSIVE TRANSACTION"); while (std::getline(sqFile, line, '\n')) { if (!line.empty()) { try { - SQLiteOperationResult result = sqliteExecuteLiteral(dbName, line); + SQLiteOperationResult result = sqliteExecuteCommand(dbName, line); rowsAffected += result.rowsAffected; commands++; } catch (NitroSQLiteException& e) { - sqliteExecuteLiteral(dbName, "ROLLBACK"); + sqliteExecuteCommand(dbName, "ROLLBACK"); sqFile.close(); throw NitroSQLiteException::CouldNotLoadFile(fileLocation, "Transaction was rolled back"); } @@ -33,11 +33,11 @@ SQLiteOperationResult importSqlFile(const std::string& dbName, const std::string } sqFile.close(); - sqliteExecuteLiteral(dbName, "COMMIT"); + sqliteExecuteCommand(dbName, "COMMIT"); return {.rowsAffected = rowsAffected, .commands = commands}; } catch (...) { sqFile.close(); - sqliteExecuteLiteral(dbName, "ROLLBACK"); + sqliteExecuteCommand(dbName, "ROLLBACK"); throw NitroSQLiteException(NitroSQLiteExceptionType::UnknownError, "Unexpected error. Transaction was rolled back"); } } else { diff --git a/packages/react-native-nitro-sqlite/cpp/operations.cpp b/packages/react-native-nitro-sqlite/cpp/operations.cpp index 289defe5..bd3f0226 100644 --- a/packages/react-native-nitro-sqlite/cpp/operations.cpp +++ b/packages/react-native-nitro-sqlite/cpp/operations.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -77,13 +78,13 @@ void sqliteCloseAll() { void sqliteAttachDb(const std::string& mainDBName, const std::string& docPath, const std::string& databaseToAttach, const std::string& alias) { /** - * There is no need to check if mainDBName is opened because sqliteExecuteLiteral will do that. + * There is no need to check if mainDBName is opened because sqliteExecuteCommand will do that. * */ std::string dbPath = get_db_path(databaseToAttach, docPath); std::string statement = "ATTACH DATABASE '" + dbPath + "' AS " + alias; try { - sqliteExecuteLiteral(mainDBName, statement); + sqliteExecuteCommand(mainDBName, statement); } catch (NitroSQLiteException& e) { throw NitroSQLiteException(NitroSQLiteExceptionType::UnableToAttachToDatabase, mainDBName + " was unable to attach another database: " + std::string(e.what())); @@ -92,12 +93,12 @@ void sqliteAttachDb(const std::string& mainDBName, const std::string& docPath, c void sqliteDetachDb(const std::string& mainDBName, const std::string& alias) { /** - * There is no need to check if mainDBName is opened because sqliteExecuteLiteral will do that. + * There is no need to check if mainDBName is opened because sqliteExecuteCommand will do that. * */ std::string statement = "DETACH DATABASE " + alias; try { - sqliteExecuteLiteral(mainDBName, statement); + sqliteExecuteCommand(mainDBName, statement); } catch (NitroSQLiteException& e) { throw NitroSQLiteException(NitroSQLiteExceptionType::UnableToAttachToDatabase, mainDBName + " was unable to detach database: " + std::string(e.what())); @@ -144,173 +145,135 @@ void bindStatement(sqlite3_stmt* statement, const SQLiteQueryParams& values) { } } -std::shared_ptr sqliteExecute(const std::string& dbName, const std::string& query, - const std::optional& params) { - if (dbMap.count(dbName) == 0) { - throw NitroSQLiteException::DatabaseNotOpen(dbName); - } - - auto db = dbMap[dbName]; +namespace { - sqlite3_stmt* statement; - int statementStatus = sqlite3_prepare_v2(db, query.c_str(), -1, &statement, NULL); - if (statementStatus == SQLITE_OK) // statement is correct, bind the passed parameters - { - if (params) { - bindStatement(statement, *params); + struct SQLiteStatementFinalizer { + void operator()(sqlite3_stmt* statement) const noexcept { + if (statement != nullptr) { + sqlite3_finalize(statement); + } } - } else { - throw NitroSQLiteException::SqlExecution(sqlite3_errmsg(db)); - } + }; - auto isConsuming = true; - auto isFailed = false; + using SQLiteStatement = std::unique_ptr; - int result, i, count, column_type; - std::string column_name; - ColumnType column_declared_type; - SQLiteQueryResultRow row; - SQLiteQueryResults results; - std::optional metadata = std::nullopt; - - while (isConsuming) { - result = sqlite3_step(statement); - - switch (result) { - case SQLITE_ROW: - i = 0; - row = std::unordered_map(); - count = sqlite3_column_count(statement); - - while (i < count) { - column_type = sqlite3_column_type(statement, i); - column_name = sqlite3_column_name(statement, i); - switch (column_type) { - - case SQLITE_INTEGER: { - auto column_value = sqlite3_column_double(statement, i); - row[column_name] = column_value; - break; - } - case SQLITE_FLOAT: { - auto column_value = sqlite3_column_double(statement, i); - row[column_name] = column_value; - break; - } - case SQLITE_TEXT: { - auto column_value = reinterpret_cast(sqlite3_column_text(statement, i)); - sqlite3_column_bytes(statement, i); - row[column_name] = column_value; - break; - } - case SQLITE_BLOB: { - int blob_size = sqlite3_column_bytes(statement, i); - const void* blob = sqlite3_column_blob(statement, i); - // Copy the SQLite BLOB into a new native ArrayBuffer. - // This avoids manual memory management and unsafe pointer handling. - if (blob_size > 0) { - const auto* blob_data = reinterpret_cast(blob); - row[column_name] = ArrayBuffer::copy(blob_data, static_cast(blob_size)); - } else { - // Represent empty BLOBs as an empty, but valid, ArrayBuffer. - row[column_name] = ArrayBuffer::allocate(0); - } - break; - } - case SQLITE_NULL: - // Intentionally left blank to switch to default case - default: - row[column_name] = NullType::null; - break; - } - i++; - } - results.push_back(std::move(row)); - break; - case SQLITE_DONE: - i = 0; - count = sqlite3_column_count(statement); - while (i < count) { - column_name = sqlite3_column_name(statement, i); - const char* tp = sqlite3_column_decltype(statement, i); - column_declared_type = mapSQLiteTypeToColumnType(tp); - auto columnMeta = NitroSQLiteQueryColumnMetadata(std::move(column_name), std::move(column_declared_type), i); - - if (!metadata) { - metadata = std::make_optional(); - } - metadata->insert({column_name, columnMeta}); - i++; - } - isConsuming = false; - break; - default: - isFailed = true; - isConsuming = false; + sqlite3* getOpenDatabase(const std::string& dbName) { + if (dbMap.count(dbName) == 0) { + throw NitroSQLiteException::DatabaseNotOpen(dbName); } + + return dbMap[dbName]; } - sqlite3_finalize(statement); + SQLiteStatement prepareStatement(sqlite3* db, const std::string& query, const std::optional& params) { + sqlite3_stmt* rawStatement = nullptr; + int statementStatus = sqlite3_prepare_v2(db, query.c_str(), -1, &rawStatement, nullptr); + SQLiteStatement statement(rawStatement); - if (isFailed) { - throw NitroSQLiteException::SqlExecution(sqlite3_errmsg(db)); - } + if (statementStatus != SQLITE_OK) { + throw NitroSQLiteException::SqlExecution(sqlite3_errmsg(db)); + } - int rowsAffected = sqlite3_changes(db); - long long latestInsertRowId = sqlite3_last_insert_rowid(db); - return std::make_shared(results, static_cast(latestInsertRowId), rowsAffected, metadata); -} + if (params) { + bindStatement(statement.get(), *params); + } -SQLiteOperationResult sqliteExecuteLiteral(const std::string& dbName, const std::string& query) { - // Check if db connection is opened - if (dbMap.count(dbName) == 0) { - throw NitroSQLiteException::DatabaseNotOpen(dbName); + return statement; } - sqlite3* db = dbMap[dbName]; + template + void consumeStatement(sqlite3* db, sqlite3_stmt* statement, OnRow&& onRow) { + while (true) { + int result = sqlite3_step(statement); - // SQLite statements need to be compiled before executed - sqlite3_stmt* statement; + if (result == SQLITE_ROW) { + onRow(statement); + continue; + } - // Compile and move result into statement memory spot - int statementStatus = sqlite3_prepare_v2(db, query.c_str(), -1, &statement, NULL); + if (result == SQLITE_DONE) { + return; + } - if (statementStatus != SQLITE_OK) // statemnet is correct, bind the passed parameters - { - throw NitroSQLiteException::SqlExecution(sqlite3_errmsg(db)); + throw NitroSQLiteException::SqlExecution(sqlite3_errmsg(db)); + } } - bool isConsuming = true; - bool isFailed = false; - - int result; - std::string column_name; +} // namespace - while (isConsuming) { - result = sqlite3_step(statement); +std::shared_ptr sqliteExecute(const std::string& dbName, const std::string& query, + const std::optional& params) { + auto db = getOpenDatabase(dbName); + auto statement = prepareStatement(db, query, params); + SQLiteQueryResults results; - switch (result) { - case SQLITE_ROW: - isConsuming = true; - break; + consumeStatement(db, statement.get(), [&](sqlite3_stmt* currentStatement) { + SQLiteQueryResultRow row; + int count = sqlite3_column_count(currentStatement); + + for (int i = 0; i < count; i++) { + int columnType = sqlite3_column_type(currentStatement, i); + std::string columnName = sqlite3_column_name(currentStatement, i); + + switch (columnType) { + case SQLITE_INTEGER: + case SQLITE_FLOAT: + row[columnName] = sqlite3_column_double(currentStatement, i); + break; + case SQLITE_TEXT: { + auto columnValue = reinterpret_cast(sqlite3_column_text(currentStatement, i)); + row[columnName] = columnValue; + break; + } + case SQLITE_BLOB: { + int blobSize = sqlite3_column_bytes(currentStatement, i); + const void* blob = sqlite3_column_blob(currentStatement, i); + if (blobSize > 0) { + const auto* blobData = reinterpret_cast(blob); + row[columnName] = ArrayBuffer::copy(blobData, static_cast(blobSize)); + } else { + row[columnName] = ArrayBuffer::allocate(0); + } + break; + } + case SQLITE_NULL: + default: + row[columnName] = NullType::null; + break; + } + } - case SQLITE_DONE: - isConsuming = false; - break; + results.push_back(std::move(row)); + }); - default: - isFailed = true; - isConsuming = false; + std::optional metadata = std::nullopt; + int count = sqlite3_column_count(statement.get()); + for (int i = 0; i < count; i++) { + std::string columnName = sqlite3_column_name(statement.get(), i); + ColumnType columnDeclaredType = mapSQLiteTypeToColumnType(sqlite3_column_decltype(statement.get(), i)); + auto columnMeta = NitroSQLiteQueryColumnMetadata(columnName, std::move(columnDeclaredType), i); + + if (!metadata) { + metadata = std::make_optional(); } + metadata->insert({columnName, std::move(columnMeta)}); } - sqlite3_finalize(statement); + int rowsAffected = sqlite3_changes(db); + long long latestInsertRowId = sqlite3_last_insert_rowid(db); + return std::make_shared(std::move(results), static_cast(latestInsertRowId), rowsAffected, + std::move(metadata)); +} - if (isFailed) { - throw NitroSQLiteException::SqlExecution(sqlite3_errmsg(db)); - } +SQLiteOperationResult sqliteExecuteCommand(const std::string& dbName, const std::string& query, + const std::optional& params) { + auto db = getOpenDatabase(dbName); + auto statement = prepareStatement(db, query, params); + bool isReadOnly = sqlite3_stmt_readonly(statement.get()) != 0; + + consumeStatement(db, statement.get(), [](sqlite3_stmt*) {}); - return {.rowsAffected = sqlite3_changes(db)}; + return {.rowsAffected = isReadOnly ? 0 : sqlite3_changes(db)}; } } // namespace margelo::rnnitrosqlite diff --git a/packages/react-native-nitro-sqlite/cpp/operations.hpp b/packages/react-native-nitro-sqlite/cpp/operations.hpp index f485bdf5..49fce8ca 100644 --- a/packages/react-native-nitro-sqlite/cpp/operations.hpp +++ b/packages/react-native-nitro-sqlite/cpp/operations.hpp @@ -19,7 +19,8 @@ void sqliteDetachDb(const std::string& mainDBName, const std::string& alias); std::shared_ptr sqliteExecute(const std::string& dbName, const std::string& query, const std::optional& params); -SQLiteOperationResult sqliteExecuteLiteral(const std::string& dbName, const std::string& query); +SQLiteOperationResult sqliteExecuteCommand(const std::string& dbName, const std::string& query, + const std::optional& params = std::nullopt); void sqliteCloseAll(); diff --git a/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.cpp b/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.cpp index ed4be4d8..a3731250 100644 --- a/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.cpp +++ b/packages/react-native-nitro-sqlite/cpp/sqliteExecuteBatch.cpp @@ -40,15 +40,13 @@ SQLiteOperationResult sqliteExecuteBatch(const std::string& dbName, const std::v try { int rowsAffected = 0; - sqliteExecuteLiteral(dbName, "BEGIN EXCLUSIVE TRANSACTION"); - for (int i = 0; i < commandCount; i++) { - const auto command = commands.at(i); - - // Batch only aggregates rowsAffected; per-command result rows are discarded. - auto result = sqliteExecute(dbName, command.sql, command.params); - rowsAffected += result->getRowsAffected(); + sqliteExecuteCommand(dbName, "BEGIN EXCLUSIVE TRANSACTION"); + for (const auto& command : commands) { + auto result = sqliteExecuteCommand(dbName, command.sql, command.params); + rowsAffected += result.rowsAffected; } - sqliteExecuteLiteral(dbName, "COMMIT"); + + sqliteExecuteCommand(dbName, "COMMIT"); return { .rowsAffected = rowsAffected, .commands = (int)commandCount, @@ -56,7 +54,7 @@ SQLiteOperationResult sqliteExecuteBatch(const std::string& dbName, const std::v } catch (NitroSQLiteException& e) { // Roll back exactly once; a failed ROLLBACK must not mask the original error. try { - sqliteExecuteLiteral(dbName, "ROLLBACK"); + sqliteExecuteCommand(dbName, "ROLLBACK"); } catch (...) { // ignore — surface the original error below } From 43a418be3ab6dd2ae3d356f1bb961b20d365f127 Mon Sep 17 00:00:00 2001 From: chrispader Date: Thu, 23 Jul 2026 17:06:49 +0200 Subject: [PATCH 2/2] style: format cpp --- packages/react-native-nitro-sqlite/cpp/operations.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/react-native-nitro-sqlite/cpp/operations.cpp b/packages/react-native-nitro-sqlite/cpp/operations.cpp index bd3f0226..ae61dc65 100644 --- a/packages/react-native-nitro-sqlite/cpp/operations.cpp +++ b/packages/react-native-nitro-sqlite/cpp/operations.cpp @@ -26,7 +26,6 @@ using namespace margelo::nitro::rnnitrosqlite; namespace margelo::rnnitrosqlite { - static constexpr double kInt64MinAsDouble = static_cast(std::numeric_limits::min()); static constexpr double kInt64UpperBoundAsDouble = -kInt64MinAsDouble; @@ -129,8 +128,7 @@ void bindStatement(sqlite3_stmt* statement, const SQLiteQueryParams& values) { } else if (std::holds_alternative(value)) { // Bind whole numbers as INTEGER so vec0 rowid/pk/partition (which reject REAL) work; SQLite still coerces to REAL for REAL columns. double doubleValue = std::get(value); - if (std::trunc(doubleValue) == doubleValue && doubleValue >= kInt64MinAsDouble && - doubleValue < kInt64UpperBoundAsDouble) { + if (std::trunc(doubleValue) == doubleValue && doubleValue >= kInt64MinAsDouble && doubleValue < kInt64UpperBoundAsDouble) { sqlite3_bind_int64(statement, sqliteIndex, static_cast(doubleValue)); } else { sqlite3_bind_double(statement, sqliteIndex, doubleValue);