Repository navigation
fix: a statement that fails to compile no longer aborts the transaction #470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,6 +156,25 @@ def test_server_connect_autocommit_false(server: dict) -> None: | |
| assert row and row[0] == 1 | ||
|
|
||
|
|
||
| def test_server_autocommit_false_failed_statement_keeps_transaction(server: dict) -> None: | ||
| # a failed statement rolls back only its own changes, the transaction stays active and can be committed | ||
| # see https://docs.snowflake.com/en/sql-reference/transactions#failed-statements-within-a-transaction | ||
| with ( | ||
| snowflake.connector.connect(**server | {"autocommit": False}, database="db1", schema="schema1") as conn, | ||
| conn.cursor() as cur, | ||
| ): | ||
| cur.execute("create or replace table failed_statement (i int)") | ||
| cur.execute("insert into failed_statement values (1)") | ||
|
|
||
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute("select * from missing_table") | ||
|
|
||
| conn.commit() | ||
| conn.rollback() | ||
| cur.execute("select count(*) from failed_statement") | ||
| assert cur.fetchone() == (1,) | ||
|
|
||
|
|
||
| def test_server_client_session_keep_alive(server: dict) -> None: | ||
| with snowflake.connector.connect(**server | {"client_session_keep_alive": True}): | ||
| # shouldn't error | ||
|
|
@@ -828,3 +847,48 @@ def test_server_fetch_multiple_arrow_batches(scur: snowflake.connector.cursor.Sn | |
|
|
||
| timestamp = datetime.datetime(2024, 1, 2, 3, 4, 5, 123456) | ||
| assert scur.fetchall() == [(i, None if i % 2 == 0 else "héllo", Decimal("12.34"), timestamp) for i in range(5000)] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("commit_via", ["connection", "sql", "autocommit"]) | ||
| def test_server_runtime_failure_commit_raises(server: dict, commit_via: str) -> None: | ||
| # Deviates from Snowflake: a runtime failure aborts DuckDB's transaction. The server must | ||
| # report the failed commit through the connector instead of returning success or HTTP 500. | ||
| with ( | ||
| snowflake.connector.connect(**server | {"autocommit": False}, database="db1", schema="schema1") as conn, | ||
| conn.cursor() as cur, | ||
| ): | ||
| cur.execute("CREATE TABLE runtime_commit (id int)") | ||
| cur.execute("INSERT INTO runtime_commit VALUES (1)") | ||
| 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: | ||
| if commit_via == "connection": | ||
| conn.commit() | ||
| elif commit_via == "sql": | ||
| cur.execute("COMMIT") | ||
| else: | ||
| cur.execute("ALTER SESSION SET AUTOCOMMIT = TRUE") | ||
| assert (excinfo.value.errno, excinfo.value.sqlstate) == (9999, "99999") | ||
| conn.rollback() | ||
| cur.execute("SELECT count(*) FROM runtime_commit") | ||
| assert cur.fetchone() == (0,) | ||
|
|
||
|
|
||
| def test_server_runtime_failure_autocommit_keeps_commit_error(server: dict) -> None: | ||
| # The real connector suppresses ALTER SESSION errors in autocommit(). Keep the aborted | ||
| # transaction pending so that a subsequent commit still reports the lost writes. | ||
| # Deviates from Snowflake, which preserves earlier writes after the failed statement. | ||
| with ( | ||
| snowflake.connector.connect(**server | {"autocommit": False}, database="db1", schema="schema1") as conn, | ||
| conn.cursor() as cur, | ||
| ): | ||
| cur.execute("CREATE TABLE runtime_autocommit (id int)") | ||
| cur.execute("INSERT INTO runtime_autocommit VALUES (1)") | ||
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute("INSERT INTO runtime_autocommit VALUES ('x')") | ||
|
|
||
| conn.autocommit(True) | ||
| with pytest.raises(snowflake.connector.errors.ProgrammingError, match="transaction was aborted"): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note 🤖 gpt-6-luna responding on behalf of tekumara On Snowflake, |
||
| conn.commit() | ||
| conn.rollback() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -221,3 +221,121 @@ def test_conn_context_exit_with_exception_rolls_back(_fakesnow: None): | |
|
|
||
| with snowflake.connector.connect(database="db1", schema="schema1") as conn: | ||
| assert conn.cursor().execute("SELECT count(*) FROM context_exit").fetchone() == (0,) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("failing_sql", ["DESC TABLE missing_table", "SELECT * FROM missing_table"]) | ||
| def test_autocommit_false_failed_statement_keeps_transaction( | ||
| conn: snowflake.connector.SnowflakeConnection, failing_sql: str | ||
| ): | ||
| # a failed statement rolls back only its own changes, the transaction stays active and can be committed | ||
| # see https://docs.snowflake.com/en/sql-reference/transactions#failed-statements-within-a-transaction | ||
| conn.autocommit(False) | ||
| with conn.cursor() as cur: | ||
| cur.execute("CREATE OR REPLACE TABLE failed_statement (id int)") | ||
| cur.execute("INSERT INTO failed_statement VALUES (1)") | ||
|
|
||
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute(failing_sql) | ||
|
|
||
| assert cur.execute("SELECT count(*) FROM failed_statement").fetchone() == (1,) | ||
| conn.commit() | ||
| conn.rollback() | ||
| assert cur.execute("SELECT count(*) FROM failed_statement").fetchone() == (1,) | ||
|
|
||
|
|
||
| def test_autocommit_false_runtime_failure_commit_raises(conn: snowflake.connector.SnowflakeConnection): | ||
| # Deviates from Snowflake, which rolls back only the failed statement and commits the earlier insert, see | ||
| # https://docs.snowflake.com/en/sql-reference/transactions#failed-statements-within-a-transaction | ||
| # DuckDB has no savepoints, so a statement that fails at runtime aborts the whole transaction. Rather than | ||
| # commit silently dropping the earlier insert, fakesnow raises and requires an explicit rollback. | ||
| conn.autocommit(False) | ||
| with conn.cursor() as cur: | ||
| cur.execute("CREATE OR REPLACE TABLE runtime_failure (id int)") | ||
| cur.execute("INSERT INTO runtime_failure VALUES (1)") | ||
|
|
||
| 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"): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note 🤖 gpt-6-luna responding on behalf of tekumara On Snowflake the failed integer conversion raises |
||
| conn.commit() | ||
| conn.rollback() | ||
| assert cur.execute("SELECT count(*) FROM runtime_failure").fetchone() == (0,) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "commit_sql", | ||
| [ | ||
| "COMMIT", | ||
| "CREATE TABLE next_table (id int)", | ||
| "ALTER SESSION SET AUTOCOMMIT = TRUE", | ||
| "ALTER SESSION SET AUTOCOMMIT = FALSE", | ||
| ], | ||
| ) | ||
| def test_runtime_failure_sql_commit_raises(conn: snowflake.connector.SnowflakeConnection, commit_sql: str): | ||
| # Deviates from Snowflake: DuckDB cannot preserve earlier writes after a runtime failure. | ||
| # Every explicit or implicit commit must report that loss instead of claiming success. | ||
| conn.autocommit(False) | ||
| with conn.cursor() as cur: | ||
| cur.execute("CREATE TABLE runtime_sql_commit (id int)") | ||
| cur.execute("INSERT INTO runtime_sql_commit VALUES (1)") | ||
| 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: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note 🤖 gpt-6-luna responding on behalf of tekumara Live Snowflake allows SQL |
||
| cur.execute(commit_sql) | ||
| assert (excinfo.value.errno, excinfo.value.sqlstate) == (9999, "99999") | ||
| conn.rollback() | ||
| assert cur.execute("SELECT count(*) FROM runtime_sql_commit").fetchone() == (0,) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("mode", [False, True]) | ||
| def test_runtime_failure_autocommit_raises(conn: snowflake.connector.SnowflakeConnection, mode: bool): | ||
| # Deviates from Snowflake: changing AUTOCOMMIT cannot preserve writes after a DuckDB runtime failure. | ||
| conn.autocommit(False) | ||
| with conn.cursor() as cur: | ||
| cur.execute("CREATE TABLE runtime_autocommit (id int)") | ||
| cur.execute("INSERT INTO runtime_autocommit VALUES (1)") | ||
| 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"): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note 🤖 gpt-6-luna responding on behalf of tekumara Both |
||
| conn.autocommit(mode) | ||
| conn.rollback() | ||
| assert cur.execute("SELECT count(*) FROM runtime_autocommit").fetchone() == (0,) | ||
|
|
||
|
|
||
| def test_runtime_failure_rollback(conn: snowflake.connector.SnowflakeConnection): | ||
| conn.autocommit(False) | ||
| with conn.cursor() as cur: | ||
| cur.execute("CREATE TABLE runtime_rollback (id int)") | ||
| cur.execute("INSERT INTO runtime_rollback VALUES (1)") | ||
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute("INSERT INTO runtime_rollback VALUES ('x')") | ||
|
|
||
| conn.rollback() | ||
| cur.execute("INSERT INTO runtime_rollback VALUES (2)") | ||
| conn.commit() | ||
| assert cur.execute("SELECT * FROM runtime_rollback").fetchall() == [(2,)] | ||
|
|
||
|
|
||
| def test_context_exit_failed_commit_closes_connection(_fakesnow: None): | ||
| # 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"), | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| conn, | ||
| conn.cursor() as cur, | ||
| ): | ||
| cur.execute("CREATE TABLE runtime_context_exit (id int)") | ||
| cur.execute("INSERT INTO runtime_context_exit VALUES (1)") | ||
| with pytest.raises(snowflake.connector.errors.ProgrammingError): | ||
| cur.execute("INSERT INTO runtime_context_exit VALUES ('x')") | ||
|
|
||
| assert conn.is_closed() | ||
|
|
||
|
|
||
| def test_commit_closed_connection(conn: snowflake.connector.SnowflakeConnection): | ||
| conn.close() | ||
| with pytest.raises(snowflake.connector.errors.DatabaseError) as excinfo: | ||
| conn.commit() | ||
| assert (excinfo.value.errno, excinfo.value.sqlstate) == (250002, "08003") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
🤖 gpt-6-luna responding on behalf of tekumara
Live Snowflake:
INSERT ... VALUES ('x')fails with100038 (22018): Numeric value 'x' is not recognized, butconn.commit(), SQLCOMMIT, andALTER SESSION SET AUTOCOMMIT = TRUEall succeed; the earlier insert remains. This does not produce the expectedtransaction was abortederror or fakesnow(9999, "99999")code.