fix: concurrent-incremental catalog deadlock, and (nolock) on the remaining catalog lookups

CHANGELOG.md

@@ -10,10 +10,6 @@
 - **Behavior change:** `dbt_sqlserver_use_native_string_types` now defaults to `True`: `STRING` maps to `VARCHAR(MAX)`, `NCHAR` to `NCHAR(1)`, and `NVARCHAR` to `NVARCHAR(4000)`, instead of the legacy `VARCHAR(8000)`/`CHAR(1)` mappings. The `False` (legacy) behavior is deprecated and will be removed in a future release.
 - Add an experimental `adbc` backend (`backend: adbc`), an alternative to `pyodbc`/`mssql-python` built on [ADBC](https://arrow.apache.org/adbc/) that talks to SQL Server via `go-mssqldb` instead of an ODBC/DB-API bridge. Install with `dbt-sqlserver[adbc]` plus the separate `dbc` CLI-installed driver binary; supports SQL Server (user/password) authentication only for now. See [docs/adbc_backend.md](docs/adbc_backend.md). [#771](https://github.com/dbt-msft/dbt-sqlserver/issues/771)
 
-#### Bugfixes
-
-- Fix `dbt_sqlserver_use_dbt_transactions: True` (now the default) 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.
-
 ### v1.11.0
 
 #### Features
@@ -30,7 +26,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/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 }}'
 

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 %}

dbt/include/sqlserver/macros/materializations/models/incremental/incremental.sql

@@ -31,6 +31,10 @@
 
   {% set to_drop = [] %}
   {% set prebuilt_handled = false %}
+  {#- true only where the statement('main') batch below carries create_table_as
+      DDL, i.e. the fresh-create / full-refresh branches. The incremental
+      branch's strategy DML stays transactional, so it leaves this false. -#}
+  {% set build_sql_is_create_table_as = false %}
 
   {% if existing_relation is none %}
     {% if config.get('full_refresh_build', 'heap_then_index') == 'prebuilt' %}
@@ -43,6 +47,7 @@
       {% set prebuilt_handled = true %}
     {% else %}
       {% set build_sql = get_create_table_as_sql(False, target_relation, sql) %}
+      {% set build_sql_is_create_table_as = true %}
     {% endif %}
   {% elif full_refresh_mode %}
     {#- the target is marked as having a full refresh in flight (blocking
@@ -69,6 +74,7 @@
       {% set prebuilt_handled = true %}
     {% else %}
       {% set build_sql = get_create_table_as_sql(False, intermediate_relation, sql) %}
+      {% set build_sql_is_create_table_as = true %}
       {% if existing_relation.type == 'table' %}
         {% do sqlserver__mark_full_refresh_incomplete(existing_relation) %}
       {% endif %}
@@ -82,6 +88,14 @@
       {% do sqlserver__assert_no_incomplete_full_refresh(existing_relation) %}
     {% endif %}
 
+    {#- The temp build is all catalog DDL (CREATE OR ALTER VIEW / SELECT *
+        INTO / DROP VIEW) and must not share the ambient transaction with the
+        strategy DML: held to commit, its sysschobjs X keylocks deadlock a
+        second worker. Nothing opens a transaction here - run_query never
+        auto-begins, and find_references (relation.sql) no longer does either -
+        so each statement autocommits and drops its catalog locks as it
+        finishes. The strategy DML below still runs transactionally, via
+        statement('main')'s default auto_begin through to adapter.commit(). -#}
     {% do run_query(get_create_table_as_sql(True, temp_relation, sql)) %}
 
     {% set contract_config = config.get('contract') %}
@@ -109,9 +123,28 @@
   {% endif %}
 
   {% if not prebuilt_handled %}
-    {% call statement("main") %}
-        {{ build_sql }}
-    {% endcall %}
+    {% if build_sql_is_create_table_as %}
+      {#- Same reason as the temp build above: this batch is create_table_as
+          catalog DDL, so letting statement() open the ambient transaction
+          would hold its sysschobjs X keylocks until adapter.commit() and
+          deadlock a second worker. -#}
+      {% call statement("main", auto_begin=False) %}
+          {{ build_sql }}
+      {% endcall %}
+    {% else %}
+      {% call statement("main") %}
+          {{ build_sql }}
+      {% endcall %}
+    {% endif %}
+    {% if build_sql_is_create_table_as %}
+      {#- Reopen the ambient transaction the batch above declined to start,
+          so the swap and the tail (grants/persist_docs/masks/indexes/
+          post-hooks) keep their semantics and adapter.commit() below has a
+          matching BEGIN rather than raising Msg 3902. No-op when the flag is
+          off. -#}
+      {% do adapter.commit_if_open() %}
+      {% do adapter.begin_if_closed() %}
+    {% endif %}
   {% endif %}
 
   {% if need_swap %}

dbt/include/sqlserver/macros/relations/table/create.sql

@@ -9,6 +9,19 @@
     {%- endif -%}
     {%- set tmp_relation = relation.incorporate(path={"identifier": relation.identifier ~ '__dbt_tmp_vw'}, type='view') -%}
 
+    {#- Now that the incremental temp build commits standalone (see
+        incremental.sql), a crash can leave a throwaway table behind and the
+        SELECT * INTO below would hit Msg 2714. Drop it first, but only for
+        adapter-generated throwaways: `temporary` covers the incremental
+        __dbt_temp build, the suffix covers the full-refresh / table-refresh
+        __dbt_tmp intermediate. Suffix match is exact, never substring, so a
+        user model named stg__dbt_tmp_x is untouched.
+        Never guard a fresh-create of the real target: dbt has decided that
+        table does not exist, so 2714 must still surface rather than silently
+        destroying an object dbt does not know about. -#}
+    {%- set _ident = relation.identifier -%}
+    {%- set build_into_temp = temporary or _ident.endswith('__dbt_tmp') or _ident.endswith('__dbt_tmp_vw') -%}
+
     {%- do adapter.drop_relation(tmp_relation) -%}
     USE [{{ relation.database }}];
     {{ get_create_view_as_sql(tmp_relation, sql) }}
@@ -33,6 +46,10 @@
             SELECT {{listColumns}} FROM {{tmp_relation}} {{ query_label }}
 
         {% else %}
+            {%- if build_into_temp -%}
+            IF OBJECT_ID('{{ escape_single_quotes(relation.schema) }}.{{ escape_single_quotes(relation.identifier) }}', 'U') IS NOT NULL
+                EXEC('DROP TABLE {{ relation }}');
+            {%- endif -%}
             SELECT * INTO {{ table_name }} FROM {{ tmp_relation }} {{ query_label }}
         {% endif %}
     {%- endset -%}