fix(table): stop Sch-M locks spanning the load in table builds

CONTRIBUTING.md

@@ -32,6 +32,19 @@ The functional tests require a running SQL Server instance. You can easily spin
 make server
 ```
 
+If you would rather not run Docker, there is a rootless [podman](https://podman.io/)
+equivalent that builds the same image and passes the same environment, so
+`test.env` works unchanged:
+
+```shell
+make server-podman        # build and start
+make server-podman-logs   # follow init; ready at "user creation completed"
+make server-podman-stop   # remove the container
+```
+
+Override `MSSQL_VERSION` to test against another release, e.g.
+`make server-podman MSSQL_VERSION=2019`.
+
 ### Backend requirements at a glance
 
 | Backend | Python package | Debian/Ubuntu system packages |

Makefile

@@ -1,5 +1,8 @@
 .DEFAULT_GOAL:=help
 THREADS ?= auto
+MSSQL_VERSION ?= 2022
+PODMAN_IMAGE ?= dbt-sqlserver-mssql:$(MSSQL_VERSION)
+PODMAN_CONTAINER ?= dbt-sqlserver-mssql
 
 .PHONY: dev
 dev: ## Installs adapter in develop mode along with development dependencies
@@ -57,6 +60,33 @@ server: ## Spins up a local MS SQL Server instance for development. Docker-compo
 	@\
 	docker compose up -d
 
+# Podman equivalents of `server`, for anyone who would rather not run Docker
+# Desktop. They build the same devops/server.Dockerfile and pass the same
+# environment docker-compose.yml does, so test.env works unchanged.
+.PHONY: server-podman
+server-podman: ## Spins up the same SQL Server instance under rootless podman.
+	@\
+	podman build -t $(PODMAN_IMAGE) --build-arg MSSQL_VERSION=$(MSSQL_VERSION) \
+		-f devops/server.Dockerfile devops && \
+	podman rm -f $(PODMAN_CONTAINER) >/dev/null 2>&1 || true; \
+	podman run -d --name $(PODMAN_CONTAINER) \
+		-e ACCEPT_EULA=Y \
+		-e SA_PASSWORD='L0calTesting!' \
+		-e COLLATION='SQL_Latin1_General_CP1_CS_AS' \
+		--env-file test.env \
+		-p 1433:1433 \
+		$(PODMAN_IMAGE)
+
+.PHONY: server-podman-stop
+server-podman-stop: ## Removes the podman SQL Server instance.
+	@\
+	podman rm -f $(PODMAN_CONTAINER)
+
+.PHONY: server-podman-logs
+server-podman-logs: ## Tails the podman SQL Server logs (init completes on "user creation completed").
+	@\
+	podman logs -f $(PODMAN_CONTAINER)
+
 .PHONY: clean
 clean: ## Removes ignored files and build artifacts from the repo.
 	@echo "cleaning repo"

README.md

@@ -177,6 +177,21 @@ The same setting is also honoured via `vars:` for backwards compatibility; the b
 
 Safe expansions are further gated by `column_type_expansion_max_rows` (default 1,000,000 rows) to avoid long-running operations on large tables.
 
+### `pre_hook_transaction_scope`
+
+_(default: `load`)_ Where a `table`, `incremental` or `snapshot` build resolves its schema
+(the tmp view and the empty `CREATE`) relative to its in-transaction pre-hooks.
+`load` stages it before them, so the new table's `Sch-M` lock is released in an
+instant and the load blocks no metadata reader in other sessions; a
+`transaction: true` pre-hook still rolls back with a failed load, except on the
+two paths that commit a full-refresh marker before the load
+(`full_refresh_build: prebuilt`, and an incremental `--full-refresh` of an
+existing table), where neither scope can roll it back. `build` stages
+it inside the hook's transaction, for the one case `load` cannot serve: a
+`transaction: true` pre-hook that creates an object the model reads. See
+[docs/transaction_scope.md](docs/transaction_scope.md) for the full flow and
+the post-hook ordering change.
+
 ### `dbt_sqlserver_use_dbt_transactions`
 
 _(default: `true`)_ Makes dbt's transaction hooks real at the SQL Server level by emitting `BEGIN TRANSACTION` / `COMMIT TRANSACTION` through the adapter's `add_begin_query` and `add_commit_query` methods.

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

@@ -52,7 +52,9 @@
 
 {#- Lower-cased names of every database principal, for the existence guard. -#}
 {% macro sqlserver__get_existing_principals() %}
-    {% call statement('get_existing_principals', fetch_result=True) %}
+    {#- Read-only probe: auto_begin=False so it cannot open the ambient
+        transaction - see sqlserver__get_columns_in_relation (#819). -#}
+    {% call statement('get_existing_principals', fetch_result=True, auto_begin=False) %}
         select name from sys.database_principals {{ information_schema_hints() }}
     {% endcall %}
     {% set result = [] %}

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

@@ -42,7 +42,13 @@
     columnstore index (it reports every column as included, never as a key), so
     a normal columnstore table has no index-key columns and masks apply freely. -#}
 {% macro sqlserver__get_mask_index_key_columns(relation) %}
-    {% call statement('get_mask_index_key_columns', fetch_result=True) %}
+    {#- Read-only probe: auto_begin=False so it cannot OPEN the ambient
+        transaction. It still joins one that is already open, so callers
+        that legitimately run inside a transaction are unaffected; what it
+        stops is a probe in the post-cutover tail reopening a transaction
+        that the following mask/index DDL then joins and holds to commit
+        (dbt-msft/dbt-sqlserver#819). -#}
+    {% call statement('get_mask_index_key_columns', fetch_result=True, auto_begin=False) %}
         select distinct col.name as name
         from sys.index_columns ic {{ information_schema_hints() }}
         inner join sys.columns col {{ information_schema_hints() }}
@@ -61,7 +67,9 @@
 {#- Columns that DDM cannot mask at all (a mask ALTER would fail): computed,
     FILESTREAM, sparse COLUMN_SET, and Always Encrypted columns. -#}
 {% macro sqlserver__get_unmaskable_columns(relation) %}
-    {% call statement('get_unmaskable_columns', fetch_result=True) %}
+    {#- Read-only probe: auto_begin=False so it cannot open the ambient
+        transaction - see sqlserver__get_columns_in_relation (#819). -#}
+    {% call statement('get_unmaskable_columns', fetch_result=True, auto_begin=False) %}
         select col.name as name
         from sys.columns col {{ information_schema_hints() }}
         where col.object_id = OBJECT_ID('{{ escape_single_quotes(relation.include(database=False)) }}')

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

@@ -90,7 +90,13 @@
 
 {% macro sqlserver__get_columns_in_relation(relation) -%}
     {% set query_label = get_query_options() %}
-    {% call statement('get_columns_in_relation', fetch_result=True) %}
+    {#- Read-only probe: auto_begin=False so it cannot OPEN the ambient
+        transaction. It still joins one that is already open, so callers
+        that legitimately run inside a transaction are unaffected; what it
+        stops is a probe in the post-cutover tail reopening a transaction
+        that the following mask/index DDL then joins and holds to commit
+        (dbt-msft/dbt-sqlserver#819). -#}
+    {% call statement('get_columns_in_relation', fetch_result=True, auto_begin=False) %}
         {{ get_use_database_sql(relation.database) }}
         select
             c.name collate database_default as column_name,

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

@@ -200,7 +200,9 @@
 
 
 {% macro drop_fk_indexes_on_table(relation) -%}
-  {% call statement('find_references', fetch_result=true) %}
+  {#- Read-only probe: auto_begin=False so it cannot open the ambient
+      transaction - see sqlserver__get_columns_in_relation (#819). -#}
+  {% call statement('find_references', fetch_result=true, auto_begin=false) %}
       {{ get_use_database_sql(relation.database) }}
       SELECT  obj.name AS FK_NAME,
       sch.name AS [schema_name],
@@ -232,7 +234,9 @@
 {% endmacro %}
 
 {% macro sqlserver__list_nonclustered_rowstore_indexes(relation) -%}
-  {% call statement('list_nonclustered_rowstore_indexes', fetch_result=True) -%}
+  {#- Read-only probe: auto_begin=False so it cannot open the ambient
+      transaction - see sqlserver__get_columns_in_relation (#819). -#}
+  {% call statement('list_nonclustered_rowstore_indexes', fetch_result=True, auto_begin=False) -%}
 
     SELECT i.name AS index_name
     , i.name + '__dbt_backup' as index_new_name
@@ -381,7 +385,13 @@
 
 
 {% macro sqlserver__describe_indexes(relation) %}
-  {% call statement('describe_indexes', fetch_result=True) -%}
+  {#- Read-only probe: auto_begin=False so it cannot OPEN the ambient
+      transaction. It still joins one that is already open, so callers
+      that legitimately run inside a transaction are unaffected; what it
+      stops is a probe in the post-cutover tail reopening a transaction
+      that the following mask/index DDL then joins and holds to commit
+      (dbt-msft/dbt-sqlserver#819). -#}
+  {% call statement('describe_indexes', fetch_result=True, auto_begin=False) -%}
     select
         i.[name] as [name],
         case when i.[type] = 1 then 'clustered'

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

@@ -21,3 +21,38 @@
     {% endif %}
   {% endfor %}
 {% endmacro %}
+
+
+{% macro sqlserver__pre_hook_transaction_scope() -%}
+  {#-
+    Resolve pre_hook_transaction_scope: where schema resolution (the tmp view
+    and the empty CREATE) sits relative to the in-transaction pre-hooks.
+    Shared by table, incremental and snapshot. Full detail in
+    docs/transaction_scope.md.
+
+      load (default)                         build
+      ------------------------------------   ------------------------------------
+      stage            autocommit            BEGIN
+      BEGIN                                  |- in-tx pre-hooks
+      |- in-tx pre-hooks                     |- stage         Sch-M on new object
+      |- load          X table lock only     |- load          ... held to COMMIT
+      |- cutover, masks, in-tx post-hooks    |- cutover, masks, in-tx post-hooks
+      COMMIT                                 COMMIT
+      |- view drops, indexes, grants, docs   |- view drops, indexes, grants, docs
+
+    Both keep a transaction: true pre-hook atomic with the load. load fixes
+    #819 (Sch-M conflicts with the Sch-S every metadata reader takes; an X
+    table lock does not) but needs the model SQL to bind before the hooks run: a
+    transaction: true pre-hook that creates an object the model reads fails
+    at the stage with Msg 208. Remedies: transaction: false on that hook
+    (outside-tx hooks run before the stage), or build for that model.
+  -#}
+  {%- set scope = config.get('pre_hook_transaction_scope', 'load') -%}
+  {%- if scope not in ['load', 'build'] -%}
+    {{ exceptions.raise_compiler_error(
+      "Invalid pre_hook_transaction_scope '" ~ scope ~ "'. "
+      "Valid values are: 'load' (default), 'build'."
+    ) }}
+  {%- endif -%}
+  {{ return(scope) }}
+{%- endmacro %}