Skip to content

fix: apply session-level SET XACT_ABORT ON to all connections - #765

Merged
axellpadilla merged 1 commit into
dbt-msft:masterfrom
axellpadilla:fix/xact-abort-session-default-upstream
Jul 27, 2026
Merged

fix: apply session-level SET XACT_ABORT ON to all connections#765
axellpadilla merged 1 commit into
dbt-msft:masterfrom
axellpadilla:fix/xact-abort-session-default-upstream

Conversation

@axellpadilla

Copy link
Copy Markdown
Collaborator

Summary

Closes #718.

table_dml_refresh and 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 to COMMIT, silently persisting the DELETE on an empty target even though dbt reports failure. This also happens with no explicit transaction at all, so the fix can't depend on dbt_sqlserver_use_dbt_transactions.

Fix: SET XACT_ABORT ON as a session-level default on every connection, so a run-time error kills the whole batch and rolls back instead of falling through to COMMIT.

Changes

  • xact_abort: bool = True credentials field (in _connection_keys(), shows in dbt debug). Independent of dbt_sqlserver_use_dbt_transactions — that flag decides who owns the transaction boundary; xact_abort decides 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.
  • Once-per-process warning when disabled, noting the current dbt_sqlserver_use_dbt_transactions state.
  • Removed the now-redundant local SET XACT_ABORT ON from indexes.sql and create.sql — session-level is the single source of truth. Comment added above table_dml_refresh pointing at Evaluate SET XACT_ABORT ON for SQL Server DML refresh transactions #718.
  • README + CHANGELOG (labeled as a behavior change, default-on in an already-published 1.11.x line).

Testing

New functional suite (test_xact_abort.py) against a real SQL Server:

  • Seed 2 rows via DML refresh, second run yields NULL → run fails and the 2 rows survive (the row-count assertion is the point; pre-fix, the run also fails but the table is empty).
  • xact_abort=True × dbt_sqlserver_use_dbt_transactions on/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 & 16384 bit check: 1 by default, 0 when disabled.

Full suite: pytest tests/unit tests/functional -n auto600 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 one pytest process. Added an autouse reset fixture. Also added -n auto to 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 besides table_dml_refresh.sql, where this PR's change applies cleanly against the Dynamic Data Masking code already on master.

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.
@axellpadilla
axellpadilla merged commit b1ba99d into dbt-msft:master Jul 27, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Evaluate SET XACT_ABORT ON for SQL Server DML refresh transactions

1 participant