diff --git a/CHANGELOG.md b/CHANGELOG.md index cc4c25a1a42..7264adb1b8e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index 3681826fe3b..7c808065436 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/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', diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 19a8c7a56e8..0822d64e54e 100644 --- a/core/dbt/parser/manifest.py +++ b/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) diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py index 41ec510945e..41da4107849 100644 --- a/core/dbt/parser/schemas.py +++ b/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) diff --git a/test/integration/025_duplicate_model_test/models-5/subdir1/model_alt.sql b/test/integration/025_duplicate_model_test/models-5/subdir1/model_alt.sql new file mode 100644 index 00000000000..62ca1a3ee7c --- /dev/null +++ b/test/integration/025_duplicate_model_test/models-5/subdir1/model_alt.sql @@ -0,0 +1 @@ +select 100 as somevalue diff --git a/test/integration/025_duplicate_model_test/models-5/subdir2/model_alt.sql b/test/integration/025_duplicate_model_test/models-5/subdir2/model_alt.sql new file mode 100644 index 00000000000..3c4a32abd2c --- /dev/null +++ b/test/integration/025_duplicate_model_test/models-5/subdir2/model_alt.sql @@ -0,0 +1,4 @@ +{{ + config(enabled = False) +}} +select 100 as altvalue diff --git a/test/integration/025_duplicate_model_test/models-5/subdir3/model_alt.sql b/test/integration/025_duplicate_model_test/models-5/subdir3/model_alt.sql new file mode 100644 index 00000000000..b5c423eaf4a --- /dev/null +++ b/test/integration/025_duplicate_model_test/models-5/subdir3/model_alt.sql @@ -0,0 +1,5 @@ +{{ + config(enabled = False) +}} + +select 100 as thirdvalue diff --git a/test/integration/025_duplicate_model_test/test_duplicate_model.py b/test/integration/025_duplicate_model_test/test_duplicate_model.py index 63177085650..ca34186adf8 100644 --- a/test/integration/025_duplicate_model_test/test_duplicate_model.py +++ b/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)