[#4013] Fix multiple disabled models (#4018)

CHANGELOG.md

@@ -15,6 +15,7 @@
 - Add generic tests defined on sources to the manifest once, not twice ([#3347](https:/dbt-labs/dbt/issues/3347), [#3880](https:/dbt-labs/dbt/pull/3880))
 - Skip partial parsing if certain macros have changed ([#3810](https:/dbt-labs/dbt/issues/3810), [#3982](https:/dbt-labs/dbt/pull/3892))
 - Enable cataloging of unlogged Postgres tables ([3961](https:/dbt-labs/dbt/issues/3961), [#3993](https:/dbt-labs/dbt/pull/3993))
+- Fix multiple disabled nodes ([#4013](https:/dbt-labs/dbt/issues/4013), [#4018](https:/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)
@@ -1007,9 +1006,10 @@ def add_exposure(self, source_file: SchemaSourceFile, exposure: ParsedExposure):
  source_file.exposures.append(exposure.unique_id)
 
  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 +1099,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,9 @@ 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,17 @@ 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.
+ 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)