fix(dml-refresh): rebuild the scratch table the way every other path does

A model with table_refresh_method: dml lost its clustered columnstore index the
first time its schema changed, and never got it back.

That path builds a scratch table and, when the columns no longer match, renames
it into position. The scratch is built with SELECT * INTO, which copies no
index and no constraint and takes nullability from the query; create_indexes
then builds only what the `indexes` config names, never the as_columnstore CCI.
So the model came back as a heap and stayed one, because every later run
matched the new schema and took the DELETE+INSERT path. Under an enforced
contract the same rename also dropped the model's NOT NULLs and its inline
constraints.

That branch now rebuilds the scratch through create_table_as before renaming
it, which is simply how this adapter creates a table, so the columnstore index
and the full column DDL carry across the swap. SELECT * INTO stays as the
schema probe, and the rebuild is confined to the branch that actually renames:
doing it up front would build, and then throw away, a columnstore index on
every steady-state refresh, which on a large table dominates the run. A schema
change is rare, so one extra build there is much the cheaper trade.

The cost on that run is a second execution of the model's SQL - the probe
having already run it once - plus the extra columnstore build. Probing the tmp
view rather than the materialized scratch would avoid the double execution, but
that changes how the probe behaves and belongs in its own change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: lll86789

CHANGELOG.md

@@ -17,6 +17,7 @@
 
 - Fix `primary_key`, `foreign_key`, `unique` and `check` constraints declared in a contract-enforced model's yaml never reaching the database - only `not_null` was ever emitted. `render_column_constraint` returned an empty string for every other type, and `sqlserver__build_model_constraints`, which its docstring pointed at as the place those were applied instead, was defined but called from nowhere, so model-level constraints were dropped outright. Column-level constraints now render inline in the `CREATE TABLE` column list, and model-level constraints render there too when they carry no `name:`. A model-level constraint *with* a `name:` is applied by `ALTER TABLE ... ADD CONSTRAINT` once the build has swapped the new table in and dropped the old one, which is the first point at which that name is free: SQL Server scopes constraint names per schema (unlike index names, which are scoped per table), so emitting a name inline would collide with the table being replaced on every rebuild after the first. A `name:` on a *column-level* constraint is ignored with a warning pointing at the model-level form. `PRIMARY KEY` and `UNIQUE` default to `NONCLUSTERED` so they can coexist with the clustered columnstore index built for `as_columnstore` (the default); declare `expression: clustered` on the constraint to override that. Foreign keys accept both the `to:` / `to_columns:` form and the older free-text `expression:` form - previously only the latter was matched, so even a wired-up model constraint using `to:` would have been silently discarded. Note that a foreign key pointing at a model makes that model's rebuild fail with `Msg 3726` while the swap's backup table is dropped; set `table_refresh_method: dml` on the referenced model to keep its table object across refreshes. [#579](https://github.com/dbt-msft/dbt-sqlserver/issues/579)
 - Apply named model constraints on every build path rather than only on the build that creates the table. Each `ALTER TABLE ... ADD CONSTRAINT` is now guarded on the name already being present on the table, so a constraint added to an existing `incremental` model (or to a model using `table_refresh_method: dml`) lands on its next run instead of silently doing nothing until `--full-refresh`. A constraint whose *definition* changes under an unchanged name is still not detected - a constraint name, unlike a `dbt_idx_` index name, is not a hash of its definition - and needs `--full-refresh`; see the README.
+- Fix a model with `table_refresh_method: dml` losing its clustered columnstore index - and, under an enforced contract, its constraints and `NOT NULL`s as well - the first time its schema changed. That path builds a scratch table and, when the columns no longer match, renames it into position; the scratch was built with `SELECT * INTO`, which copies no constraint and no index and infers nullability from the query rather than from the contract, and `create_indexes` only builds what the `indexes` config names, never the `as_columnstore` CCI. So the model came back stripped and stayed that way, since every later run matched the new schema and took the DELETE+INSERT path. That branch now rebuilds the scratch table through `create_table_as` - the way every other build path in the adapter creates a table - which carries the full column DDL and the columnstore index across the swap. On a schema-change run - and only there - that costs a second execution of the model's SQL (once for the `SELECT ... INTO` schema probe, once for the rebuild) and one extra columnstore build. It gives up the minimally-logged `SELECT ... INTO` for that run, though `INSERT ... WITH (TABLOCK)` is itself minimally logged under the simple and bulk-logged recovery models, so the extra log volume lands on full-recovery databases only. Steady-state refreshes are unchanged.
 
 - Fix a `view` model whose SQL text is unchanged (and so skips its `CREATE`/`ALTER`) going stale when a table it selects `*` from gains, loses, or reorders columns. SQL Server resolves an unqualified `select *` and caches the result at `CREATE`/`ALTER VIEW` time; skipping that statement left the cached column list silently out of sync with the underlying table, so the view kept serving old columns under their old names/positions even though every dbt run reported success. The skip path now runs `sp_refreshview` against the view instead of a no-op, which re-derives the cached metadata from the table's current shape without reissuing the `CREATE`. The refresh carries the model's database as a `USE` prefix, so a cross-database view model refreshes the intended object rather than erroring on a name that does not resolve in the connection's current database. Note that a metadata refresh advances the view's `sys.objects.modify_date` just as an `ALTER` would, so that column no longer distinguishes a skipped run from a rebuild.
 - 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.

