Add flags.deprecate_package_materialization_builtin_override (#9956)

.changes/unreleased/Features-20240422-173703.yaml

@@ -0,0 +1,6 @@
+kind: Features
+body: Add require_explicit_package_overrides_for_builtin_materializations to dbt_project.yml flags, which can be used to opt-out of overriding built-in materializations from packages
+time: 2024-04-22T17:37:03.892268-04:00
+custom:
+  Author: michelleark
+  Issue: "10007"

core/dbt/contracts/graph/manifest.py

@@ -617,11 +617,25 @@ def __lt__(self, other: object) -> bool:
 
 
 class CandidateList(List[M]):
-    def last_candidate(self) -> Optional[MacroCandidate]:
+    def last_candidate(
+        self, valid_localities: Optional[List[Locality]] = None
+    ) -> Optional[MacroCandidate]:
+        """
+        Obtain the last (highest precedence) MacroCandidate from the CandidateList of any locality in valid_localities.
+        If valid_localities is not specified, return the last MacroCandidate of any locality.
+        """
         if not self:
             return None
         self.sort()
-        return self[-1]
+
+        if valid_localities is None:
+            return self[-1]
+
+        for candidate in reversed(self):
+            if candidate.locality in valid_localities:
+                return candidate
+
+        return None
 
     def last(self) -> Optional[Macro]:
         last_candidate = self.last_candidate()
@@ -930,11 +944,20 @@ def find_materialization_macro_by_name(
             and materialization_candidate.locality == Locality.Imported
             and core_candidates
         ):
-            deprecations.warn(
-                "package-materialization-override",
-                package_name=materialization_candidate.macro.package_name,
-                materialization_name=materialization_name,
-            )
+            # preserve legacy behaviour - allow materialization override
+            if (
+                get_flags().require_explicit_package_overrides_for_builtin_materializations
+                is False
+            ):
+                deprecations.warn(
+                    "package-materialization-override",
+                    package_name=materialization_candidate.macro.package_name,
+                    materialization_name=materialization_name,
+                )
+            else:
+                materialization_candidate = candidates.last_candidate(
+                    valid_localities=[Locality.Core, Locality.Root]
+                )
 
         return materialization_candidate.macro if materialization_candidate else None
 

core/dbt/contracts/project.py

@@ -295,6 +295,7 @@ class ProjectFlags(ExtensibleDbtClassMixin, Replaceable):
     partial_parse: Optional[bool] = None
     populate_cache: Optional[bool] = None
     printer_width: Optional[int] = None
+    require_explicit_package_overrides_for_builtin_materializations: bool = False
     send_anonymous_usage_stats: bool = DEFAULT_SEND_ANONYMOUS_USAGE_STATS
     static_parser: Optional[bool] = None
     use_colors: Optional[bool] = None
@@ -307,7 +308,9 @@ class ProjectFlags(ExtensibleDbtClassMixin, Replaceable):
 
     @property
     def project_only_flags(self) -> Dict[str, Any]:
-        return {}
+        return {
+            "require_explicit_package_overrides_for_builtin_materializations": self.require_explicit_package_overrides_for_builtin_materializations,
+        }
 
 
 @dataclass

tests/functional/materializations/test_custom_materialization.py

@@ -44,6 +44,56 @@ def test_adapter_dependency(self, project, override_view_adapter_dep, set_up_dep
         assert deprecations.active_deprecations == {"package-materialization-override"}
 
 
+class TestOverrideAdapterDependencyDeprecated:
+    # make sure that if there's a dependency with an adapter-specific
+    # materialization, we honor that materialization
+    @pytest.fixture(scope="class")
+    def packages(self):
+        return {"packages": [{"local": "override-view-adapter-dep"}]}
+
+    @pytest.fixture(scope="class")
+    def project_config_update(self):
+        return {
+            "flags": {
+                "require_explicit_package_overrides_for_builtin_materializations": True,
+            },
+        }
+
+    def test_adapter_dependency_deprecate_overrides(
+        self, project, override_view_adapter_dep, set_up_deprecations
+    ):
+        run_dbt(["deps"])
+        # this should pass because the override is buggy and unused
+        run_dbt(["run"])
+
+        # no deprecation warning -- flag used correctly
+        assert deprecations.active_deprecations == set()
+
+
+class TestOverrideAdapterDependencyLegacy:
+    # make sure that if there's a dependency with an adapter-specific
+    # materialization, we honor that materialization
+    @pytest.fixture(scope="class")
+    def packages(self):
+        return {"packages": [{"local": "override-view-adapter-dep"}]}
+
+    @pytest.fixture(scope="class")
+    def project_config_update(self):
+        return {
+            "flags": {
+                "require_explicit_package_overrides_for_builtin_materializations": False,
+            },
+        }
+
+    def test_adapter_dependency(self, project, override_view_adapter_dep, set_up_deprecations):
+        run_dbt(["deps"])
+        # this should error because the override is buggy
+        run_dbt(["run"], expect_pass=False)
+
+        # overriding a built-in materialization scoped to adapter from package is deprecated
+        assert deprecations.active_deprecations == {"package-materialization-override"}
+
+
 class TestOverrideDefaultDependency:
     @pytest.fixture(scope="class")
     def packages(self):
@@ -58,6 +108,52 @@ def test_default_dependency(self, project, override_view_default_dep, set_up_dep
         assert deprecations.active_deprecations == {"package-materialization-override"}
 
 
+class TestOverrideDefaultDependencyDeprecated:
+    @pytest.fixture(scope="class")
+    def packages(self):
+        return {"packages": [{"local": "override-view-default-dep"}]}
+
+    @pytest.fixture(scope="class")
+    def project_config_update(self):
+        return {
+            "flags": {
+                "require_explicit_package_overrides_for_builtin_materializations": True,
+            },
+        }
+
+    def test_default_dependency_deprecated(
+        self, project, override_view_default_dep, set_up_deprecations
+    ):
+        run_dbt(["deps"])
+        # this should pass because the override is buggy and unused
+        run_dbt(["run"])
+
+        # overriding a built-in materialization from package is deprecated
+        assert deprecations.active_deprecations == set()
+
+
+class TestOverrideDefaultDependencyLegacy:
+    @pytest.fixture(scope="class")
+    def packages(self):
+        return {"packages": [{"local": "override-view-default-dep"}]}
+
+    @pytest.fixture(scope="class")
+    def project_config_update(self):
+        return {
+            "flags": {
+                "require_explicit_package_overrides_for_builtin_materializations": False,
+            },
+        }
+
+    def test_default_dependency(self, project, override_view_default_dep, set_up_deprecations):
+        run_dbt(["deps"])
+        # this should error because the override is buggy
+        run_dbt(["run"], expect_pass=False)
+
+        # overriding a built-in materialization from package is deprecated
+        assert deprecations.active_deprecations == {"package-materialization-override"}
+
+
 root_view_override_macro = """
 {% materialization view, default %}
  {{ return(view_default_override.materialization_view_default()) }}

tests/unit/test_manifest.py

@@ -1239,7 +1239,7 @@ def test_find_generate_macros_by_name(macros, expectations):
 FindMaterializationSpec = namedtuple("FindMaterializationSpec", "macros,adapter_type,expected")
 
 
-def _materialization_parameter_sets():
+def _materialization_parameter_sets_legacy():
     # inject the plugins used for materialization parameter tests
     with mock.patch("dbt.adapters.base.plugin.project_name_from_path") as get_name:
         get_name.return_value = "foo"
@@ -1386,12 +1386,187 @@ def id_mat(arg):
         return "_".join(arg)
 
 
+@pytest.mark.parametrize(
+    "macros,adapter_type,expected",
+    _materialization_parameter_sets_legacy(),
+    ids=id_mat,
+)
+def test_find_materialization_by_name_legacy(macros, adapter_type, expected):
+    set_from_args(
+        Namespace(
+            SEND_ANONYMOUS_USAGE_STATS=False,
+            REQUIRE_EXPLICIT_PACKAGE_OVERRIDES_FOR_BUILTIN_MATERIALIZATIONS=False,
+        ),
+        None,
+    )
+
+    manifest = make_manifest(macros=macros)
+    result = manifest.find_materialization_macro_by_name(
+        project_name="root",
+        materialization_name="my_materialization",
+        adapter_type=adapter_type,
+    )
+    if expected is None:
+        assert result is expected
+    else:
+        expected_package, expected_adapter_type = expected
+        assert result.adapter_type == expected_adapter_type
+        assert result.package_name == expected_package
+
+
+def _materialization_parameter_sets():
+    # inject the plugins used for materialization parameter tests
+    with mock.patch("dbt.adapters.base.plugin.project_name_from_path") as get_name:
+        get_name.return_value = "foo"
+        FooPlugin = AdapterPlugin(
+            adapter=mock.MagicMock(),
+            credentials=mock.MagicMock(),
+            include_path="/path/to/root/plugin",
+        )
+        FooPlugin.adapter.type.return_value = "foo"
+        inject_plugin(FooPlugin)
+
+        BarPlugin = AdapterPlugin(
+            adapter=mock.MagicMock(),
+            credentials=mock.MagicMock(),
+            include_path="/path/to/root/plugin",
+            dependencies=["foo"],
+        )
+        BarPlugin.adapter.type.return_value = "bar"
+        inject_plugin(BarPlugin)
+
+        sets = [
+            FindMaterializationSpec(macros=[], adapter_type="foo", expected=None),
+        ]
+
+        # default only, each project
+        sets.extend(
+            FindMaterializationSpec(
+                macros=[MockMaterialization(project, adapter_type=None)],
+                adapter_type="foo",
+                expected=(project, "default"),
+            )
+            for project in ["root", "dep", "dbt"]
+        )
+
+        # other type only, each project
+        sets.extend(
+            FindMaterializationSpec(
+                macros=[MockMaterialization(project, adapter_type="bar")],
+                adapter_type="foo",
+                expected=None,
+            )
+            for project in ["root", "dep", "dbt"]
+        )
+
+        # matching type only, each project
+        sets.extend(
+            FindMaterializationSpec(
+                macros=[MockMaterialization(project, adapter_type="foo")],
+                adapter_type="foo",
+                expected=(project, "foo"),
+            )
+            for project in ["root", "dep", "dbt"]
+        )
+
+        sets.extend(
+            [
+                # matching type and default everywhere
+                FindMaterializationSpec(
+                    macros=[
+                        MockMaterialization(project, adapter_type=atype)
+                        for (project, atype) in product(["root", "dep", "dbt"], ["foo", None])
+                    ],
+                    adapter_type="foo",
+                    expected=("root", "foo"),
+                ),
+                # default in core, override is in dep, and root has unrelated override
+                # should find the dbt default because default materializations cannot be overwritten by packages.
+                FindMaterializationSpec(
+                    macros=[
+                        MockMaterialization("root", adapter_type="bar"),
+                        MockMaterialization("dep", adapter_type="foo"),
+                        MockMaterialization("dbt", adapter_type=None),
+                    ],
+                    adapter_type="foo",
+                    expected=("dbt", "default"),
+                ),
+                # default in core, unrelated override is in dep, and root has an override
+                # should find the root override.
+                FindMaterializationSpec(
+                    macros=[
+                        MockMaterialization("root", adapter_type="foo"),
+                        MockMaterialization("dep", adapter_type="bar"),
+                        MockMaterialization("dbt", adapter_type=None),
+                    ],
+                    adapter_type="foo",
+                    expected=("root", "foo"),
+                ),
+                # default in core, override is in dep, and root has an override too.
+                # should find the root override.
+                FindMaterializationSpec(
+                    macros=[
+                        MockMaterialization("root", adapter_type="foo"),
+                        MockMaterialization("dep", adapter_type="foo"),
+                        MockMaterialization("dbt", adapter_type=None),
+                    ],
+                    adapter_type="foo",
+                    expected=("root", "foo"),
+                ),
+                # core has default + adapter, dep has adapter, root has default
+                # should find the default adapter implementation, because it's the most specific
+                # and default materializations cannot be overwritten by packages
+                FindMaterializationSpec(
+                    macros=[
+                        MockMaterialization("root", adapter_type=None),
+                        MockMaterialization("dep", adapter_type="foo"),
+                        MockMaterialization("dbt", adapter_type=None),
+                        MockMaterialization("dbt", adapter_type="foo"),
+                    ],
+                    adapter_type="foo",
+                    expected=("dbt", "foo"),
+                ),
+            ]
+        )
+
+        # inherit from parent adapter
+        sets.extend(
+            FindMaterializationSpec(
+                macros=[MockMaterialization(project, adapter_type="foo")],
+                adapter_type="bar",
+                expected=(project, "foo"),
+            )
+            for project in ["root", "dep", "dbt"]
+        )
+        sets.extend(
+            FindMaterializationSpec(
+                macros=[
+                    MockMaterialization(project, adapter_type="foo"),
+                    MockMaterialization(project, adapter_type="bar"),
+                ],
+                adapter_type="bar",
+                expected=(project, "bar"),
+            )
+            for project in ["root", "dep", "dbt"]
+        )
+
+        return sets
+
+
 @pytest.mark.parametrize(
     "macros,adapter_type,expected",
     _materialization_parameter_sets(),
     ids=id_mat,
 )
 def test_find_materialization_by_name(macros, adapter_type, expected):
+    set_from_args(
+        Namespace(
+            SEND_ANONYMOUS_USAGE_STATS=False,
+            REQUIRE_EXPLICIT_PACKAGE_OVERRIDES_FOR_BUILTIN_MATERIALIZATIONS=True,
+        ),
+        None,
+    )
+
     manifest = make_manifest(macros=macros)
     result = manifest.find_materialization_macro_by_name(
         project_name="root",