From fd7c95d1d2ca989c43005514dd2e47b71f2494b1 Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Wed, 13 Oct 2021 17:03:41 -0400 Subject: [PATCH] [#4012] Performance: Use child_map to find tests for nodes in resolve_graph (#4022) --- CHANGELOG.md | 1 + core/dbt/compilation.py | 15 ++++--- .../models-interdependent/model_a.sql | 1 + .../models-interdependent/model_c.sql | 1 + .../models-interdependent/schema.yml | 41 +++++++++++++++++++ .../069_build_test/test-files/model_b.sql | 1 + .../test-files/model_b_null.sql | 1 + test/integration/069_build_test/test_build.py | 41 ++++++++++++++++++- 8 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 test/integration/069_build_test/models-interdependent/model_a.sql create mode 100644 test/integration/069_build_test/models-interdependent/model_c.sql create mode 100644 test/integration/069_build_test/models-interdependent/schema.yml create mode 100644 test/integration/069_build_test/test-files/model_b.sql create mode 100644 test/integration/069_build_test/test-files/model_b_null.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index bfa5948e5f6..e820bb80143 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - Fix multiple disabled nodes ([#4013](https://github.com/dbt-labs/dbt/issues/4013), [#4018](https://github.com/dbt-labs/dbt/pull/4018)) - Fix multiple partial parsing errors ([#3996](https://github.com/dbt-labs/dbt/issues/3006), [#4020](https://github.com/dbt-labs/dbt/pull/4018)) - Return an error instead of a warning when runing with `--warn-error` and no models are selected ([#4006](https://github.com/dbt-labs/dbt/issues/4006), [#4019](https://github.com/dbt-labs/dbt/pull/4019)) +- Performance: Use child_map to find tests for nodes in resolve_graph ([#4012](https://github.com/dbt-labs/dbt/issues/4012), [#4022](https://github.com/dbt-labs/dbt/pull/4022)) ### Under the hood diff --git a/core/dbt/compilation.py b/core/dbt/compilation.py index 2e87675f21c..2618d542558 100644 --- a/core/dbt/compilation.py +++ b/core/dbt/compilation.py @@ -111,12 +111,13 @@ def _get_tests_for_node(manifest: Manifest, unique_id: UniqueID) -> List[UniqueI """ Get a list of tests that depend on the node with the provided unique id """ - return [ - node.unique_id - for _, node in manifest.nodes.items() - if node.resource_type == NodeType.Test and - unique_id in node.depends_on_nodes - ] + tests = [] + if unique_id in manifest.child_map: + for child_unique_id in manifest.child_map[unique_id]: + if child_unique_id.startswith('test.'): + tests.append(child_unique_id) + + return tests class Linker: @@ -430,6 +431,8 @@ def link_graph(self, linker: Linker, manifest: Manifest): if cycle: raise RuntimeError("Found a cycle: {}".format(cycle)) + manifest.build_parent_and_child_maps() + self.resolve_graph(linker, manifest) def resolve_graph(self, linker: Linker, manifest: Manifest) -> None: diff --git a/test/integration/069_build_test/models-interdependent/model_a.sql b/test/integration/069_build_test/models-interdependent/model_a.sql new file mode 100644 index 00000000000..43258a71464 --- /dev/null +++ b/test/integration/069_build_test/models-interdependent/model_a.sql @@ -0,0 +1 @@ +select 1 as id diff --git a/test/integration/069_build_test/models-interdependent/model_c.sql b/test/integration/069_build_test/models-interdependent/model_c.sql new file mode 100644 index 00000000000..6b5ce07801a --- /dev/null +++ b/test/integration/069_build_test/models-interdependent/model_c.sql @@ -0,0 +1 @@ +select * from {{ ref('model_b') }} diff --git a/test/integration/069_build_test/models-interdependent/schema.yml b/test/integration/069_build_test/models-interdependent/schema.yml new file mode 100644 index 00000000000..1d3fe4a9bfa --- /dev/null +++ b/test/integration/069_build_test/models-interdependent/schema.yml @@ -0,0 +1,41 @@ +version: 2 + +models: + - name: model_a + columns: + - name: id + tests: + - unique + - not_null + - relationships: + to: ref('model_b') + field: id + - relationships: + to: ref('model_c') + field: id + + - name: model_b + columns: + - name: id + tests: + - unique + - not_null + - relationships: + to: ref('model_a') + field: id + - relationships: + to: ref('model_c') + field: id + + - name: model_c + columns: + - name: id + tests: + - unique + - not_null + - relationships: + to: ref('model_a') + field: id + - relationships: + to: ref('model_b') + field: id diff --git a/test/integration/069_build_test/test-files/model_b.sql b/test/integration/069_build_test/test-files/model_b.sql new file mode 100644 index 00000000000..24cb03c7e01 --- /dev/null +++ b/test/integration/069_build_test/test-files/model_b.sql @@ -0,0 +1 @@ +select * from {{ ref('model_a') }} diff --git a/test/integration/069_build_test/test-files/model_b_null.sql b/test/integration/069_build_test/test-files/model_b_null.sql new file mode 100644 index 00000000000..4e5224ddf72 --- /dev/null +++ b/test/integration/069_build_test/test-files/model_b_null.sql @@ -0,0 +1 @@ +select null from {{ ref('model_a') }} diff --git a/test/integration/069_build_test/test_build.py b/test/integration/069_build_test/test_build.py index bae325bd73f..ef40db1edf1 100644 --- a/test/integration/069_build_test/test_build.py +++ b/test/integration/069_build_test/test_build.py @@ -1,5 +1,7 @@ -from test.integration.base import DBTIntegrationTest, use_profile +from test.integration.base import DBTIntegrationTest, use_profile, normalize import yaml +import shutil +import os class TestBuildBase(DBTIntegrationTest): @@ -79,3 +81,40 @@ def test__postgres_circular_relationship_test_success(self): actual = [r.status for r in results] expected = ['success']*7 + ['pass']*2 self.assertEqual(sorted(actual), sorted(expected)) + +class TestInterdependentModels(TestBuildBase): + + @property + def project_config(self): + return { + "config-version": 2, + "snapshot-paths": ["snapshots-none"], + "seeds": { + "quote_columns": False, + }, + } + + @property + def models(self): + return "models-interdependent" + + def tearDown(self): + if os.path.exists(normalize('models-interdependent/model_b.sql')): + os.remove(normalize('models-interdependent/model_b.sql')) + + + @use_profile("postgres") + def test__postgres_interdependent_models(self): + # check that basic build works + shutil.copyfile('test-files/model_b.sql', 'models-interdependent/model_b.sql') + results = self.build() + self.assertEqual(len(results), 16) + + # return null from model_b + shutil.copyfile('test-files/model_b_null.sql', 'models-interdependent/model_b.sql') + results = self.build(expect_pass=False) + self.assertEqual(len(results), 16) + actual = [str(r.status) for r in results] + expected = ['error']*4 + ['skipped']*7 + ['pass']*2 + ['success']*3 + self.assertEqual(sorted(actual), sorted(expected)) +