From 3639e5e3108b3b98e348c9792ec1e84c2f01be8c Mon Sep 17 00:00:00 2001 From: Axell Padilla <68310020+axellpadilla@users.noreply.github.com> Date: Sat, 16 May 2026 06:15:53 +0000 Subject: [PATCH 1/4] Fix: Handle view alterations and existing table replacements This commit addresses the following: - Modifies the `create view` macro to use `create or alter view` when the view already exists, preventing errors during re-materialization. - Updates the view materialization logic to correctly avoid renaming if no table exists. - Adds test cases to verify that materializing an existing view updates its definition correctly, including column changes, previous grants and avoids table changes if view fails. --- .../materializations/models/view/view.sql | 35 +++++++---- .../macros/relations/views/create.sql | 2 +- tests/functional/adapter/dbt/test_basic.py | 58 +++++++++++++++++- .../adapter/dbt/test_constraints.py | 4 +- .../adapter/mssql/test_materialize_change.py | 60 +++++++++++++++++++ 5 files changed, 145 insertions(+), 14 deletions(-) diff --git a/dbt/include/sqlserver/macros/materializations/models/view/view.sql b/dbt/include/sqlserver/macros/materializations/models/view/view.sql index 4ae35c9ef..aa26c1d89 100644 --- a/dbt/include/sqlserver/macros/materializations/models/view/view.sql +++ b/dbt/include/sqlserver/macros/materializations/models/view/view.sql @@ -26,6 +26,13 @@ {%- set preexisting_backup_relation = load_cached_relation(backup_relation) -%} -- grab current tables grants config for comparision later on {% set grant_config = config.get('grants') %} + {% set preserved_grants = {} %} + + {% if existing_relation is not none and existing_relation.type != 'view' %} + {% set current_grants_table = run_query(get_show_grant_sql(existing_relation)) %} + {% set current_grants_dict = adapter.standardize_grants_dict(current_grants_table) %} + {% set preserved_grants = diff_of_two_dicts(current_grants_dict, grant_config) %} + {% endif %} {{ run_hooks(pre_hooks, inside_transaction=False) }} @@ -36,26 +43,34 @@ -- `BEGIN` happens here: {{ run_hooks(pre_hooks, inside_transaction=True) }} - -- build model - {% call statement('main') -%} - {{ get_create_view_as_sql(intermediate_relation, sql) }} - {%- endcall %} + {% if existing_relation is not none and existing_relation.type != 'view' %} + -- build model + {% call statement('main') -%} + {{ get_create_view_as_sql(intermediate_relation, sql) }} + {%- endcall %} - -- cleanup - -- move the existing view out of the way - {% if existing_relation is not none %} - /* Do the equivalent of rename_if_exists. 'existing_relation' could have been dropped - since the variable was first set. */ + -- cleanup + -- move the existing relation out of the way {% set existing_relation = load_cached_relation(existing_relation) %} {% if existing_relation is not none %} {{ adapter.rename_relation(existing_relation, backup_relation) }} {% endif %} + + {{ adapter.rename_relation(intermediate_relation, target_relation) }} + {% else %} + -- build model + {% call statement('main') -%} + {{ get_create_view_as_sql(target_relation, sql) }} + {%- endcall %} {% endif %} - {{ adapter.rename_relation(intermediate_relation, target_relation) }} {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %} {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %} + {% if preserved_grants %} + {% do apply_grants(target_relation, preserved_grants, should_revoke=False) %} + {% endif %} + {% do persist_docs(target_relation, model) %} {{ run_hooks(post_hooks, inside_transaction=True) }} diff --git a/dbt/include/sqlserver/macros/relations/views/create.sql b/dbt/include/sqlserver/macros/relations/views/create.sql index ef1ba3034..874c2e562 100644 --- a/dbt/include/sqlserver/macros/relations/views/create.sql +++ b/dbt/include/sqlserver/macros/relations/views/create.sql @@ -7,7 +7,7 @@ {%- endif %} {% set query %} - create view {{ relation.include(database=False) }} as {{ sql }}; + CREATE OR ALTER VIEW {{ relation.include(database=False) }} AS {{ sql }}; {% endset %} {% set tst %} diff --git a/tests/functional/adapter/dbt/test_basic.py b/tests/functional/adapter/dbt/test_basic.py index bdbe38f86..028d078c8 100644 --- a/tests/functional/adapter/dbt/test_basic.py +++ b/tests/functional/adapter/dbt/test_basic.py @@ -1,3 +1,5 @@ +import os + import pytest from dbt.tests.adapter.basic.test_adapter_methods import BaseAdapterMethod from dbt.tests.adapter.basic.test_base import BaseSimpleMaterializations @@ -9,10 +11,64 @@ from dbt.tests.adapter.basic.test_singular_tests_ephemeral import BaseSingularTestsEphemeral from dbt.tests.adapter.basic.test_snapshot_check_cols import BaseSnapshotCheckCols from dbt.tests.adapter.basic.test_snapshot_timestamp import BaseSnapshotTimestamp +from dbt.tests.util import run_dbt class TestSimpleMaterializations(BaseSimpleMaterializations): - pass + + def test_existing_view_materialization(self, project, models): + """Test that materializing an existing view works correctly.""" + # Create a temporary model file directly in the project + model_path = os.path.join(project.project_root, "models", "view_model_exists.sql") + + # Write the initial model without the value column + with open(model_path, "w") as f: + f.write( + """ + {{ config(materialized='view') }} + select + 1 as id + {% if var('include_value_column', false) %} + , 2 as value + {% endif %} + """ + ) + + # First run to create the view without the extra column + results = run_dbt(["run", "-m", "view_model_exists"]) + assert len(results) == 1 + + # Generate catalog to get column information + catalog = run_dbt(["docs", "generate"]) + + # Check columns in the catalog + node_id = "model.base.view_model_exists" + assert node_id in catalog.nodes + + # Get columns from the catalog + columns = catalog.nodes[node_id].columns + column_names = [name.lower() for name in columns.keys()] + + # Verify only the id column exists + assert "id" in column_names + assert "value" not in column_names + + # Second run with a variable to include the extra column + results = run_dbt( + ["run", "-m", "view_model_exists", "--vars", '{"include_value_column": true}'] + ) + assert len(results) == 1 + + # Generate catalog again to get updated column information + catalog = run_dbt(["docs", "generate"]) + + # Get updated columns from the catalog + columns = catalog.nodes[node_id].columns + column_names = [name.lower() for name in columns.keys()] + + # Verify both columns exist now + assert "id" in column_names + assert "value" in column_names class TestSingularTests(BaseSingularTests): diff --git a/tests/functional/adapter/dbt/test_constraints.py b/tests/functional/adapter/dbt/test_constraints.py index d2ec1080a..a963bcc3e 100644 --- a/tests/functional/adapter/dbt/test_constraints.py +++ b/tests/functional/adapter/dbt/test_constraints.py @@ -471,7 +471,7 @@ def models(self): @pytest.fixture(scope="class") def expected_sql(self): return """ - EXEC(' create view as -- depends_on: select ''blue'' as color, 1 as id, ''2019-01-01'' as date_day; ') EXEC(' CREATE TABLE ( id int not null , color varchar(100), date_day varchar(100) ) INSERT INTO ( [id], [color], [date_day] ) SELECT [id], [color], [date_day] FROM ') EXEC('DROP VIEW IF EXISTS + EXEC(' CREATE OR ALTER VIEW AS -- depends_on: select ''blue'' as color, 1 as id, ''2019-01-01'' as date_day; ') EXEC(' CREATE TABLE ( id int not null , color varchar(100), date_day varchar(100) ) INSERT INTO ( [id], [color], [date_day] ) SELECT [id], [color], [date_day] FROM ') EXEC('DROP VIEW IF EXISTS """ # EXEC('DROP view IF EXISTS @@ -591,7 +591,7 @@ def models(self): @pytest.fixture(scope="class") def expected_sql(self): return """ - EXEC(' create view as -- depends_on: select ''blue'' as color, 1 as id, ''2019-01-01'' as date_day; ') EXEC(' CREATE TABLE ( id int not null , color varchar(100), date_day varchar(100) ) INSERT INTO ( [id], [color], [date_day] ) SELECT [id], [color], [date_day] FROM ') EXEC('DROP VIEW IF EXISTS + EXEC(' CREATE OR ALTER VIEW AS -- depends_on: select ''blue'' as color, 1 as id, ''2019-01-01'' as date_day; ') EXEC(' CREATE TABLE ( id int not null , color varchar(100), date_day varchar(100) ) INSERT INTO ( [id], [color], [date_day] ) SELECT [id], [color], [date_day] FROM ') EXEC('DROP VIEW IF EXISTS """ def test__model_constraints_ddl(self, project, expected_sql): diff --git a/tests/functional/adapter/mssql/test_materialize_change.py b/tests/functional/adapter/mssql/test_materialize_change.py index abd9f393c..9f31f05e3 100644 --- a/tests/functional/adapter/mssql/test_materialize_change.py +++ b/tests/functional/adapter/mssql/test_materialize_change.py @@ -23,6 +23,15 @@ SELECT 1 AS data """ +invalid_view_mat = """ +{{ + config({ + "materialized": 'view' + }) +}} +SELECT * FROM missing_relation +""" + schema = """ version: 2 models: @@ -50,6 +59,57 @@ def test_passes(self, project): run_dbt(["run"]) +class TestTabletoViewRollback(BaseTableView): + """Test that a failed table to view replacement leaves the original table intact.""" + + @pytest.fixture(scope="class") + def models(self): + return {"mat_object.sql": invalid_view_mat, "schema.yml": schema} + + def test_existing_table_is_preserved(self, project): + self.create_object( + project, f"SELECT * INTO {project.test_schema}.mat_object FROM ({model_sql}) t" + ) + + failing_results = run_dbt(["run"], expect_pass=False) + assert len(failing_results) == 1 + + rows = project.run_sql(f"select * from {project.test_schema}.mat_object", fetch="all") + assert len(rows) == 1 + assert rows[0][0] == 1 + + +class TestTabletoViewPreservesGrants(BaseTableView): + """Test that grants on the existing table are preserved on the replaced view.""" + + @pytest.fixture(scope="class") + def models(self): + return {"mat_object.sql": view_mat, "schema.yml": schema} + + def test_public_select_grant_survives_swap(self, project): + self.create_object( + project, f"SELECT * INTO {project.test_schema}.mat_object FROM ({model_sql}) t" + ) + project.run_sql(f"grant select on object::{project.test_schema}.mat_object to public") + + run_dbt(["run"]) + + grants = project.run_sql( + f""" + select pr.name as grantee, pe.permission_name + from sys.database_permissions pe + join sys.objects o on pe.major_id = o.object_id + join sys.schemas s on o.schema_id = s.schema_id + join sys.database_principals pr + on pe.grantee_principal_id = pr.principal_id + where s.name = '{project.test_schema}' + and o.name = 'mat_object' + """, + fetch="all", + ) + assert ("PUBLIC", "SELECT") in [(row[0].upper(), row[1].upper()) for row in grants] + + class TestViewtoTable(BaseTableView): """Test if changing from a view object to a table object correctly replaces""" From cc6da9956d93dca35952fcf6bd06c83a20e5b9c1 Mon Sep 17 00:00:00 2001 From: Axell Padilla <68310020+axellpadilla@users.noreply.github.com> Date: Sat, 16 May 2026 06:36:32 +0000 Subject: [PATCH 2/4] feat: Add SQL Server view materialization no-op optimization Add get_view_definition_sql metadata macro for SQL Server views Detect unchanged view definition and skip rebuild by using a no-op statement Add test ensuring rerunning an unchanged view does not alter modify_date --- .../sqlserver/macros/adapters/metadata.sql | 14 +++++++ .../materializations/models/view/view.sql | 22 ++++++++++- .../adapter/mssql/test_materialize_change.py | 38 +++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/dbt/include/sqlserver/macros/adapters/metadata.sql b/dbt/include/sqlserver/macros/adapters/metadata.sql index 4d550fcd9..1af96ccd2 100644 --- a/dbt/include/sqlserver/macros/adapters/metadata.sql +++ b/dbt/include/sqlserver/macros/adapters/metadata.sql @@ -90,6 +90,20 @@ {{ return(load_result('get_relation_without_caching').table) }} {% endmacro %} +{% macro get_view_definition_sql(relation) %} + {{ return(adapter.dispatch('get_view_definition_sql')(relation)) }} +{% endmacro %} + +{% macro sqlserver__get_view_definition_sql(relation) -%} + {{ get_use_database_sql(relation.database) }} + select object_definition(v.object_id) as definition + from sys.views as v {{ information_schema_hints() }} + inner join sys.schemas as s {{ information_schema_hints() }} + on v.schema_id = s.schema_id + where upper(s.name) = upper('{{ relation.schema }}') + and upper(v.name) = upper('{{ relation.identifier }}') +{% endmacro %} + {% macro sqlserver__get_relation_last_modified(information_schema, relations) -%} {%- call statement('last_modified', fetch_result=True) -%} select diff --git a/dbt/include/sqlserver/macros/materializations/models/view/view.sql b/dbt/include/sqlserver/macros/materializations/models/view/view.sql index aa26c1d89..146c7315d 100644 --- a/dbt/include/sqlserver/macros/materializations/models/view/view.sql +++ b/dbt/include/sqlserver/macros/materializations/models/view/view.sql @@ -27,11 +27,29 @@ -- grab current tables grants config for comparision later on {% set grant_config = config.get('grants') %} {% set preserved_grants = {} %} + {% set should_skip_view_update = false %} + {% set build_sql = none %} {% if existing_relation is not none and existing_relation.type != 'view' %} {% set current_grants_table = run_query(get_show_grant_sql(existing_relation)) %} {% set current_grants_dict = adapter.standardize_grants_dict(current_grants_table) %} {% set preserved_grants = diff_of_two_dicts(current_grants_dict, grant_config) %} + {% set build_sql = get_create_view_as_sql(intermediate_relation, sql) %} + {% elif existing_relation is not none and existing_relation.type == 'view' %} + {% set current_view_definition_table = run_query(get_view_definition_sql(existing_relation)) %} + {% if current_view_definition_table is not none and current_view_definition_table.rows | length > 0 %} + {% set normalized_relation = target_relation.include(database=False) | lower | replace('\n', '') | replace('\r', '') | replace('\t', '') | replace(' ', '') | replace(';', '') %} + {% set normalized_sql = sql | lower | replace('\n', '') | replace('\r', '') | replace('\t', '') | replace(' ', '') | replace(';', '') %} + {% set normalized_definition = current_view_definition_table.rows[0][0] | lower | replace('\n', '') | replace('\r', '') | replace('\t', '') | replace(' ', '') | replace(';', '') %} + {% set should_skip_view_update = normalized_definition.endswith(normalized_sql) %} + {% endif %} + {% if should_skip_view_update %} + {% set build_sql = 'declare @dbt_sqlserver_noop int;' %} + {% else %} + {% set build_sql = get_create_view_as_sql(target_relation, sql) %} + {% endif %} + {% else %} + {% set build_sql = get_create_view_as_sql(target_relation, sql) %} {% endif %} {{ run_hooks(pre_hooks, inside_transaction=False) }} @@ -46,7 +64,7 @@ {% if existing_relation is not none and existing_relation.type != 'view' %} -- build model {% call statement('main') -%} - {{ get_create_view_as_sql(intermediate_relation, sql) }} + {{ build_sql }} {%- endcall %} -- cleanup @@ -60,7 +78,7 @@ {% else %} -- build model {% call statement('main') -%} - {{ get_create_view_as_sql(target_relation, sql) }} + {{ build_sql }} {%- endcall %} {% endif %} diff --git a/tests/functional/adapter/mssql/test_materialize_change.py b/tests/functional/adapter/mssql/test_materialize_change.py index 9f31f05e3..6cabaaa01 100644 --- a/tests/functional/adapter/mssql/test_materialize_change.py +++ b/tests/functional/adapter/mssql/test_materialize_change.py @@ -110,6 +110,44 @@ def test_public_select_grant_survives_swap(self, project): assert ("PUBLIC", "SELECT") in [(row[0].upper(), row[1].upper()) for row in grants] +class TestViewMaterializationNoOp(BaseTableView): + """Test that rerunning an unchanged view avoids altering the view.""" + + @pytest.fixture(scope="class") + def models(self): + return {"mat_object.sql": view_mat, "schema.yml": schema} + + def test_unchanged_view_does_not_alter(self, project): + self.create_object(project, f"CREATE VIEW {project.test_schema}.mat_object AS {model_sql}") + + before_modify_date = project.run_sql( + f""" + select modify_date + from sys.objects o + join sys.schemas s on o.schema_id = s.schema_id + where upper(s.name) = upper('{project.test_schema}') + and upper(o.name) = upper('mat_object') + """, + fetch="one", + )[0] + + results = run_dbt(["run"]) + assert len(results) == 1 + + after_modify_date = project.run_sql( + f""" + select modify_date + from sys.objects o + join sys.schemas s on o.schema_id = s.schema_id + where upper(s.name) = upper('{project.test_schema}') + and upper(o.name) = upper('mat_object') + """, + fetch="one", + )[0] + + assert after_modify_date == before_modify_date + + class TestViewtoTable(BaseTableView): """Test if changing from a view object to a table object correctly replaces""" From 19d344b84b5a3199d00220afacc605a913cc8c7e Mon Sep 17 00:00:00 2001 From: Axell Padilla <68310020+axellpadilla@users.noreply.github.com> Date: Sat, 16 May 2026 22:32:48 +0000 Subject: [PATCH 3/4] fix(mssql): hardened SELECT/INSERT/UPDATE/DELETE grants survive table-to-view swap and no other grants pass on table swap --- .../macros/materializations/models/view/view.sql | 7 ++++++- .../adapter/mssql/test_materialize_change.py | 14 +++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/dbt/include/sqlserver/macros/materializations/models/view/view.sql b/dbt/include/sqlserver/macros/materializations/models/view/view.sql index 146c7315d..7bd03ed86 100644 --- a/dbt/include/sqlserver/macros/materializations/models/view/view.sql +++ b/dbt/include/sqlserver/macros/materializations/models/view/view.sql @@ -33,7 +33,12 @@ {% if existing_relation is not none and existing_relation.type != 'view' %} {% set current_grants_table = run_query(get_show_grant_sql(existing_relation)) %} {% set current_grants_dict = adapter.standardize_grants_dict(current_grants_table) %} - {% set preserved_grants = diff_of_two_dicts(current_grants_dict, grant_config) %} + {% set preserved_grants = {} %} + {% for privilege, grantees in diff_of_two_dicts(current_grants_dict, grant_config).items() %} + {% if privilege | lower in ['select', 'insert', 'update', 'delete'] %} + {% do preserved_grants.update({privilege: grantees}) %} + {% endif %} + {% endfor %} {% set build_sql = get_create_view_as_sql(intermediate_relation, sql) %} {% elif existing_relation is not none and existing_relation.type == 'view' %} {% set current_view_definition_table = run_query(get_view_definition_sql(existing_relation)) %} diff --git a/tests/functional/adapter/mssql/test_materialize_change.py b/tests/functional/adapter/mssql/test_materialize_change.py index 6cabaaa01..f2a083042 100644 --- a/tests/functional/adapter/mssql/test_materialize_change.py +++ b/tests/functional/adapter/mssql/test_materialize_change.py @@ -90,13 +90,16 @@ def test_public_select_grant_survives_swap(self, project): self.create_object( project, f"SELECT * INTO {project.test_schema}.mat_object FROM ({model_sql}) t" ) - project.run_sql(f"grant select on object::{project.test_schema}.mat_object to public") + project.run_sql( + f"""grant select, insert, update, delete + on object::{project.test_schema}.mat_object to public""" + ) run_dbt(["run"]) - grants = project.run_sql( + grant_count = project.run_sql( f""" - select pr.name as grantee, pe.permission_name + select count(*) from sys.database_permissions pe join sys.objects o on pe.major_id = o.object_id join sys.schemas s on o.schema_id = s.schema_id @@ -104,10 +107,11 @@ def test_public_select_grant_survives_swap(self, project): on pe.grantee_principal_id = pr.principal_id where s.name = '{project.test_schema}' and o.name = 'mat_object' + and pe.permission_name in ('SELECT', 'INSERT', 'UPDATE', 'DELETE') """, - fetch="all", + fetch="one", ) - assert ("PUBLIC", "SELECT") in [(row[0].upper(), row[1].upper()) for row in grants] + assert grant_count[0] == 4 class TestViewMaterializationNoOp(BaseTableView): From f07f23947e1f03608a6cd054abe98fcab74da018 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 22:33:53 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/functional/adapter/dbt/test_basic.py | 6 ++---- tests/functional/adapter/mssql/test_materialize_change.py | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/functional/adapter/dbt/test_basic.py b/tests/functional/adapter/dbt/test_basic.py index 087d6d0a1..56ead9ac6 100644 --- a/tests/functional/adapter/dbt/test_basic.py +++ b/tests/functional/adapter/dbt/test_basic.py @@ -24,16 +24,14 @@ def test_existing_view_materialization(self, project, models): # Write the initial model without the value column with open(model_path, "w") as f: - f.write( - """ + f.write(""" {{ config(materialized='view') }} select 1 as id {% if var('include_value_column', false) %} , 2 as value {% endif %} - """ - ) + """) # First run to create the view without the extra column results = run_dbt(["run", "-m", "view_model_exists"]) diff --git a/tests/functional/adapter/mssql/test_materialize_change.py b/tests/functional/adapter/mssql/test_materialize_change.py index 61c8f00b8..5137d8d1a 100644 --- a/tests/functional/adapter/mssql/test_materialize_change.py +++ b/tests/functional/adapter/mssql/test_materialize_change.py @@ -91,10 +91,8 @@ def test_public_select_grant_survives_swap(self, project): self.create_object( project, f"SELECT * INTO {project.test_schema}.mat_object FROM ({model_sql}) t" ) - project.run_sql( - f"""grant select, insert, update, delete - on object::{project.test_schema}.mat_object to public""" - ) + project.run_sql(f"""grant select, insert, update, delete + on object::{project.test_schema}.mat_object to public""") run_dbt(["run"])