[#4013] Fix multiple disabled models

CHANGELOG.md

@@ -15,6 +15,7 @@
 - Add generic tests defined on sources to the manifest once, not twice ([#3347](https://github.com/dbt-labs/dbt/issues/3347), [#3880](https://github.com/dbt-labs/dbt/pull/3880))
 - Skip partial parsing if certain macros have changed ([#3810](https://github.com/dbt-labs/dbt/issues/3810), [#3982](https://github.com/dbt-labs/dbt/pull/3892))
 - Enable cataloging of unlogged Postgres tables ([3961](https://github.com/dbt-labs/dbt/issues/3961), [#3993](https://github.com/dbt-labs/dbt/pull/3993))
+- Fix multiple disabled nodes ([#4013](https://github.com/dbt-labs/dbt/issues/4013), [#4018](https://github.com/dbt-labs/dbt/pull/4018))
 
 ### Under the hood
 

core/dbt/contracts/graph/manifest.py

@@ -382,7 +382,7 @@ def search_name(self) -> str:
 
 
 @dataclass
-class NameSearcher(Generic[N]):
+class DisabledNameSearcher():
     name: str
     package: Optional[str]
     nodetypes: List[NodeType]
@@ -404,9 +404,10 @@ def _matches(self, model: N) -> bool:
 
     def search(self, haystack) -> Optional[N]:
         """Find an entry in the given iterable by name."""
-        for model in haystack.values():
-            if self._matches(model):
-                return model
+        for model_list in haystack.values():
+            for model in model_list:
+                if self._matches(model):
+                    return model
         return None
 
 
@@ -563,10 +564,8 @@ class Manifest(MacroMethods, DataClassMessagePackMixin, dbtClassMixin):
     metadata: ManifestMetadata = field(default_factory=ManifestMetadata)
     flat_graph: Dict[str, Any] = field(default_factory=dict)
     state_check: ManifestStateCheck = field(default_factory=ManifestStateCheck)
-    # Moved from the ParseResult object
     source_patches: MutableMapping[SourceKey, SourcePatch] = field(default_factory=dict)
-    # following is from ParseResult
-    disabled: MutableMapping[str, CompileResultNode] = field(default_factory=dict)
+    disabled: MutableMapping[str, List[CompileResultNode]] = field(default_factory=dict)
 
     _doc_lookup: Optional[DocLookup] = field(
         default=None, metadata={'serialize': lambda x: None, 'deserialize': lambda x: None}
@@ -653,7 +652,7 @@ def build_flat_graph(self):
     def find_disabled_by_name(
         self, name: str, package: Optional[str] = None
     ) -> Optional[ManifestNode]:
-        searcher: NameSearcher = NameSearcher(
+        searcher: DisabledNameSearcher = DisabledNameSearcher(
             name, package, NodeType.refable()
         )
         result = searcher.search(self.disabled)
@@ -663,7 +662,7 @@ def find_disabled_source_by_name(
         self, source_name: str, table_name: str, package: Optional[str] = None
     ) -> Optional[ParsedSourceDefinition]:
         search_name = f'{source_name}.{table_name}'
-        searcher: NameSearcher = NameSearcher(
+        searcher: DisabledNameSearcher = DisabledNameSearcher(
             search_name, package, [NodeType.Source]
         )
         result = searcher.search(self.disabled)
@@ -1008,8 +1007,10 @@ def add_exposure(self, source_file: SchemaSourceFile, exposure: ParsedExposure):
 
     def add_disabled_nofile(self, node: CompileResultNode):
         # a disabled node should only show up once
-        _check_duplicates(node, self.disabled)
-        self.disabled[node.unique_id] = node
+        if node.unique_id in self.disabled:
+            self.disabled[node.unique_id].append(node)
+        else:
+            self.disabled[node.unique_id] = [node]
 
     def add_disabled(self, source_file: AnySourceFile, node: CompileResultNode, test_from=None):
         self.add_disabled_nofile(node)
@@ -1099,8 +1100,8 @@ class WritableManifest(ArtifactMixin):
             'The selectors defined in selectors.yml'
         ))
     )
-    disabled: Optional[Mapping[UniqueID, CompileResultNode]] = field(metadata=dict(
-        description='A list of the disabled nodes in the target'
+    disabled: Optional[Mapping[UniqueID, List[CompileResultNode]]] = field(metadata=dict(
+        description='A mapping of the disabled nodes in the target'
     ))
     parent_map: Optional[NodeEdgeMap] = field(metadata=dict(
         description='A mapping from child nodes to their dependencies',

core/dbt/parser/manifest.py

@@ -6,6 +6,7 @@
 from typing import (
     Dict, Optional, Mapping, Callable, Any, List, Type, Union, Tuple
 )
+from itertools import chain
 import time
 
 import dbt.exceptions
@@ -893,7 +894,7 @@ def _warn_for_unused_resource_config_paths(
     manifest: Manifest, config: RuntimeConfig
 ) -> None:
     resource_fqns: Mapping[str, PathSet] = manifest.get_resource_fqns()
-    disabled_fqns: PathSet = frozenset(tuple(n.fqn) for n in manifest.disabled.values())
+    disabled_fqns: PathSet = frozenset(tuple(n.fqn) for n in list(chain.from_iterable(manifest.disabled.values())))
     config.warn_for_unused_resource_config_paths(resource_fqns, disabled_fqns)
 
 

core/dbt/parser/schemas.py

@@ -827,8 +827,20 @@ def parse_patch(
                 f'file {source_file.path.original_file_path}'
             )
         if unique_id is None:
-            # This will usually happen when a node is disabled
-            return
+            # Node might be disabled. Following call returns first matching node encountered.
+            found_node = self.manifest.find_disabled_by_name(patch.package_name, patch.name)
+            if found_node:
+                # There might be multiple disabled nodes for this model
+                for node in self.manifest.disabled[found_node.unique_id]:
+                    # We're saving the patch_path because we need to schedule
+                    # re-application of the patch in partial parsing.
+                    # We could also actually apply the patch to all of the
+                    # disabled nodes, but we haven't patched disabled nodes
+                    # in the past.
+                    node.patch_path = source_file.file_id
+            else:
+                # Should we issue a warning message here?
+                return
 
         # patches can't be overwritten
         node = self.manifest.nodes.get(unique_id)

test/integration/025_duplicate_model_test/models-5/subdir1/model_alt.sql

@@ -0,0 +1 @@
+select 100 as somevalue

test/integration/025_duplicate_model_test/models-5/subdir2/model_alt.sql

@@ -0,0 +1,4 @@
+{{
+  config(enabled = False)
+}}
+select 100 as altvalue

test/integration/025_duplicate_model_test/models-5/subdir3/model_alt.sql

@@ -0,0 +1,5 @@
+{{
+  config(enabled = False)
+}}
+
+select 100 as thirdvalue

test/integration/025_duplicate_model_test/test_duplicate_model.py

@@ -1,5 +1,5 @@
 from dbt.exceptions import CompilationException
-from test.integration.base import DBTIntegrationTest, use_profile
+from test.integration.base import DBTIntegrationTest, use_profile, get_manifest
 
 
 class TestDuplicateModelEnabled(DBTIntegrationTest):
@@ -150,3 +150,21 @@ def test_postgres_duplicate_test_model_paths(self):
         self.run_dbt(['compile'])
         self.run_dbt(['--partial-parse', 'compile'])
         self.run_dbt(['--partial-parse', 'compile'])
+
+
+class TestMultipleDisabledModels(DBTIntegrationTest):
+
+    @property
+    def schema(self):
+        return "duplicate_model_025"
+
+    @property
+    def models(self):
+        return "models-5"
+
+    @use_profile('postgres')
+    def test_postgres_multiple_disabled_models(self):
+        self.run_dbt(['run'])
+        manifest = get_manifest()
+        model_id = 'model.test.model_alt'
+        self.assertIn(model_id, manifest.nodes)