nbenn opened a new issue, #4827: URL: https://github.com/apache/arrow-adbc/issues/4827
### What happened? With the SQLite driver, a statement executed with an output stream that fails with `SQLITE_CONSTRAINT` reports only `INTERNAL: (unknown error)`, and releasing it then fails with the constraint error, so neither the statement nor its connection can be released. Executed without an output stream, the same statement reports `IO: failed to execute query: UNIQUE constraint failed: t.a` and releases cleanly. Through adbi, this leaves `dbDisconnect()` unable to close the connection after a `dbExecute()` that violates a constraint. The export reader handles only `SQLITE_ERROR` from its first `sqlite3_step()` by setting the message and resetting the statement. Every other error code, `SQLITE_CONSTRAINT` among them, falls through to `ADBC_STATUS_INTERNAL` with no message and no reset ([source](https://github.com/apache/arrow-adbc/blob/769e340d9193846366357d969da93d787e928383/c/driver/sqlite/statement_reader.c#L1234-L1243)). Without the reset, `sqlite3_finalize()` in `ReleaseImpl()` returns the step's error code again, and the release reports it as a failure ([source](https://github.com/apache/arrow-adbc/blob/769e340d9193846366357d969da93d787e928383/c/driver/sqlite/sqlite.cc#L1216-L1227)), which is the case the comment on the reset guards against. From R, a second release then fails with `INVALID_STATE: [Driver Manager] AdbcStatementRelease: must call AdbcStatementNew first`, while the statement still counts as a child of the connection. Would it make sense to treat every code other than `SQLITE_ROW` and `SQLITE_DONE` the way `SQLITE_ERROR` is treated there? That also gives these errors the `IO` status the same statement gets without a stream. ```diff - } else if (rc == SQLITE_ERROR) { + } else if (rc != SQLITE_ROW) { InternalAdbcSetError(error, "Failed to step query: %s", sqlite3_errmsg(db)); status = ADBC_STATUS_IO; // Reset here so that we don't get an error again in StatementRelease (void)sqlite3_reset(stmt); break; - } else if (rc != SQLITE_ROW) { - status = ADBC_STATUS_INTERNAL; - break; } ``` Built from 769e340 with this change, the reproducer below reports `IO: Failed to step query: UNIQUE constraint failed: t.a`, both releases succeed, and the R package's tests pass. I can open a PR. ### How can we reproduce the bug? ```r library(adbcdrivermanager) db <- adbc_database_init(adbcsqlite::adbcsqlite(), uri = ":memory:") con <- adbc_connection_init(db) execute_adbc(con, "CREATE TABLE t (a INTEGER PRIMARY KEY)") execute_adbc(con, "INSERT INTO t VALUES (1)") stmt <- adbc_statement_init(con) adbc_statement_set_sql_query(stmt, "INSERT INTO t VALUES (1)") stream <- nanoarrow::nanoarrow_allocate_array_stream() adbc_statement_execute_query(stmt, stream) #> Error in adbc_statement_execute_query(stmt, stream) : #> INTERNAL: (unknown error) adbc_statement_release(stmt) #> Error in adbc_statement_release(stmt) : #> IO: [SQLite] Failed to finalize statement: (19) UNIQUE constraint failed: t.a adbc_connection_release(con) #> Error in adbc_connection_release(con) : #> <adbcsqlite_connection/adbc_connection/adbc_xptr> has 1 unreleased child object ``` ### Environment/Setup Measured with adbcsqlite 0.24.0-2 and adbcdrivermanager 0.24.0-1 from CRAN, and with adbcsqlite built from 769e340, all against the system SQLite 3.45.1 on Ubuntu 24.04 with R 4.6.1. 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]
