fix: don't let a spurious mssql-python driver error during trailing drain fail a successful batch

CHANGELOG.md

@@ -13,21 +13,26 @@
 
 #### Bugfixes
 
-- Fix a `view` model silently skipping a rebuild when text was removed from the *start* of its body (e.g. deleting a leading comment or CTE). The skip test compared the stored definition against the model with `endswith()`, so any edit whose new body was a tail of the old one looked unchanged: `dbt run` reported `PASS` but the change never reached the database, and `--full-refresh` did not fix it. The header (`CREATE [OR ALTER] VIEW <name> AS`) is now split off at its separating ` AS ` and the body compared exactly. The comparison also no longer lowercases or strips whitespace, both of which made genuinely different bodies (a string literal differing only in case, or any literal containing spaces) compare equal; where the definition cannot be parsed with certainty the view is rebuilt rather than skipped.
-- Fix snapshots failing on their second and later runs with `Invalid object name '..._dbt_tmp'`, and contract-enforced models silently losing their in-transaction `pre_hook` writes. `get_column_schema_from_query` reads a query's column shape by executing it, then returned without fetching the rows or closing the cursor. Closing a cursor whose result set the server is still producing makes the driver cancel the request, and SQL Server answers that cancel by rolling back the open transaction, since every connection runs `SET XACT_ABORT ON` (#718). Nothing is raised for any of it, so the snapshot lost the staging table it had just built and failed against it a statement later. The probe now drains and closes its cursor, as does the row-count probe in `expand_column_types`. Only queries opening with a CTE were affected - anything else is wrapped as `select * from (...) where 1 = 0` by `sqlserver__get_empty_subquery_sql` and returns no rows - which is why snapshot staging queries (`with snapshot_query as ...`, both `check` and `timestamp` strategies) and CTE-headed contract models were the ones that broke.
 - Fix models failing with `Incorrect syntax near '\'` when the schema name needs delimiters, such as a domain-qualified `domain\user`. The clustered columnstore index name embeds the schema and was emitted as a bare identifier, so the generated DDL did not parse. [#409](https://github.com/dbt-msft/dbt-sqlserver/issues/409)
 - Fix identifiers built inside string literals not being quoted, which broke schema names containing a `.` or a `"`. `OBJECT_ID('schema.table')` returns `NULL` rather than erroring for such a name, so the failures were silent: the drop-before-create guards in `create_table_as` treated an existing table as absent (then hit `Msg 2714`), and the mask introspection in `apply_masks` found no columns, so configured masks were never applied. `sp_rename` was affected too, failing the table rename-swap with `No item by the name of ...`. All now pass quoted, qualified names. [#785](https://github.com/dbt-msft/dbt-sqlserver/issues/785)
 
 #### Under the hood
 
-- `get_column_schema_from_query` now reads a CTE-headed query's shape with `sp_describe_first_result_set` instead of executing it. Such queries cannot be wrapped as `select * from (...) where 1 = 0`, so they previously ran in full - a snapshot executed its whole staging query once for the probe and again to build the staging table, and a contract-enforced model ran twice per build. Describing compiles without scanning, so the cost no longer tracks data volume. Reported column names and types are unchanged: the describe path is mapped back onto the coarser names reading `cursor.description` produces, and falls back to executing whenever it cannot guarantee that (the `adbc` backend, a type outside the mapping, or a query `sp_describe_first_result_set` declines to describe, such as one reading a `#temp` table). The Python class behind a column is the driver's choice rather than SQL Server's, and the backends do not agree on all of them - `mssql-python` decodes `uniqueidentifier`, `datetimeoffset` and `sql_variant` into richer types than `pyodbc` does - so the mapping follows the backend in use, and `datetimeoffset` on `pyodbc` (whose class depends on when the `-155` output converter was registered) falls back to executing.
 - **Behavior change (generated SQL only):** the few macros that hand-formatted `[bracket]` identifiers now use `adapter.quote()`, matching the `"double quoted"` identifiers `{{ relation }}` already rendered, so one statement no longer mixes both styles. Affects `USE` (now via the existing `get_use_database_sql()` helper), `CREATE SCHEMA`, contract column lists, grantees, index/constraint names and generated test view names. Server-side `QUOTENAME()` keeps brackets by design. Visible only if you parse dbt's generated SQL or have custom macros assuming brackets. [#785](https://github.com/dbt-msft/dbt-sqlserver/issues/785)
 - Identifier quoting escapes an embedded `"` by doubling it (`ab"cd` → `"ab""cd"`), in both `adapter.quote()` and relation rendering (`SQLServerRelation.quoted`). Needed twice over: a `"` requires no escaping inside `[brackets]` but does inside double quotes, so the delimiter change above would otherwise have rejected names the adapter previously accepted; and the string-literal fix above renders identifiers through the relation, so a schema containing a `"` depends on it. [#785](https://github.com/dbt-msft/dbt-sqlserver/issues/785)
 
 #### Under the hood
 
 - Replace mypy with [`ty`](https://docs.astral.sh/ty/) for type checking, completing the move to a single Astral toolchain (Ruff for lint and format, `ty` for types). Same scope as before (`dbt/adapters`, unresolvable `dbt.*` imports ignored), configured in `pyproject.toml`. `ty` runs in a new Type check workflow rather than on pre-commit.ci, which blocks the network access it needs. [#712](https://github.com/dbt-msft/dbt-sqlserver/issues/712)
 
+### v1.11.1
+
+#### Bugfixes
+
+- Fix a `view` model silently skipping a rebuild when text was removed from the *start* of its body (e.g. deleting a leading comment or CTE). The skip test compared the stored definition against the model with `endswith()`, so any edit whose new body was a tail of the old one looked unchanged: `dbt run` reported `PASS` but the change never reached the database, and `--full-refresh` did not fix it. The header (`CREATE [OR ALTER] VIEW <name> AS`) is now split off at its separating ` AS ` and the body compared exactly. The comparison also no longer lowercases or strips whitespace, both of which made genuinely different bodies (a string literal differing only in case, or any literal containing spaces) compare equal; where the definition cannot be parsed with certainty the view is rebuilt rather than skipped. [#807](https://github.com/dbt-msft/dbt-sqlserver/issues/807)
+- Fix snapshots failing on their second and later runs with `Invalid object name '..._dbt_tmp'`, and contract-enforced models silently losing their in-transaction `pre_hook` writes. `get_column_schema_from_query` reads a query's column shape by executing it, then returned without fetching the rows or closing the cursor. Closing a cursor whose result set the server is still producing makes the driver cancel the request, and SQL Server answers that cancel by rolling back the open transaction, since every connection runs `SET XACT_ABORT ON` (#718). Nothing is raised for any of it, so the snapshot lost the staging table it had just built and failed against it a statement later. The probe now drains and closes its cursor, as does the row-count probe in `expand_column_types`. Only queries opening with a CTE were affected - anything else is wrapped as `select * from (...) where 1 = 0` by `sqlserver__get_empty_subquery_sql` and returns no rows - which is why snapshot staging queries (`with snapshot_query as ...`, both `check` and `timestamp` strategies) and CTE-headed contract models were the ones that broke. A CTE-headed probe is now also described with `sp_describe_first_result_set` rather than executed, so it no longer runs its query twice per build. [#809](https://github.com/dbt-msft/dbt-sqlserver/issues/809)
+- Fix a successful multi-statement batch (e.g. the `delete+insert` incremental strategy, or a `table_refresh_method: dml` swap) failing the run when `dbt_sqlserver_use_dbt_transactions` is `false` and `threads > 1` on the `mssql-python` backend. `execute()` had already read the batch's response (`cursor.rowcount`) before walking the trailing `DONE`/`DONE_IN_PROC` chain via `nextset()`; that walk could raise a spurious `New transaction is not allowed because there are other threads running in the session` from inside the driver even though nothing failed and no other thread shared the connection (a known open defect, microsoft/mssql-python#229). That exact error no longer fails the run - the SQL already succeeded and its response was already captured. Only that specific message is treated as non-fatal: `nextset()` can also carry a genuine, deferred error from a later statement in the same batch (SQL Server's deferred name resolution means a bad column reference in a `CREATE VIEW` only surfaces once something queries the view), so every other error still fails the build exactly as before.
+
 ### v1.11.0
 
 #### Features

dbt/adapters/sqlserver/sqlserver_connections.py

@@ -234,6 +234,74 @@ def _try_drain_nextset(cursor: Any) -> bool:
         raise
 
 
+# Substring of the exact SQL Server error text mssql-python has been observed
+# raising from nextset() after a *successful* multi-statement batch (see
+# _drain_trailing_results). Matched literally, not as a regex.
+_MSSQL_PYTHON_SPURIOUS_TRAILING_DRAIN_ERROR = (
+    "new transaction is not allowed because there are other threads running in the session"
+)
+
+
+def _is_spurious_mssql_python_trailing_drain_error(e: Exception) -> bool:
+    """True only for the exact known-spurious mssql-python nextset() defect.
+
+    nextset() can also raise a *real*, deferred error from a later statement
+    in the same batch -- SQL Server's deferred name resolution lets
+    ``CREATE VIEW ... AS SELECT bad_column FROM t`` succeed, so a query
+    against that view (e.g. the ``SELECT * INTO`` that follows it in
+    sqlserver__create_table_as) only fails once nextset() walks to it, not on
+    the initial execute(). Swallowing every nextset() exception here would
+    hide that failure and report a broken model as built (caught by
+    test_concurrency.py::TestConcurrency::test_concurrency, which asserts the
+    known-broken ``invalid`` model actually fails). Only the exact known
+    spurious message is treated as non-fatal; everything else -- a different
+    message, a different exception type, a different backend -- re-raises
+    unchanged.
+    """
+    return (
+        type(e).__name__ == "ProgrammingError"
+        and type(e).__module__.startswith("mssql_python")
+        and _MSSQL_PYTHON_SPURIOUS_TRAILING_DRAIN_ERROR in str(e).lower()
+    )
+
+
+def _drain_trailing_results(cursor: Any) -> None:
+    """Advance past whatever result sets are left after execute()'s own
+    response has already been read.
+
+    A multi-statement batch (e.g. the delete+insert incremental strategy's
+    ``SET NOCOUNT ON; delete ...; SET NOCOUNT OFF; insert ...``, or the DML
+    table-refresh swap's ``BEGIN TRANSACTION; delete ...; insert ...; COMMIT
+    TRANSACTION;``) leaves a DONE/DONE_IN_PROC token per statement for the
+    driver to walk through via nextset() -- dbt-msft/dbt-sqlserver#814:
+    mssql-python (>=1.7.1, confirmed through at least 1.12.0) can raise a
+    spurious "New transaction is not allowed because there are other threads
+    running in the session" from nextset() here, on a batch that already
+    completed successfully and with no other thread sharing the connection.
+    Matches an open upstream defect (microsoft/mssql-python#229): a
+    multi-statement batch that turns ``SET NOCOUNT`` back off before its
+    last statement -- done deliberately so that statement's rowcount is
+    reported -- leaves that statement's own DONE_IN_PROC token in the
+    stream, which the driver's SQLMoreResults handling does not always walk
+    cleanly.
+
+    Only that exact known-spurious error is swallowed (logged at debug).
+    nextset() can also carry a *real*, deferred error from a later statement
+    in the same batch -- SQL Server's deferred name resolution means a
+    referenced-but-nonexistent column in a ``CREATE VIEW`` only surfaces once
+    something queries the view, which can be a later statement walked to by
+    nextset() rather than the initial execute() call. Everything else
+    re-raises unchanged.
+    """
+    try:
+        while _try_drain_nextset(cursor):
+            pass
+    except Exception as e:
+        if not _is_spurious_mssql_python_trailing_drain_error(e):
+            raise
+        logger.debug(f"Draining trailing result sets failed: {e}")
+
+
 # Rows per round trip when shedding a result set nobody asked for. Large
 # enough that discarding even a big one costs a handful of round trips.
 _DISCARD_CHUNK_SIZE = 10000
@@ -862,8 +930,7 @@ def execute(
                 table = self.get_result_from_cursor(cursor, limit)
             else:
                 table = empty_table()
-            while _try_drain_nextset(cursor):
-                pass
+            _drain_trailing_results(cursor)
             return response, table
         finally:
             cursor.close()

tests/unit/adapters/mssql/test_sqlserver_connection_manager.py

@@ -2191,3 +2191,101 @@ def test_add_query_clears_in_flight_cursor_after_failure(
             manager.add_query("select 1", auto_begin=False)
 
     assert connection._dbt_sqlserver_in_flight_cursor is None
+
+
+class _MssqlPythonProgrammingError(Exception):
+    """Stand-in for mssql_python.ProgrammingError.
+
+    Real message observed in production (dbt-msft/dbt-sqlserver, mssql-python
+    1.12.0, dbt_sqlserver_use_dbt_transactions: false, threads > 1, an
+    incremental delete+insert model): the batch itself succeeded -- dbt's own
+    rowcount/response was already read -- but walking the trailing DONE/
+    DONE_IN_PROC chain via nextset() afterwards raised this from inside the
+    driver's DDBCSQLMoreResults. Matches microsoft/mssql-python#229 (open as
+    of this writing): multi-statement batches that don't hold SET NOCOUNT ON
+    for their *entire* duration leave intermediate DONE_IN_PROC tokens for
+    nextset() to walk -- our delete+insert strategy turns NOCOUNT back OFF
+    before the final INSERT specifically so its rowcount is reported, which
+    is exactly that shape.
+    """
+
+
+# The fix duck-types on the real exception's __module__ and __name__
+# (mssql_python.exceptions.ProgrammingError), so the fake must match both to
+# exercise the same branch a real driver error would.
+_MssqlPythonProgrammingError.__module__ = "mssql_python.exceptions"
+_MssqlPythonProgrammingError.__name__ = "ProgrammingError"
+_MssqlPythonProgrammingError.__qualname__ = "ProgrammingError"
+
+
+def test_execute_survives_a_spurious_mssql_python_error_during_trailing_drain(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Pins the bug as reported: a successful batch must not be killed by
+    drain. Without _drain_trailing_results, this raises out of execute()
+    from the exact call site in the report (execute -> _try_drain_nextset ->
+    cursor.nextset()). cursor.rowcount is set before nextset() is ever
+    called, matching "the SQL execution itself succeeds" from the report.
+    """
+    cursor = MagicMock()
+    cursor.rowcount = 5
+    cursor.nextset.side_effect = [
+        True,  # the DELETE's DONE_IN_PROC
+        _MssqlPythonProgrammingError(
+            "Driver Error: Syntax error or access violation; DDBC Error: "
+            "[Microsoft][SQL Server]New transaction is not allowed because "
+            "there are other threads running in the session."
+        ),
+    ]
+
+    manager, _connection = _build_cancel_test_manager(monkeypatch, cursor)
+    monkeypatch.setattr(manager, "_add_query_comment", lambda sql: sql)
+
+    with patch("dbt.adapters.sqlserver.sqlserver_connections.fire_event"):
+        response, _table = manager.execute(
+            "SET NOCOUNT ON; delete from t where id in (1,2,3); "
+            "SET NOCOUNT OFF; insert into t select * from s;",
+            auto_begin=False,
+            fetch=False,
+        )
+
+    # The batch's own result was already captured before the drain ran.
+    assert response.rows_affected == 5
+    # The trailing drain must not be allowed to fail the run: the cursor is
+    # still closed, and nothing about the already-captured response changes.
+    assert cursor.close.called
+
+
+def test_execute_still_raises_a_genuine_deferred_error_during_trailing_drain(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """A real, different error from nextset() must still fail the run.
+
+    Regression guard for a mistake caught by
+    tests/functional/adapter/dbt/test_concurrency.py::TestConcurrency::test_concurrency:
+    an earlier version of this fix swallowed *every* exception from the
+    trailing drain, not just the known-spurious one. SQL Server's deferred
+    name resolution lets `CREATE VIEW ... AS SELECT bad_column FROM t`
+    succeed at create time; sqlserver__create_table_as's SELECT * INTO that
+    queries that view only fails once nextset() walks to it (not on the
+    initial execute()), so broadly swallowing nextset() errors here silently
+    turned a broken model's build into a reported success.
+    """
+    cursor = MagicMock()
+    cursor.rowcount = 0
+    cursor.nextset.side_effect = _MssqlPythonProgrammingError(
+        "Driver Error: Column not found; DDBC Error: "
+        "[Microsoft][SQL Server]Invalid column name 'a_field_that_does_not_exist'."
+    )
+
+    manager, _connection = _build_cancel_test_manager(monkeypatch, cursor)
+    monkeypatch.setattr(manager, "_add_query_comment", lambda sql: sql)
+
+    with patch("dbt.adapters.sqlserver.sqlserver_connections.fire_event"):
+        with pytest.raises(_MssqlPythonProgrammingError, match="Invalid column name"):
+            manager.execute(
+                "EXEC('CREATE OR ALTER VIEW v AS SELECT bad_column FROM t'); "
+                "EXEC('SELECT * INTO tgt FROM v');",
+                auto_begin=False,
+                fetch=False,
+            )