backport(1.11): transaction-mode fixes — resumable indexes, full-refresh marker, concurrent-incremental deadlock, catalog nolock

CHANGELOG.md

@@ -16,7 +16,10 @@
 
 #### Bugfixes
 
+- Fix `dbt_sqlserver_use_dbt_transactions: True` (opt-in in 1.11, the default from 1.12) breaking two things that assumed the old autocommit-per-statement behavior: building an index with `build_options: {online: true}` or `{resumable: true}` on a fresh table or `table_refresh_method`'s rename-swap path (SQL Server rejects `RESUMABLE` inside a user transaction outright — `create_indexes` didn't split these out the way index reconciliation already did); and the `dbt_full_refresh_incomplete` crash marker on `full_refresh_build=prebuilt` and incremental full refreshes, which previously rode the same ambient transaction as the rebuild it's meant to survive, so a real failure mid-rebuild rolled the marker back along with everything else instead of leaving it in place to block the next normal run.
+- Fix concurrent incremental models deadlocking (error 1205) at `threads` > 1 with `dbt_sqlserver_use_dbt_transactions: True` (opt-in in 1.11, the default from 1.12). The temp-relation build — `CREATE OR ALTER VIEW …__dbt_tmp_vw`, `SELECT * INTO …__dbt_tmp`, `DROP VIEW` — is all system-catalog DDL, and it was running inside the materialization's ambient transaction, so its exclusive `sys.sysschobjs` key locks were held until commit while a second worker's name resolution asked for shared locks on the same rows. Nothing in that build needs a transaction (the relation is throwaway); it was being opened accidentally, by a read-only catalog lookup that used `statement()`'s default `auto_begin=True`. The temp build and the fresh-create/full-refresh `create_table_as` batch now autocommit statement by statement and release their catalog locks as they go, while the incremental strategy DML stays transactional through to `adapter.commit()` as before. Pre-hook rollback semantics are unchanged: nothing is committed early, dbt only declines to open a transaction for a read. Because the temp build now commits standalone, a crashed run can leave a `__dbt_tmp` table behind, so the `SELECT * INTO` path drops adapter-generated throwaways first instead of failing the next run with `Msg 2714`; a fresh create of a real target still surfaces `Msg 2714` rather than silently destroying an object dbt does not know about.
 - Fix `drop_all_indexes_on_table()` and its component macros (`drop_pk_constraints()`, `drop_fk_constraints()`, `drop_xml_indexes()`, `drop_spatial_indexes()`) matching tables by name only. A post-hook on a model dropped the primary key, inbound foreign keys, and XML/spatial indexes of every same-named table in *other* schemas of the same database. All four now filter on the model's schema as well. Identifiers in the generated dynamic SQL are also wrapped in `QUOTENAME()`.
