Evaluate `SET XACT_ABORT ON` for SQL Server DML refresh transactions

[axellpadilla]

Summary

@joshmarkovic detected some problems on dml, quote:

Issue: The dml refresh swap runs BEGIN TRANSACTION; DELETE FROM target; INSERT INTO target SELECT ... ; COMMIT; as one batch on an autocommit connection, without SET XACT_ABORT ON. Statement-aborting errors (e.g. a NULL or constraint violation on the INSERT) do not stop a T-SQL batch — execution continues to the trailing COMMIT, which persists the DELETE. Result: the target is left committed-empty. Silent data loss, reproduced live (other connections see 0 rows, @@TRANCOUNT = 0, nothing to roll back).

table_dml_refresh performs a critical delete/insert replacement inside a transaction. We should evaluate whether this path should explicitly set XACT_ABORT ON while the DML swap runs, so SQL Server automatically aborts the transaction for most runtime errors.

Context

The DML refresh path performs an all-or-nothing operation:

BEGIN TRANSACTION;

DELETE FROM target_relation;

INSERT INTO target_relation (...)
SELECT ...
FROM intermediate_relation;

COMMIT TRANSACTION;

Without XACT_ABORT ON, some SQL Server statement errors can leave the transaction open or committable depending on the error type. For a delete/insert replacement, that can make failure handling more fragile because the target table may already have been modified before the insert fails.

XACT_ABORT ON is commonly recommended for explicit SQL Server transactions that should fail atomically.

Proposal

Evaluate adding scoped XACT_ABORT ON around the DML refresh swap.

The implementation should preserve and restore the previous session setting:

DECLARE @OriginalXactAbort INT;
SET @OriginalXactAbort = CASE WHEN (@@OPTIONS & 16384) > 0 THEN 1 ELSE 0 END;

BEGIN TRY
    SET XACT_ABORT ON;

    BEGIN TRANSACTION;

    DELETE FROM target_relation;

    INSERT INTO target_relation (...)
    SELECT ...
    FROM intermediate_relation;

    COMMIT TRANSACTION;

    IF @OriginalXactAbort = 1
        SET XACT_ABORT ON;
    ELSE
        SET XACT_ABORT OFF;
END TRY
BEGIN CATCH
    IF @@TRANCOUNT > 0
        ROLLBACK TRANSACTION;

    IF @OriginalXactAbort = 1
        SET XACT_ABORT ON;
    ELSE
        SET XACT_ABORT OFF;

    THROW;
END CATCH;

If dbt-managed transactions are enabled, the implementation should avoid adding a redundant nested BEGIN TRANSACTION / COMMIT TRANSACTION unless explicitly needed. In that mode, XACT_ABORT ON may still be useful around the DML body, but transaction ownership should remain with dbt.

Questions to resolve

1. Should XACT_ABORT ON be applied only to table_dml_refresh, or more broadly to SQL Server transactions managed by the adapter?
2. Should the DML refresh macro own rollback in the catch block when dbt-managed transactions are enabled, or should it only restore session state and rethrow?
3. Does restoring XACT_ABORT after failure behave correctly with the adapter’s connection reuse model?

Acceptance criteria

• Preserve the original XACT_ABORT session setting after success and after failure.
• Document why XACT_ABORT ON is used in this DML path.
• One functional test proving this (that fails without xact_abort on)

[Benjamin-Knight]

1. Should XACT_ABORT ON be applied only to table_dml_refresh, or more broadly to SQL Server transactions managed by the adapter?

Whilst the DML highlights the issue due to its implicit transaction if we start running DBT inside its normal transaction we should be running xact abort as well otherwise we will have inconsistency inside our transactions, we actually have inconsistency now, a DBT run can error out part way through and leave corrupted or temporary tables and views, but we accept this as its not a transaction.

2. Should the DML refresh macro own rollback in the catch block when dbt-managed transactions are enabled, or should it only restore session state and rethrow?

It should be however we decide to handle a normal DBT transaction which will cover all other models throwing an error and rolling backing.

3. Does restoring XACT_ABORT after failure behave correctly with the adapter’s connection reuse model?

I'm not sure we should be trying to toggle this on and off, it should either be on for the adapter or off, otherwise it could get confusing.