Repository navigation
fix: a statement that fails to compile no longer aborts the transaction - #470
THEtheChad wants to merge 2 commits into
Conversation
Since 0.12.0 (DuckDB 2.0) a statement that fails inside a transaction aborts it: with autocommit=False every later statement raises "Current transaction is aborted (please ROLLBACK)", and a later commit succeeds but silently discards the earlier writes. snowflake-sqlalchemy hits this whenever Table.create(checkfirst=True) probes for a missing table. Snowflake rolls back only the failed statement and keeps the transaction active, so it can still be committed with the changes of the successful statements. DuckDB 2.0 invalidates the transaction on all errors by default. Set default_transaction_invalidation_policy to SYNTACTIC_ERRORS_DO_NOT_INVALIDATE, which restores DuckDB 1.x behaviour: parser, binder and catalog errors such as a missing table leave the transaction intact, in process and in server mode. DuckDB has no savepoints, so a statement that fails at runtime (eg: a conversion or constraint error) still aborts the whole transaction, as it did on DuckDB 1.x. Committing it, explicitly or implicitly before DDL or an autocommit change, now rolls back and raises instead of silently dropping the earlier writes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Handle ALTER SESSION autocommit changes before their success query, and retain an aborted transaction until rollback or close. The real connector suppresses autocommit errors, so a later commit must still reject the lost writes. Preserve transaction bookkeeping when a commit fails. Close connections when context-manager commits fail, preserve closed-connection error conversion, and cover direct, SQL, implicit, and server commit routes. Co-Authored-By: GPT-6 Astra <noreply@openai.com>
Outside autocommit, DDL ends a transaction a failed statement aborted (see tekumara#470), so CREATE TEMPORARY TABLE RECOVERY (ID INT) succeeds there. Since temporary tables resolve their schema, it instead raised "Current transaction is aborted", because the schema is looked up before the statement runs, even with no temporary tables. When the aborted transaction rejects the lookup, look the schema up again once the DDL has ended the transaction, just before the table is created, and drop orphaned temporary tables then too. If the schema doesn't exist, raise the usual error without running the statement, so CREATE OR REPLACE never replaces an existing table first. Otherwise the table is registered, resolves by its qualified name and is dropped with its schema. MERGE, which first replaces a temporary table of candidates, takes the same path. A qualified CREATE TEMPORARY TABLE takes the same path, so it's created too. With autocommit on, the transaction is still aborted when the statement runs, so the lookup fails as the statement itself would. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Outside autocommit, DDL ends a transaction a failed statement aborted (see tekumara#470), so CREATE TABLE RECOVERY AS SELECT * FROM DB1.S1.P succeeds there. Once any temporary table had been created, even an unrelated one, since dropped, it instead raised "Current transaction is aborted", because each qualified reference read the catalog to check for a temporary table before the statement ran. The same went for temporary CTAS, CLONE and ALTER. Keep the names of the session's registered temporary tables, added as each is created and refreshed from the catalog before each statement, which picks up renames, drops and ROLLBACK. A qualified reference to any other name can't be a temporary table, so it's left as written, without reading the catalog. A reference whose name does match a temporary table, such as DB1.S1.P while a temporary P exists, still reads it, so it still fails in an aborted transaction; tekumara#470 owns that. While a transaction is open the names only grow, because ending it can restore a temporary table dropped or renamed in it. Otherwise, after BEGIN, DROP TABLE T and a failed statement, DROP TABLE DB1.S1.T outside autocommit skipped T, and once its implicit commit had rolled back the transaction, restoring the temporary T, dropped the permanent T. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… with it (tekumara#472) Conflicts resolved against tekumara#470 (a failed statement aborts an explicit transaction): cursor.py keeps tekumara#472's tracking of BEGIN in either autocommit mode and its temporary_tables.validate(), but leaves COMMIT and ROLLBACK to end the transaction only once they succeed, as tekumara#470 does. temporary_tables.py keeps tekumara#472's registry (table OID -> catalog, schema, schema OID) and its _names guard, with fork main's quoted temp.main identifiers; owners() returns the catalog and schema from the new registry. The tests are both sides' additions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ROLLBACK Since tekumara#470, a failed statement inside an explicit transaction aborts it and fakesnow refuses the DDL that tekumara#472's tests ran next, rather than letting it end the transaction. Each test now expects that refusal, rolls back, and then checks the temporary table's state and name resolution as before. The MERGE is refused by DuckDB's own error, as other DML in an aborted transaction is, because resolving its table names reads the catalog first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute("INSERT INTO runtime_commit VALUES ('x')") | ||
|
|
||
| with pytest.raises(snowflake.connector.errors.ProgrammingError, match="transaction was aborted") as excinfo: |
There was a problem hiding this comment.
Note
🤖 gpt-6-luna responding on behalf of tekumara
Live Snowflake: INSERT ... VALUES ('x') fails with 100038 (22018): Numeric value 'x' is not recognized, but conn.commit(), SQL COMMIT, and ALTER SESSION SET AUTOCOMMIT = TRUE all succeed; the earlier insert remains. This does not produce the expected transaction was aborted error or fakesnow (9999, "99999") code.
| cur.execute("INSERT INTO runtime_autocommit VALUES ('x')") | ||
|
|
||
| conn.autocommit(True) | ||
| with pytest.raises(snowflake.connector.errors.ProgrammingError, match="transaction was aborted"): |
There was a problem hiding this comment.
Note
🤖 gpt-6-luna responding on behalf of tekumara
On Snowflake, conn.autocommit(True) succeeds after the failed insert, and the following conn.commit() succeeds too. The earlier row remains; the expected transaction was aborted error is not raised.
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute("INSERT INTO runtime_failure VALUES ('x')") | ||
|
|
||
| with pytest.raises(snowflake.connector.errors.ProgrammingError, match="transaction was aborted"): |
There was a problem hiding this comment.
Note
🤖 gpt-6-luna responding on behalf of tekumara
On Snowflake the failed integer conversion raises 100038 (22018): Numeric value 'x' is not recognized, but conn.commit() succeeds and commits the earlier insert. A subsequent rollback does not undo that commit, so the row count is 1 rather than 0.
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute("INSERT INTO runtime_sql_commit VALUES ('x')") | ||
|
|
||
| with pytest.raises(snowflake.connector.errors.ProgrammingError, match="transaction was aborted") as excinfo: |
There was a problem hiding this comment.
Note
🤖 gpt-6-luna responding on behalf of tekumara
Live Snowflake allows SQL COMMIT and both ALTER SESSION SET AUTOCOMMIT variants after this failed insert; the aborted-transaction error is not raised. A temporary-table DDL follow-up also succeeded and preserved the earlier row. The persistent CREATE TABLE next_table probe could not be run with this account: 003540 (42501): SQL execution error: Creating table on shared database 'SNOWFLAKE' is not allowed.
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute("INSERT INTO runtime_autocommit VALUES ('x')") | ||
|
|
||
| with pytest.raises(snowflake.connector.errors.ProgrammingError, match="transaction was aborted"): |
There was a problem hiding this comment.
Note
🤖 gpt-6-luna responding on behalf of tekumara
Both conn.autocommit(False) and conn.autocommit(True) return without an error on Snowflake after the runtime failure. The earlier row remains after rollback, rather than producing the expected exception and a zero count.
| # Deviates from Snowflake: the commit fails after a runtime error, but context exit must still close. | ||
| conn = snowflake.connector.connect(database="db1", schema="schema1", autocommit=False) | ||
| with ( | ||
| pytest.raises(snowflake.connector.errors.ProgrammingError, match="transaction was aborted"), |
There was a problem hiding this comment.
Note
🤖 gpt-6-luna responding on behalf of tekumara
N/A to real Snowflake: this test requires context exit to raise because an earlier runtime failure aborted the transaction. Snowflake rolls back only the failed statement, so context exit can commit the earlier insert instead; this failed-commit path is fakesnow-specific.
Fixes #459.
Since 0.12.0 (DuckDB 2.0), catalog and binder errors abort an open transaction by default. With
autocommit=False, later statements fail, andcommit()silently discards earlier writes. This affects snowflake-sqlalchemy'sTable.create(checkfirst=True), which probes for a missing table usingDESC TABLE. Snowflake instead rolls back only the failed statement and allows successful statements in the transaction to be committed.FakeSnownow setsdefault_transaction_invalidation_policy = SYNTACTIC_ERRORS_DO_NOT_INVALIDATE, restoring DuckDB 1.x behavior for parser, binder and catalog errors. The setting is supported by the pinned DuckDB 2.0.0 and applies to both in-process connections and server sessions.Runtime failures such as conversion and constraint errors still abort the whole transaction in DuckDB, which has no savepoints. To prevent a subsequent commit from silently discarding writes, the connection checks for an aborted transaction before
conn.commit(), SQLCOMMIT, implicit commits before supported DDL, and autocommit changes. It raisesProgrammingErrorwith errno9999and SQLSTATE99999, the project's existing convention for unsupported behavior. The error explains the difference from Snowflake and requires rollback before starting another transaction.The guard leaves the transaction aborted until explicit rollback or connection close. This matters because the real Snowflake Python connector's
autocommit()suppresses connector errors: server-modeconn.autocommit(...)can swallow the warning, but a subsequentconn.commit()still fails. SQLALTER SESSION SET AUTOCOMMITreports the error directly. Context-manager exit closes the connection even if its commit fails.The check uses
SELECT 1; DuckDB's Python API exposes no transaction-validity flag. In DuckDB 2.0.0 this query raisesTransactionExceptionin an aborted transaction and leaves no transaction open when run outside one. Ordinary rollback remains unguarded so callers can recover.Remaining deviation: Snowflake preserves earlier writes after runtime failures; fakesnow rejects the commit instead. Statements attempted before rollback still encounter DuckDB's aborted-transaction error. Replaying earlier statements to emulate rollback of only one statement is outside this fix, especially because replay is unsafe for non-deterministic statements.
Validation: the five original regression cases fail on base
3f65f02. Expanded tests cover SQLAlchemy's check-first pattern, direct and server commits, implicit DDL commits, autocommit changes, rollback recovery, context-manager cleanup, and closed-connection error conversion. The full suite passes: 551 passed, 2 xfailed. Ruff lint and formatting pass; pyright reports 0 errors and 0 warnings.Checked against a live Snowflake account (10.38): with autocommit off, after
insert (1), a failingselect * from no_such_table, andinsert (2), the commit succeeds there and here and keeps both rows. After a runtime failure instead (insert into txn values ('x')into an INT column,100038 (22018)there), Snowflake still commits both rows, while fakesnow refuses the commit: the deviation described above, now confirmed.🤖 Generated with Claude Code