fix: apply session-level SET XACT_ABORT ON to all connections - #765
Merged
axellpadilla merged 1 commit intoJul 27, 2026
Merged
Conversation
Without it, a run-time error partway through a multi-statement batch (e.g. the DML table refresh's DELETE+INSERT swap) only aborts the failing statement, not the batch, so a trailing COMMIT can still persist a partial result -- most notably an emptied target after the DELETE succeeds and the INSERT fails on a constraint violation. This is silent, committed data loss and is independent of whether dbt owns the transaction boundary (dbt_sqlserver_use_dbt_transactions). - Add xact_abort credentials field (default true), applied once per connection in SQLServerConnectionManager.open() via a new _apply_session_settings(); a connection that fails this setup is closed rather than handed back to dbt as usable. - Warn once per process (not per connection) when xact_abort is disabled, noting the current dbt_sqlserver_use_dbt_transactions state. - Remove the now-redundant local `SET XACT_ABORT ON` from the index reconcile batch and the prebuilt full-refresh insert -- session-level is now the single source of truth. - Fix a test-isolation bug found while validating this change: unit tests stub the process-global pyodbc/mssql-python module cache via configure_runtime_state_for_test() but never reset it afterward, so whichever test runs last leaks a fake driver module into every test that follows in the same process -- including real functional tests run in the same `pytest` invocation. Added an autouse fixture to reset it before/after each test. - Run functional tests in parallel (-n auto) in CI, matching unit tests. Closes dbt-msft#718.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #718.
table_dml_refreshand other multi-statement paths (index reconcile, prebuilt full-refresh) send DML as one autocommit batch (BEGIN; DELETE; INSERT; COMMIT). A statement-abort error (NOT NULL, constraint violation, etc.) only aborts that statement, not the batch — execution falls through toCOMMIT, silently persisting theDELETEon an empty target even though dbt reports failure. This also happens with no explicit transaction at all, so the fix can't depend ondbt_sqlserver_use_dbt_transactions.Fix:
SET XACT_ABORT ONas a session-level default on every connection, so a run-time error kills the whole batch and rolls back instead of falling through toCOMMIT.Changes
xact_abort: bool = Truecredentials field (in_connection_keys(), shows indbt debug). Independent ofdbt_sqlserver_use_dbt_transactions— that flag decides who owns the transaction boundary;xact_abortdecides how the server reacts to a mid-batch error, which matters most exactly when there's no explicit transaction.open()applies it via new_apply_session_settings(); a connection that fails this setup is closed rather than handed back to dbt.dbt_sqlserver_use_dbt_transactionsstate.SET XACT_ABORT ONfromindexes.sqlandcreate.sql— session-level is the single source of truth. Comment added abovetable_dml_refreshpointing at EvaluateSET XACT_ABORT ONfor SQL Server DML refresh transactions #718.Testing
New functional suite (
test_xact_abort.py) against a real SQL Server:xact_abort=True×dbt_sqlserver_use_dbt_transactionson/off: rows preserved both ways.xact_abort=False+ transactions on: preserved via dbt's own rollback, not XACT_ABORT.xact_abort=False+ transactions off (both disabled): not asserted as passing — manually confirmed this still loses rows, which is the documented, intentional limit of disabling the flag, not a regression.@@OPTIONS & 16384bit check: 1 by default, 0 when disabled.Full suite:
pytest tests/unit tests/functional -n auto→ 600 passed, 48 skipped, 2 xfailed, 0 failed (298s).Also fixed along the way: a pre-existing test-isolation bug where unit tests stub a process-global driver-module cache (
_RUNTIME_STATE) without resetting it, leaking a fake module into real functional tests when both run in onepytestprocess. Added an autouse reset fixture. Also added-n autoto the integration-tests CI workflow (verified safe locally).Notes
Based directly on current upstream
master, independent of #763 (UDF function resource support) — no shared files besidestable_dml_refresh.sql, where this PR's change applies cleanly against the Dynamic Data Masking code already onmaster.