nbenn opened a new issue, #4828:
URL: https://github.com/apache/arrow-adbc/issues/4828
An ingest into an existing table whose `INSERT` fails to prepare, as for a
column the table lacks, leaves the connection in the transaction the SQLite
driver began for it. Whatever the connection runs afterwards goes into that
transaction: other connections do not see it, and nothing commits it. With
adbcsqlite 0.24.0-2:
```r
library(adbcdrivermanager)
db <- adbc_database_init(adbcsqlite::adbcsqlite(), uri = tempfile(fileext =
".sqlite"))
con <- adbc_connection_init(db)
other <- adbc_connection_init(db)
tables <- function(con) {
as.data.frame(read_adbc(con, "SELECT name FROM sqlite_master WHERE type =
'table'"))$name
}
write_adbc(data.frame(a = 1L), con, "t")
try(write_adbc(data.frame(b = 1L), con, "t", mode = "append"))
#> Error in adbc_statement_execute_query(stmt) :
#> INTERNAL: failed to prepare: table main.t has no column named b
#> query was: INSERT INTO main . "t" ("b") VALUES (?)
execute_adbc(con, "CREATE TABLE u (x INTEGER)")
tables(con)
#> [1] "t" "u"
tables(other)
#> [1] "t"
```
In autocommit mode, `ExecuteIngestImpl()` runs `BEGIN` before it prepares
the `INSERT` and ends the transaction with `COMMIT` or `ROLLBACK` after the
insert loop, but the `return` for a failed `sqlite3_prepare_v2()` comes before
that
([source](https://github.com/apache/arrow-adbc/blob/f1378d664b61ce65f25a54c3b863aa62ea9d2480/c/driver/sqlite/sqlite.cc#L1005-L1022),
[source](https://github.com/apache/arrow-adbc/blob/f1378d664b61ce65f25a54c3b863aa62ea9d2480/c/driver/sqlite/sqlite.cc#L1051-L1057)).
It came up in adbi, whose DBItest run skips two tests that fail after
`write_table_append_incompatible` because of it (r-dbi/adbi#106).
Rolling back before that `return` as well fixes it:
```diff
if (rc != SQLITE_OK) {
std::ignore = sqlite3_finalize(stmt);
- return status::fmt::Internal("failed to prepare: {}\nquery was: {}",
- sqlite3_errmsg(conn_), insert);
+ Status prepare_status = status::fmt::Internal(
+ "failed to prepare: {}\nquery was: {}", sqlite3_errmsg(conn_),
insert);
+ if (is_autocommit) {
+ UNWRAP_STATUS(::adbc::sqlite::SqliteQuery::Execute(conn_,
"ROLLBACK"));
+ }
+ return prepare_status;
}
```
Built from the adbcsqlite 0.24.0-2 sources with this change, `tables(other)`
above returns `"t"` and `"u"`, and both DBItest tests pass. I have not run the
driver's own test suite with it. I can open a PR.
This report was drafted with an AI assistant; the code above was run as
shown.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
