fix(indexes): ensure CCI created on final relation, not __dbt_tmp (#578)

[mopthe]

Fixes #578 - Creating nonclustered indexes in a SQL Server materialized table was resulting in an orphaned clustered columnstore index.

The key fix: index names are now derived from the final relation name (with __dbt_tmp / __dbt_backup suffixes stripped). This ensures:

• A second run produces an identical index name (idempotent)
• CCIs are not orphaned after intermediate relations are renamed
• No conflicts when drop_all_indexes_on_table runs

Add comprehensive integration and unit tests covering incremental runs, INCLUDE columns, quoted identifiers, and idempotence.

[Copilot]

Pull request overview

This PR fixes SQL Server index-macro behavior to prevent “orphaned” clustered columnstore index (CCI) naming during materializations that involve intermediate __dbt_tmp / __dbt_backup relations, by deriving index names from the final relation name and improving identifier quoting. It also adds unit + integration coverage around determinism/idempotence, INCLUDE columns, and quoted identifiers.

Changes:

• Refactors index macros to use deterministic index naming derived from the final relation identifier (suffix-stripped) and improved quoting.
• Updates the CCI creation macro to avoid __dbt_tmp / __dbt_backup leaking into the index name.
• Adds comprehensive unit tests for macro rendering plus live SQL Server integration tests validating idempotence, INCLUDE semantics, quoted identifiers, and the orphaned-CCI regression.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
dbt/include/sqlserver/macros/adapters/indexes.sql	Adds quoting/qualification helpers and refactors index/CCI macros to produce deterministic names and safer SQL.
dbt/include/sqlserver/macros/relations/table/create.sql	Updates the inline comment to document the new “final relation name” index naming behavior.
tests/unit/adapters/mssql/test_indexes.py	Adds Jinja-rendered unit tests for quoting, suffix-stripping, deterministic names, and emitted SQL shapes.
tests/functional/adapter/mssql/test_index_macros.py	Adds integration tests against live SQL Server for incremental stability, INCLUDE columns, quoted identifiers, and orphaned-CCI regression.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mopthe]

Migration Note: Orphaned Indexes

This release prevents orphaned CCI indexes from being created on intermediate __dbt_tmp tables across all materializations.

• table materialization: No action required. The table is fully recreated on each run, so any pre-existing orphaned indexes are dropped automatically.
• incremental materialization: Orphaned indexes created by prior versions persist and must be cleaned up manually (see below).

Cleaning up orphaned indexes

-- Identify orphaned CCI indexes
SELECT
    name AS index_name,
    OBJECT_SCHEMA_NAME(object_id) AS schema_name,
    OBJECT_NAME(object_id) AS table_name
FROM sys.indexes
WHERE name LIKE '%__dbt_tmp%cci';

-- Drop each one
DROP INDEX [index_name] ON [schema_name].[table_name];

[Benjamin-Knight]

I think you have converted the file endings because the file is showing a huge number of changes and I don't think there are any there, it makes the pull request difficult to parse.

[mopthe]

@Benjamin-Knight Thanks! Should be readable now.

[axellpadilla]

mssql__ is not used anywhere on the repo, also I think a quoting macro already exists, adapter.quote, quotename, escape_single_quotes, can you check please for standarization and re-use where possible

[axellpadilla]

After a careful analysis, this is a bug with how Commvault works, we deliberatelly chose to make a swap and avoid building this heavy index while the table is being read, this approach doesnt fix the issue because commvault just uses names to check object ownership, can you warrant it indeed fixes it @mopthe ?

[mopthe]

Thanks for the Commvault context. Just to clarify, this PR only changes the CCI name by removing the __dbt_tmp suffix. It doesn't change when the index is created. The CCI is still built on the __dbt_tmp table before the swap.

If Commvault tracks ownership by index name, consistent naming should improve compatibility, not break it. Could you share more detail about the specific interaction you're seeing?

[Benjamin-Knight]

The index work I did also means that a certain prefix is required for DBT to consider indexes part of its managed set for that code, and that it could drop codes if you enable the flag that DBT does not think its managing.

I believe CCIs are exempt from this but best to check how your changes interact with that code.

[mopthe]

@Benjamin-Knight You are right that CCI is exempt by type, not name. Reconciliation checks type == 'clustered columnstore' directly from sys.indexes, renaming it has no effect on whether it's protected from drops.