+- Apply the existing `information_schema_hints()` (`with (nolock)`) to the read-only catalog lookups that were missing it: `get_relation_last_modified` (reached by `dbt source freshness` on a source with no `loaded_at_field`), the foreign-key and primary-key introspection in `drop_fk_constraints` / `drop_pk_constraints` / `drop_fk_references`, `list_nonclustered_rowstore_indexes`, and `find_references`. SQL Server holds transaction-scoped exclusive locks on the system catalog for the duration of any `CREATE`/`ALTER`/`DROP`, so an un-hinted catalog read from dbt waits on every other writer in the database — a second dbt project, an ETL tool, a DBA mid-deployment — and since the default `LOCK_TIMEOUT` is `-1` it waits indefinitely, with dbt appearing to hang and no error. Catalog reads that act as correctness guards rather than discovery (the schema-exists check before `CREATE SCHEMA`, the principal-exists check in `apply_grants`, and the `dbt_full_refresh_incomplete` extended-property marker) are deliberately left un-hinted, since a dirty read there would weaken the guard.
 - **Behavior change:** `SET XACT_ABORT ON` is now a session-level default on every connection the adapter opens, fixing silent, committed data loss on the DML table refresh path (and any other multi-statement batch): a mid-batch run-time error (e.g. a `NOT NULL`/constraint violation on the swap `INSERT`) previously aborted only the failing statement, not the batch, so a trailing `COMMIT` could still commit a partial result — most notably an emptied target after the `DELETE` succeeded and the `INSERT` failed. This is on by default starting in this release; opt out per profile with `xact_abort: false` if you rely on continue-on-error batch semantics in custom hooks. [#718](https://github.com/dbt-msft/dbt-sqlserver/issues/718)
 
 #### Under the hood

dbt/adapters/sqlserver/sqlserver_adapter.py

@@ -419,6 +419,57 @@ def expand_target_column_types(
     def parse_index(self, raw_index: Any) -> Optional[SQLServerIndexConfig]:
         return SQLServerIndexConfig.parse(raw_index)
 
+    @available
+    def index_needs_own_batch(self, raw_index: Any) -> bool:
+        """True when raw_index's build_options (ONLINE / RESUMABLE) force its
+        CREATE INDEX to run outside any transaction: SQL Server rejects
+        RESUMABLE inside a user transaction (error 574), and an ONLINE build
+        wrapped in one holds its locks until commit, negating the point."""
+        parsed = self.parse_index(raw_index)
+        if not parsed:
+            return False
+        return create_needs_own_batch(parsed.build_options)
+
+    @available
+    def commit_if_open(self) -> None:
+        """Commit the current transaction if one is open - a no-op otherwise.
+
+        dbt_sqlserver_use_dbt_transactions on (default) wraps a
+        materialization's whole build, from its first statement through its
+        own trailing ``adapter.commit()``, in one continuous ambient
+        transaction. Some statements must not share that transaction with
+        whatever runs after them: an ONLINE/RESUMABLE index build (SQL Server
+        rejects RESUMABLE inside a user transaction outright, and ONLINE
+        holds its locks until commit either way), or a full-refresh-in-
+        progress marker, which exists specifically to survive a later
+        failure and so must not roll back with it. This makes the prior
+        statement durable on its own; pair with begin_if_closed once the
+        statement(s) that must run outside a transaction are done, to leave
+        later code (more such statements, or the materialization's own
+        trailing ``adapter.commit()``, which raises if it finds nothing open)
+        working as if this call had never happened.
+
+        A no-op when no transaction is open: the caller's statement already
+        ran autocommitted on its own (e.g. via ``run_query``'s
+        ``auto_begin=false``, before anything else began one), so there is
+        nothing to flush. Also a no-op, at the SQL level, whenever
+        dbt_sqlserver_use_dbt_transactions is off: begin/commit still flip
+        dbt-core's bookkeeping (see
+        SQLServerConnectionManager.add_begin_query/add_commit_query), but
+        emit no real T-SQL, matching the driver's own autocommit.
+        """
+        connection = self.connections.get_thread_connection()
+        if connection is not None and connection.transaction_open:
+            self.connections.commit()
+
+    @available
+    def begin_if_closed(self) -> None:
+        """Begin a transaction if none is open - a no-op otherwise. See
+        commit_if_open, which this pairs with."""
+        connection = self.connections.get_thread_connection()
+        if connection is not None and not connection.transaction_open:
+            self.connections.begin()
+
     @available
     def validate_indexes(
         self, raw_indexes: Any, as_columnstore: Any = False, drop_unmanaged: Any = False

dbt/include/sqlserver/macros/adapters/indexes.sql

@@ -65,8 +65,8 @@
     declare @drop_fk_constraints nvarchar(max);
     select @drop_fk_constraints = (
         select 'IF OBJECT_ID(''' + REPLACE(QUOTENAME(SCHEMA_NAME(sys.foreign_keys.[schema_id])) + '.' + QUOTENAME(sys.foreign_keys.[name]), '''', '''''') + ''', ''F'') IS NOT NULL ALTER TABLE ' + QUOTENAME(SCHEMA_NAME(sys.foreign_keys.[schema_id])) + '.' + QUOTENAME(OBJECT_NAME(sys.foreign_keys.[parent_object_id])) + ' DROP CONSTRAINT ' + QUOTENAME(sys.foreign_keys.[name]) + ';'
-        from sys.foreign_keys
-        inner join sys.tables on sys.foreign_keys.[referenced_object_id] = sys.tables.[object_id]
+        from sys.foreign_keys {{ information_schema_hints() }}
+        inner join sys.tables {{ information_schema_hints() }} on sys.foreign_keys.[referenced_object_id] = sys.tables.[object_id]
         where SCHEMA_NAME(sys.tables.[schema_id]) = '{{ this.schema }}'
         and sys.tables.[name] = '{{ this.table }}'
         for xml path('')
@@ -90,8 +90,8 @@
     declare @drop_pk_constraints nvarchar(max);
     select @drop_pk_constraints = (
         select 'IF INDEXPROPERTY(' + CONVERT(VARCHAR(MAX), sys.tables.[object_id]) + ', ' + QUOTENAME(sys.indexes.[name], '''') + ', ''IndexId'') IS NOT NULL ALTER TABLE ' + QUOTENAME(SCHEMA_NAME(sys.tables.[schema_id])) + '.' + QUOTENAME(sys.tables.[name]) + ' DROP CONSTRAINT ' + QUOTENAME(sys.indexes.[name]) + ';'
-        from sys.indexes
-        inner join sys.tables on sys.indexes.[object_id] = sys.tables.[object_id]
+        from sys.indexes {{ information_schema_hints() }}
+        inner join sys.tables {{ information_schema_hints() }} on sys.indexes.[object_id] = sys.tables.[object_id]
         where sys.indexes.is_primary_key = 1
         and SCHEMA_NAME(sys.tables.[schema_id]) = '{{ this.schema }}'
         and sys.tables.[name] = '{{ this.table }}'
@@ -187,18 +187,18 @@
       col1.name AS [column],
       tab2.name AS [referenced_table],
       col2.name AS [referenced_column]
-      FROM sys.foreign_key_columns fkc
-      INNER JOIN sys.objects obj
+      FROM sys.foreign_key_columns fkc {{ information_schema_hints() }}
+      INNER JOIN sys.objects obj {{ information_schema_hints() }}
           ON obj.object_id = fkc.constraint_object_id
-      INNER JOIN sys.tables tab1
+      INNER JOIN sys.tables tab1 {{ information_schema_hints() }}
           ON tab1.object_id = fkc.parent_object_id
-      INNER JOIN sys.schemas sch
+      INNER JOIN sys.schemas sch {{ information_schema_hints() }}
           ON tab1.schema_id = sch.schema_id
-      INNER JOIN sys.columns col1
+      INNER JOIN sys.columns col1 {{ information_schema_hints() }}
           ON col1.column_id = parent_column_id AND col1.object_id = tab1.object_id
-      INNER JOIN sys.tables tab2
+      INNER JOIN sys.tables tab2 {{ information_schema_hints() }}
           ON tab2.object_id = fkc.referenced_object_id
-      INNER JOIN sys.columns col2
+      INNER JOIN sys.columns col2 {{ information_schema_hints() }}
           ON col2.column_id = referenced_column_id AND col2.object_id = tab2.object_id
       WHERE sch.name = '{{ relation.schema }}' and tab2.name = '{{ relation.identifier }}'
   {% endcall %}
@@ -216,8 +216,8 @@
     SELECT i.name AS index_name
     , i.name + '__dbt_backup' as index_new_name
     , COL_NAME(ic.object_id,ic.column_id) AS column_name
-    FROM sys.indexes AS i
-    INNER JOIN sys.index_columns AS ic
+    FROM sys.indexes AS i {{ information_schema_hints() }}
+    INNER JOIN sys.index_columns AS ic {{ information_schema_hints() }}
         ON i.object_id = ic.object_id AND i.index_id = ic.index_id and i.type <> 5
     WHERE i.object_id = OBJECT_ID('{{ relation.schema }}.{{ relation.identifier }}')
 
@@ -226,18 +226,18 @@
     SELECT  obj.name AS index_name
     , obj.name + '__dbt_backup' as index_new_name
     , col1.name AS column_name
-    FROM sys.foreign_key_columns fkc
-    INNER JOIN sys.objects obj
+    FROM sys.foreign_key_columns fkc {{ information_schema_hints() }}
+    INNER JOIN sys.objects obj {{ information_schema_hints() }}
         ON obj.object_id = fkc.constraint_object_id
-    INNER JOIN sys.tables tab1
+    INNER JOIN sys.tables tab1 {{ information_schema_hints() }}
         ON tab1.object_id = fkc.parent_object_id
-    INNER JOIN sys.schemas sch
+    INNER JOIN sys.schemas sch {{ information_schema_hints() }}
         ON tab1.schema_id = sch.schema_id
-    INNER JOIN sys.columns col1
+    INNER JOIN sys.columns col1 {{ information_schema_hints() }}
         ON col1.column_id = parent_column_id AND col1.object_id = tab1.object_id
-    INNER JOIN sys.tables tab2
+    INNER JOIN sys.tables tab2 {{ information_schema_hints() }}
         ON tab2.object_id = fkc.referenced_object_id
-    INNER JOIN sys.columns col2
+    INNER JOIN sys.columns col2 {{ information_schema_hints() }}
         ON col2.column_id = referenced_column_id AND col2.object_id = tab2.object_id
     WHERE sch.name = '{{ relation.schema }}' and tab1.name = '{{ relation.identifier }}'
 
@@ -453,12 +453,51 @@
   {%- do adapter.validate_indexes(
       raw_indexes, as_columnstore, config.get('drop_unmanaged_indexes', default=false)
   ) -%}
+  {%- set creates_no_txn = [] -%}
   {%- for _index_dict in raw_indexes %}
     {%- set create_index_sql = get_create_index_sql(relation, _index_dict) -%}
     {% if create_index_sql %}
-      {% do run_query(create_index_sql) %}
+      {% if adapter.index_needs_own_batch(_index_dict) %}
+        {%- do creates_no_txn.append(_index_dict) -%}
+      {% else %}
+        {% do run_query(create_index_sql) %}
+      {% endif %}
     {% endif %}
   {%- endfor %}
+  {% do sqlserver__create_indexes_no_txn(relation, creates_no_txn) %}
+{% endmacro %}
+
+
+{% macro sqlserver__create_indexes_no_txn(relation, index_dicts) %}
+  {#-
+    Build ONLINE / RESUMABLE indexes as standalone (non-transactional)
+    statements. SQL Server forbids RESUMABLE in a user transaction, and an
+    ONLINE build wrapped in one holds its locks until commit, negating the
+    non-blocking intent.
+
+    With dbt_sqlserver_use_dbt_transactions on (default), the
+    materialization's whole build - from the first statement through its own
+    trailing adapter.commit() - runs inside one continuous ambient
+    transaction, so run_query's auto_begin=false is not enough on its own to
+    escape it: commit it first (see adapter.commit_if_open), before ANY of
+    these run - committing between them would just reopen one and land the
+    next create right back inside it - then reopen once after the whole
+    batch (adapter.begin_if_closed), which always leaves one open afterward
+    even if none was open before, so later code (grants, persist_docs, the
+    materialization's own trailing adapter.commit(), which raises if it
+    finds nothing open) sees one exactly as it would without this macro.
+    When the flag is off, or no earlier statement this run has opened one,
+    commit_if_open is a no-op and these still run standalone via run_query's
+    auto_begin=false.
+  -#}
+  {% if index_dicts %}
+    {{ adapter.commit_if_open() }}
+    {% for index_dict in index_dicts %}
+      {% do log("Creating ONLINE/RESUMABLE index outside the transaction on " ~ relation, info=true) %}
+      {% do run_query(get_create_index_sql(relation, index_dict)) %}
+    {% endfor %}
+    {{ adapter.begin_if_closed() }}
+  {% endif %}
 {% endmacro %}
 
 
@@ -500,15 +539,13 @@
         ~ ";\ncommit transaction;"
     ) %}
   {% endif %}
-  {#- ONLINE / RESUMABLE creates cannot run inside the transaction above
-      (SQL Server forbids RESUMABLE in a user transaction, and an ONLINE build
-      wrapped in one holds its locks until commit, negating the non-blocking
-      intent). Apply them individually in autocommit. The drops above have
-      already committed, so a replacement index still builds after its
-      predecessor is gone; these are not part of the atomic batch, so there is
-      a brief window where the new index is absent for readers. -#}
-  {%- for index_dict in result['creates_no_txn'] %}
-    {% do log("Creating ONLINE/RESUMABLE index outside the reconcile transaction on " ~ relation, info=true) %}
-    {% do run_query(sqlserver__get_create_index_sql(relation, index_dict)) %}
-  {%- endfor %}
+  {#- ONLINE / RESUMABLE creates cannot run inside the transaction above, nor
+      inside the ambient dbt-managed transaction that wraps the whole
+      materialization when dbt_sqlserver_use_dbt_transactions is on - see
+      sqlserver__create_indexes_no_txn. The drops (and any transactional
+      creates) above have already committed by the time these run, so a
+      replacement index still builds after its predecessor is gone; these are
+      not part of the atomic batch, so there is a brief window where the new
+      index is absent for readers. -#}
+  {% do sqlserver__create_indexes_no_txn(relation, result['creates_no_txn']) %}
 {% endmacro %}

dbt/include/sqlserver/macros/adapters/metadata.sql

@@ -194,8 +194,8 @@
             , s.name as [schema]
             , o.modify_date as last_modified
             , current_timestamp as snapshotted_at
-        from sys.objects o
-        inner join sys.schemas s on o.schema_id = s.schema_id and [type] = 'U'
+        from sys.objects o {{ information_schema_hints() }}
+        inner join sys.schemas s {{ information_schema_hints() }} on o.schema_id = s.schema_id and [type] = 'U'
         where (
             {%- for relation in relations -%}
             (upper(s.name) = upper('{{ relation.schema }}') and

dbt/include/sqlserver/macros/adapters/relation.sql

@@ -8,15 +8,21 @@
 
 {% macro sqlserver__get_drop_sql(relation) -%}
   {% if relation.type == 'view' -%}
-      {% call statement('find_references', fetch_result=true) %}
+      {#- auto_begin=false, matching dbt-adapters' own relations/drop.sql: a
+          read-only lookup must not open the ambient transaction. This is the
+          first statement of the temp-relation build (create.sql ->
+          adapter.drop_relation), so with the default auto_begin=True it held
+          that build's sys.sysschobjs X locks until the materialization
+          committed, deadlocking a concurrent worker (error 1205). -#}
+      {% call statement('find_references', fetch_result=true, auto_begin=false) %}
         {{ get_use_database_sql(relation.database) }}
         select
             sch.name as schema_name,
             obj.name as view_name
-        from sys.sql_expression_dependencies refs
-        inner join sys.objects obj
+        from sys.sql_expression_dependencies refs {{ information_schema_hints() }}
+        inner join sys.objects obj {{ information_schema_hints() }}
         on refs.referencing_id = obj.object_id
-        inner join sys.schemas sch
+        inner join sys.schemas sch {{ information_schema_hints() }}
         on obj.schema_id = sch.schema_id
         where refs.referenced_database_name = '{{ relation.database }}'
         and refs.referenced_schema_name = '{{ relation.schema }}'

dbt/include/sqlserver/macros/materializations/hooks.sql

@@ -5,7 +5,11 @@
         {% if not adapter.behavior.dbt_sqlserver_use_dbt_transactions %}
           if @@trancount > 0 commit; -- post hooks after fictitious transaction work as expected
         {% else %}
-          commit; -- align transaction=False hook behavior with dbt-core transaction semantics.
+          {#- guarded, not a bare COMMIT: nothing guarantees a transaction is
+              open here. A bare one appeared safe only because some earlier
+              statement had auto-begun one (find_references, until relation.sql
+              stopped doing that); with @@TRANCOUNT = 0 it raises Msg 3902. -#}
+          if @@trancount > 0 commit; -- align transaction=False hook behavior with dbt-core transaction semantics.
         {% endif %}
       {% endcall %}
     {% endif %}