README.md

@@ -359,6 +359,8 @@ Every bullet above is about *named* constraints. Unnamed ones ride the `CREATE T
 
 #### Build-shape notes
 
+With `table_refresh_method: dml`, a schema change makes the refresh fall back to a rename-swap. On that run — and only that run — the scratch table is rebuilt through `CREATE TABLE … INSERT … WITH (TABLOCK)`, so it carries the model's columnstore index, and under an enforced contract its `NOT NULL`s and inline constraints, into the swap. That run therefore executes the model's SQL twice — once for the `SELECT … INTO` that probes for the schema change, once for the rebuild. Steady-state refreshes are unaffected and keep the single, cheaper `SELECT … INTO`.
+
 An *unnamed* `primary_key` or `unique` constraint on a column that also carries a [data mask](#dynamic-data-masking-masked_with--masks) is rejected by the build. The constraint rides the `CREATE TABLE`, so its index already exists by the time `apply_masks` runs, and the adapter refuses to mask any column that an index has as a key: *is configured for masking but is also an index key column*. Declare that constraint at the model level **with a `name:`** instead — named constraints are applied by `ALTER TABLE` after the masks are in place, which the adapter allows.
 
 With `full_refresh_build: prebuilt`, a `primary_key` or `unique` constraint creates a nonclustered index on the table *before* the bulk load. That secondary index has to be maintained row by row during `INSERT … WITH (TABLOCK)`, which is fully logged and adds to a load that `prebuilt` exists to make cheap. If a model is on `prebuilt` because its load time matters, weigh the key constraints against that; `check`, `not_null` and `foreign_key` do not create indexes and do not carry this cost.

dbt/include/sqlserver/macros/materializations/models/table/table_dml_refresh.sql

@@ -110,6 +110,35 @@
     {%- set backup_relation = make_backup_relation(target_relation, backup_relation_type) -%}
     {{ drop_relation_if_exists(backup_relation) }}
 
+    {#- The scratch table above came from SELECT * INTO, which is the right
+        shape for the schema probe and the wrong one for the object that is
+        about to be renamed into position: it copies no constraint and no
+        index, and takes nullability from the query rather than from a
+        contract. Left as-is it silently strips the model of its clustered
+        columnstore index - create_indexes only builds what the `indexes`
+        config names, never the as_columnstore CCI - and, under a contract, of
+        its NOT NULLs and inline constraints too. None of it came back on a
+        later run, because every later run matched the new schema and took the
+        DELETE+INSERT path above.
+
+        So rebuild it the way this adapter builds every other table. The
+        rebuild belongs on this branch alone: doing it up front would build,
+        and then throw away, a full columnstore index on every steady-state
+        refresh, which on a large table dominates the run. A schema change is
+        rare, so one extra build here is much the cheaper trade.
+
+        It is not free, though: the model's SQL runs a second time here, the
+        SELECT * INTO above having already run it once as the schema probe.
+        Probing the tmp view instead of the materialized scratch would avoid
+        that, at the cost of changing how the probe behaves - a separate
+        change, not this one.
+
+        create_table_as builds and drops its own __dbt_tmp_vw. -#}
+    {{ drop_relation_if_exists(refresh_relation) }}
+    {% call statement('dml_refresh_rebuild') -%}
+      {{ get_create_table_as_sql(False, refresh_relation, sql) }}
+    {%- endcall %}
+
     {# Rename scratch table into position #}
     {% set existing_relation = load_cached_relation(target_relation) %}
     {% if existing_relation is not none %}
@@ -118,7 +147,7 @@
 
     {{ adapter.rename_relation(refresh_relation, target_relation) }}
 
-    {# Rebuilt via SELECT INTO (no masks carried), so apply masks before
+    {# Freshly rebuilt (no masks carried), so apply masks before
        create_indexes — a mask cannot be added to a column an index depends
        on (documented for all SQL Server versions). #}
     {% do apply_masks(target_relation, mask_config) %}

tests/functional/adapter/mssql/test_constraints_applied.py

@@ -152,6 +152,42 @@ def model_sql(**config):
 """
 
 
+dml_model_sql = model_sql(table_refresh_method="dml")
+
+# Adds a column, which is what pushes the DML refresh onto its rename-swap
+# fallback.
+dml_model_wider_sql = dml_model_sql + ", 'x' as extra"
+
+dml_schema_yml = """
+version: 2
+models:
+  - name: dml_model
+    config:
+      contract:
+        enforced: true
+    constraints:
+      - type: primary_key
+        name: PK_dml_model
+        columns: [id]
+      - type: check
+        expression: id > 0
+    columns:
+      - name: id
+        data_type: int
+        constraints:
+          - type: not_null
+      - name: color
+        data_type: varchar(100)
+"""
+
+dml_schema_wider_yml = (
+    dml_schema_yml
+    + """      - name: extra
+        data_type: varchar(100)
+"""
+)
+
+
 fk_parent_sql = """
 {{ config(materialized='table', as_columnstore=False) }}
 select 1 as id
@@ -236,6 +272,23 @@ def _index_type(project, table, index_name):
     return _indexes(project, table).get(index_name)
 
 
+def _not_null_columns(project, table):
+    rows = project.run_sql(
+        f"""
+        select c.name
+        from sys.columns c
+        where c.object_id = OBJECT_ID('{project.test_schema}.{table}')
+          and c.is_nullable = 0
+        """,
+        fetch="all",
+    )
+    return sorted(row[0] for row in rows)
+
+
+def _has_columnstore(project, table):
+    return "CLUSTERED COLUMNSTORE" in _indexes(project, table).values()
+
+
 class TestNamedModelConstraints:
     @pytest.fixture(scope="class")
     def models(self):
@@ -395,6 +448,47 @@ def test_it_lands_on_the_next_incremental_run(self, project):
         ]
 
 
+class TestDmlRefreshKeepsConstraints:
+    """table_refresh_method='dml' rebuilds through a rename-swap whenever the
+    schema changes. That rebuild used to land a table built by SELECT * INTO,
+    which carries no constraints, no NOT NULL and no columnstore index.
+
+    One test per class - see TestConstraintAddedToAnExistingModel.
+    """
+
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "dml_model.sql": dml_model_sql,
+            "schema.yml": dml_schema_yml,
+        }
+
+    def test_constraints_survive_the_schema_change_fallback(self, project):
+        run_dbt(["run"])
+        before = _constraints(project, "dml_model")
+        assert before.get("PRIMARY_KEY_CONSTRAINT") == ["PK_dml_model"]
+        assert len(before.get("CHECK_CONSTRAINT", [])) == 1
+        assert _not_null_columns(project, "dml_model") == ["id"]
+        # Default as_columnstore, so the table is built on a CCI.
+        assert _has_columnstore(project, "dml_model")
+
+        # A steady-state refresh keeps the table object and everything on it.
+        run_dbt(["run"])
+        assert _constraints(project, "dml_model").get("PRIMARY_KEY_CONSTRAINT") == ["PK_dml_model"]
+
+        # Add a column: the DML refresh cannot swap by DELETE+INSERT any more
+        # and falls back to rename-swap.
+        write_file(dml_model_wider_sql, "models", "dml_model.sql")
+        write_file(dml_schema_wider_yml, "models", "schema.yml")
+        run_dbt(["run"])
+
+        after = _constraints(project, "dml_model")
+        assert after.get("PRIMARY_KEY_CONSTRAINT") == ["PK_dml_model"]
+        assert len(after.get("CHECK_CONSTRAINT", [])) == 1
+        assert _not_null_columns(project, "dml_model") == ["id"]
+        assert _has_columnstore(project, "dml_model")
+
+
 class TestForeignKeyToRef:
     """`to: ref(...)` is the form dbt-core actually produces, and it resolves to
     a fully rendered relation - database included. T-SQL's REFERENCES grammar
@@ -482,3 +576,27 @@ def test_the_mask_and_the_constraint_coexist(self, project):
             fetch="all",
         )
         assert [row[0] for row in masked] == ["id"]
+
+
+plain_dml_model_sql = model_sql(table_refresh_method="dml")
+plain_dml_model_wider_sql = plain_dml_model_sql + ", 'x' as extra"
+
+
+class TestDmlRefreshKeepsTheColumnstoreWithoutAContract:
+    """The rename-swap fallback renames the scratch table into position, and
+    create_indexes only builds what the `indexes` config names - never the
+    as_columnstore CCI. Rebuilding the scratch through create_table_as is what
+    carries the columnstore across, and that is not contract-specific."""
+
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {"plain_dml_model.sql": plain_dml_model_sql}
+
+    def test_the_columnstore_survives_a_schema_change(self, project):
+        run_dbt(["run"])
+        assert _has_columnstore(project, "plain_dml_model")
+
+        write_file(plain_dml_model_wider_sql, "models", "plain_dml_model.sql")
+        run_dbt(["run"])
+
+        assert _has_columnstore(project, "plain_dml_model")