Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check for node name uniqueness across refable resource types #737

Merged
merged 5 commits into from
Apr 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions dbt/compilation.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,22 @@ def get_all_projects(self):

return all_projects

def _check_resource_uniqueness(cls, flat_graph):
nodes = flat_graph['nodes']
names_resources = {}

for resource, node in nodes.items():
if node.get('resource_type') not in NodeType.refable():
continue

name = node['name']
existing_node = names_resources.get(name)
if existing_node is not None:
dbt.exceptions.raise_duplicate_resource_name(
existing_node, node)

names_resources[name] = node

def compile(self):
linker = Linker()

Expand All @@ -270,6 +286,8 @@ def compile(self):
flat_graph = dbt.loader.GraphLoader.load_all(
root_project, all_projects)

self._check_resource_uniqueness(flat_graph)

flat_graph = dbt.parser.process_refs(flat_graph,
root_project.get('name'))

Expand Down
14 changes: 14 additions & 0 deletions dbt/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,3 +291,17 @@ def raise_dep_not_found(node, node_description, required_pkg):
'Error while parsing {}.\nThe required package "{}" was not found. '
'Is the package installed?\nHint: You may need to run '
'`dbt deps`.'.format(node_description, required_pkg), node=node)


def raise_duplicate_resource_name(node_1, node_2):
duped_name = node_1['name']

raise_compiler_error(
'dbt found two resources with the name "{}". Since these resources '
'have the same name,\ndbt will be unable to find the correct resource '
'when ref("{}") is used. To fix this,\nchange the name of one of '
'these resources:\n- {} ({})\n- {} ({})'.format(
duped_name,
duped_name,
node_1['unique_id'], node_1['original_file_path'],
node_2['unique_id'], node_2['original_file_path']))
10 changes: 0 additions & 10 deletions dbt/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,6 @@ def load_all(cls, root_project, all_projects, macros=None):

to_return.update(project_loaded)

# Check for duplicate model names
names_models = {}
for model, attribs in to_return.items():
name = attribs['name']
existing_name = names_models.get(name)
if existing_name is not None:
raise dbt.exceptions.CompilationException(
'Found models with the same name: \n- %s\n- %s' % (
model, existing_name))
names_models[name] = model
return to_return

@classmethod
Expand Down
2 changes: 1 addition & 1 deletion dbt/node_runners.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def raise_on_first_error(self):

@classmethod
def is_refable(cls, node):
return node.get('resource_type') in [NodeType.Model, NodeType.Seed]
return node.get('resource_type') in NodeType.refable()

@classmethod
def is_ephemeral(cls, node):
Expand Down
7 changes: 7 additions & 0 deletions dbt/node_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ def executable(cls):
cls.Seed,
]

@classmethod
def refable(cls):
return [
cls.Model,
cls.Seed,
]


class RunHookType:
Start = 'on-run-start'
Expand Down
6 changes: 2 additions & 4 deletions dbt/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,8 @@ def parse_sql_nodes(nodes, root_project, projects, tags=None, macros=None):
# Check for duplicate model names
existing_node = to_return.get(node_path)
if existing_node is not None:
raise dbt.exceptions.CompilationException(
'Found models with the same name:\n- %s\n- %s' % (
existing_node.get('original_file_path'),
node.get('original_file_path')))
dbt.exceptions.raise_duplicate_resource_name(
existing_node, node_parsed)

to_return[node_path] = node_parsed

Expand Down
2 changes: 1 addition & 1 deletion dbt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def model_immediate_name(model, non_destructive):

def find_refable_by_name(flat_graph, target_name, target_package):
return find_by_name(flat_graph, target_name, target_package,
'nodes', [NodeType.Model, NodeType.Seed])
'nodes', NodeType.refable())


def find_macro_by_name(flat_graph, target_name, target_package):
Expand Down
14 changes: 10 additions & 4 deletions test/integration/025_duplicate_model_test/test_duplicate_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ def profile_config(self):

@attr(type="postgres")
def test_duplicate_model_enabled(self):
message = "Found models with the same name:.*"
with self.assertRaisesRegexp(CompilationException, message):
message = "dbt found two resources with the name"
try:
self.run_dbt(["run"])
self.assertTrue(False, "dbt did not throw for duplicate models")
except CompilationException as e:
self.assertTrue(message in str(e), "dbt did not throw the correct error message")


class TestDuplicateModelDisabled(DBTIntegrationTest):
Expand Down Expand Up @@ -114,9 +117,12 @@ def project_config(self):
@attr(type="postgres")
def test_duplicate_model_enabled_across_packages(self):
self.run_dbt(["deps"])
message = "Found models with the same name:.*"
with self.assertRaisesRegexp(CompilationException, message):
message = "dbt found two resources with the name"
try:
self.run_dbt(["run"])
self.assertTrue(False, "dbt did not throw for duplicate models")
except CompilationException as e:
self.assertTrue(message in str(e), "dbt did not throw the correct error message")


class TestDuplicateModelDisabledAcrossPackages(DBTIntegrationTest):
Expand Down