diff --git a/CHANGELOG.md b/CHANGELOG.md index b5d0b230..e7d0ef2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +### Unreleased + +#### Bugfixes + +- 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()`. + ### v1.11.0 #### Features diff --git a/dbt/include/sqlserver/macros/adapters/indexes.sql b/dbt/include/sqlserver/macros/adapters/indexes.sql index c1514481..7c1b1c36 100644 --- a/dbt/include/sqlserver/macros/adapters/indexes.sql +++ b/dbt/include/sqlserver/macros/adapters/indexes.sql @@ -19,12 +19,13 @@ declare @drop_xml_indexes nvarchar(max); select @drop_xml_indexes = ( - select 'IF INDEXPROPERTY(' + CONVERT(VARCHAR(MAX), sys.tables.[object_id]) + ', ''' + sys.indexes.[name] + ''', ''IndexId'') IS NOT NULL DROP INDEX [' + sys.indexes.[name] + '] ON ' + '[' + SCHEMA_NAME(sys.tables.[schema_id]) + '].[' + OBJECT_NAME(sys.tables.[object_id]) + ']; ' + select 'IF INDEXPROPERTY(' + CONVERT(VARCHAR(MAX), sys.tables.[object_id]) + ', ' + QUOTENAME(sys.indexes.[name], '''') + ', ''IndexId'') IS NOT NULL DROP INDEX ' + QUOTENAME(sys.indexes.[name]) + ' ON ' + QUOTENAME(SCHEMA_NAME(sys.tables.[schema_id])) + '.' + QUOTENAME(OBJECT_NAME(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.[name] is not null and sys.indexes.type_desc = 'XML' + and SCHEMA_NAME(sys.tables.[schema_id]) = '{{ this.schema }}' and sys.tables.[name] = '{{ this.table }}' for xml path('') ); exec sp_executesql @drop_xml_indexes; @@ -39,12 +40,13 @@ declare @drop_spatial_indexes nvarchar(max); select @drop_spatial_indexes = ( - select 'IF INDEXPROPERTY(' + CONVERT(VARCHAR(MAX), sys.tables.[object_id]) + ', ''' + sys.indexes.[name] + ''', ''IndexId'') IS NOT NULL DROP INDEX [' + sys.indexes.[name] + '] ON ' + '[' + SCHEMA_NAME(sys.tables.[schema_id]) + '].[' + OBJECT_NAME(sys.tables.[object_id]) + ']; ' + select 'IF INDEXPROPERTY(' + CONVERT(VARCHAR(MAX), sys.tables.[object_id]) + ', ' + QUOTENAME(sys.indexes.[name], '''') + ', ''IndexId'') IS NOT NULL DROP INDEX ' + QUOTENAME(sys.indexes.[name]) + ' ON ' + QUOTENAME(SCHEMA_NAME(sys.tables.[schema_id])) + '.' + QUOTENAME(OBJECT_NAME(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.[name] is not null and sys.indexes.type_desc = 'Spatial' + and SCHEMA_NAME(sys.tables.[schema_id]) = '{{ this.schema }}' and sys.tables.[name] = '{{ this.table }}' for xml path('') ); exec sp_executesql @drop_spatial_indexes; @@ -56,12 +58,17 @@ {{ log("Running drop_fk_constraints() macro...") }} + {# The schema filter applies to the referenced table (this model). The + constraint itself is dropped from the referencing (parent) table, which + may legitimately live in another schema, so it is not filtered. #} + declare @drop_fk_constraints nvarchar(max); select @drop_fk_constraints = ( - select 'IF OBJECT_ID(''' + SCHEMA_NAME(CONVERT(VARCHAR(MAX), sys.foreign_keys.[schema_id])) + '.' + sys.foreign_keys.[name] + ''', ''F'') IS NOT NULL ALTER TABLE [' + SCHEMA_NAME(sys.foreign_keys.[schema_id]) + '].[' + OBJECT_NAME(sys.foreign_keys.[parent_object_id]) + '] DROP CONSTRAINT [' + sys.foreign_keys.[name]+ '];' + 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] - where sys.tables.[name] = '{{ this.table }}' + where SCHEMA_NAME(sys.tables.[schema_id]) = '{{ this.schema }}' + and sys.tables.[name] = '{{ this.table }}' for xml path('') ); exec sp_executesql @drop_fk_constraints; @@ -82,10 +89,11 @@ declare @drop_pk_constraints nvarchar(max); select @drop_pk_constraints = ( - select 'IF INDEXPROPERTY(' + CONVERT(VARCHAR(MAX), sys.tables.[object_id]) + ', ''' + sys.indexes.[name] + ''', ''IndexId'') IS NOT NULL ALTER TABLE [' + SCHEMA_NAME(sys.tables.[schema_id]) + '].[' + sys.tables.[name] + '] DROP CONSTRAINT [' + sys.indexes.[name]+ '];' + 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] where sys.indexes.is_primary_key = 1 + and SCHEMA_NAME(sys.tables.[schema_id]) = '{{ this.schema }}' and sys.tables.[name] = '{{ this.table }}' for xml path('') ); exec sp_executesql @drop_pk_constraints; @@ -103,12 +111,12 @@ declare @drop_remaining_indexes_last nvarchar(max); select @drop_remaining_indexes_last = ( - select 'IF INDEXPROPERTY(' + CONVERT(VARCHAR(MAX), sys.tables.[object_id]) + ', ''' + sys.indexes.[name] + ''', ''IndexId'') IS NOT NULL DROP INDEX [' + sys.indexes.[name] + '] ON ' + '[' + SCHEMA_NAME(sys.tables.[schema_id]) + '].[' + OBJECT_NAME(sys.tables.[object_id]) + ']; ' + select 'IF INDEXPROPERTY(' + CONVERT(VARCHAR(MAX), sys.tables.[object_id]) + ', ' + QUOTENAME(sys.indexes.[name], '''') + ', ''IndexId'') IS NOT NULL DROP INDEX ' + QUOTENAME(sys.indexes.[name]) + ' ON ' + QUOTENAME(SCHEMA_NAME(sys.tables.[schema_id])) + '.' + QUOTENAME(OBJECT_NAME(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.[name] is not null - and SCHEMA_NAME(sys.tables.schema_id) = '{{ this.schema }}' + and SCHEMA_NAME(sys.tables.[schema_id]) = '{{ this.schema }}' and sys.tables.[name] = '{{ this.table }}' for xml path('') ); exec sp_executesql @drop_remaining_indexes_last; diff --git a/tests/functional/adapter/mssql/test_index_macros.py b/tests/functional/adapter/mssql/test_index_macros.py index 0863876c..6a779634 100644 --- a/tests/functional/adapter/mssql/test_index_macros.py +++ b/tests/functional/adapter/mssql/test_index_macros.py @@ -68,6 +68,23 @@ select * from {{ ref('raw_data') }} """ +other_schema_pk_count = """ +select count(*) +from sys.key_constraints kc +inner join sys.tables t on kc.parent_object_id = t.object_id +where kc.[type] = 'PK' + and schema_name(t.schema_id) = '{schema_name}' + and t.[name] = '{table_name}' +""" + +other_schema_fk_count = """ +select count(*) +from sys.foreign_keys fk +inner join sys.tables t on fk.referenced_object_id = t.object_id +where schema_name(t.schema_id) = '{schema_name}' + and t.[name] = '{table_name}' +""" + class TestIndex: @pytest.fixture(scope="class") @@ -142,41 +159,68 @@ def create_table_and_index_other_schema(self, project): END """ + # Same table name as the dbt model, in a different schema. The primary + # key and the inbound foreign key are what the drop_pk_constraints() / + # drop_fk_constraints() legs of drop_all_indexes_on_table() reach. create_table = f""" CREATE TABLE {_schema}.index_model ( - IDCOL BIGINT + IDCOL BIGINT NOT NULL, + CONSTRAINT pk_other_index_model PRIMARY KEY (IDCOL) ) """ create_index = f""" CREATE INDEX sample_schema ON {_schema}.index_model (IDCOL) """ + + create_child_table = f""" + CREATE TABLE {_schema}.index_model_child ( + IDCOL BIGINT NOT NULL, + CONSTRAINT fk_other_index_model FOREIGN KEY (IDCOL) + REFERENCES {_schema}.index_model (IDCOL) + ) + """ with get_connection(project.adapter): project.adapter.execute(create_sql, fetch=True) project.adapter.execute(create_table) project.adapter.execute(create_index) + project.adapter.execute(create_child_table) def drop_schema_artifacts(self, project): _schema = project.test_schema + "other" + drop_child_table = f"DROP TABLE IF EXISTS {_schema}.index_model_child" drop_index = f"DROP INDEX IF EXISTS sample_schema ON {_schema}.index_model" drop_table = f"DROP TABLE IF EXISTS {_schema}.index_model" drop_schema = f"DROP SCHEMA IF EXISTS {_schema}" with get_connection(project.adapter): - project.adapter.execute(drop_index, fetch=True) + project.adapter.execute(drop_child_table, fetch=True) + project.adapter.execute(drop_index) project.adapter.execute(drop_table) project.adapter.execute(drop_schema) def validate_other_schema(self, project): + _schema = project.test_schema + "other" with get_connection(project.adapter): result, table = project.adapter.execute( - indexes_def.format( - schema_name=project.test_schema + "other", table_name="index_model" - ), + indexes_def.format(schema_name=_schema, table_name="index_model"), + fetch=True, + ) + + _, pk_table = project.adapter.execute( + other_schema_pk_count.format(schema_name=_schema, table_name="index_model"), + fetch=True, + ) + + _, fk_table = project.adapter.execute( + other_schema_fk_count.format(schema_name=_schema, table_name="index_model"), fetch=True, ) - assert len(table.rows) == 1 + # The nonclustered index plus the clustered index backing the primary key. + assert len(table.rows) == 2 + assert pk_table.rows[0][0] == 1 + assert fk_table.rows[0][0] == 1 def test_create_index(self, project): self.create_table_and_index_other_schema(project)