Skip to content

Commit

Permalink
fix the ambiguous alias exception to respect databases
Browse files Browse the repository at this point in the history
The alias name check is now tied to the behavior of adapter.Relation.create_from(...)
Plugins that override the `include` of their relations will use whatever they render to for the check
the actual exception-raising code gets the name that was compared instead of generating its own
Finally, I added a reasonable fallback behavior since this method was exposed to the context
  • Loading branch information
Jacob Beck committed May 1, 2020
1 parent 68babfb commit 20c5c4c
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
- Fix "Object of type Decimal is not JSON serializable" error when BigQuery queries returned numeric types in nested data structures ([#2336](https://github.com/fishtown-analytics/dbt/issues/2336), [#2348](https://github.com/fishtown-analytics/dbt/pull/2348))
- No longer query the information_schema.schemata view on bigquery ([#2320](https://github.com/fishtown-analytics/dbt/issues/2320), [#2382](https://github.com/fishtown-analytics/dbt/pull/2382))
- Add support for `sql_header` config in incremental models ([#2136](https://github.com/fishtown-analytics/dbt/issues/2136), [#2200](https://github.com/fishtown-analytics/dbt/pull/2200))
- The ambiguous alias check now examines the node's database value as well as the schema/identifier ([#2326](https://github.com/fishtown-analytics/dbt/issues/2326), [#2387](https://github.com/fishtown-analytics/dbt/pull/2387))

### Under the hood
- Added more tests for source inheritance ([#2264](https://github.com/fishtown-analytics/dbt/issues/2264), [#2291](https://github.com/fishtown-analytics/dbt/pull/2291))
Expand Down
9 changes: 5 additions & 4 deletions core/dbt/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -741,14 +741,15 @@ def raise_duplicate_resource_name(node_1, node_2):
node_2.unique_id, node_2.original_file_path))


def raise_ambiguous_alias(node_1, node_2):
duped_name = "{}.{}".format(node_1.schema, node_1.alias)
def raise_ambiguous_alias(node_1, node_2, duped_name=None):
if duped_name is None:
duped_name = f"{node_1.database}.{node_1.schema}.{node_1.alias}"

raise_compiler_error(
'dbt found two resources with the database representation "{}".\ndbt '
'cannot create two resources with identical database representations. '
'To fix this,\nchange the "schema" or "alias" configuration of one of '
'these resources:\n- {} ({})\n- {} ({})'.format(
'To fix this,\nchange the configuration of one of these resources:'
'\n- {} ({})\n- {} ({})'.format(
duped_name,
node_1.unique_id, node_1.original_file_path,
node_2.unique_id, node_2.original_file_path))
Expand Down
19 changes: 13 additions & 6 deletions core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import dbt.flags

from dbt import deprecations
from dbt.adapters.factory import get_relation_class_by_name
from dbt.helper_types import PathSet
from dbt.include.global_project import PACKAGES
from dbt.logger import GLOBAL_LOGGER as logger, DbtProcessState
Expand Down Expand Up @@ -367,7 +368,10 @@ def load_internal(cls, root_config: RuntimeConfig) -> Manifest:
return loader.load_only_macros()


def _check_resource_uniqueness(manifest: Manifest) -> None:
def _check_resource_uniqueness(
manifest: Manifest,
config: RuntimeConfig,
) -> None:
names_resources: Dict[str, NonSourceNode] = {}
alias_resources: Dict[str, NonSourceNode] = {}

Expand All @@ -378,22 +382,25 @@ def _check_resource_uniqueness(manifest: Manifest) -> None:
assert not isinstance(node, ParsedSourceDefinition)

name = node.name
alias = "{}.{}".format(node.schema, node.alias)
# the full node name is really defined by the adapter's relation
relation_cls = get_relation_class_by_name(config.credentials.type)
relation = relation_cls.create_from(config=config, node=node)
full_node_name = str(relation)

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

existing_alias = alias_resources.get(alias)
existing_alias = alias_resources.get(full_node_name)
if existing_alias is not None:
dbt.exceptions.raise_ambiguous_alias(
existing_alias, node
existing_alias, node, full_node_name
)

names_resources[name] = node
alias_resources[alias] = node
alias_resources[full_node_name] = node


def _warn_for_unused_resource_config_paths(
Expand All @@ -405,7 +412,7 @@ def _warn_for_unused_resource_config_paths(


def _check_manifest(manifest: Manifest, config: RuntimeConfig) -> None:
_check_resource_uniqueness(manifest)
_check_resource_uniqueness(manifest, config)
_warn_for_unused_resource_config_paths(manifest, config)


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
these should succeed, as both models have the same alias,
but they are configured to be built in _different_ schemas
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
select {{ string_literal(this.name) }} as tablename
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
select {{ string_literal(this.name) }} as tablename
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
version: 2
models:
- name: model_a
tests:
- expect_value:
field: tablename
value: duped_alias
- name: model_b
tests:
- expect_value:
field: tablename
value: duped_alias
36 changes: 36 additions & 0 deletions test/integration/026_aliases_test/test_aliases.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,39 @@ def test__postgres_same_alias_succeeds_in_different_schemas(self):

# Make extra sure the tests ran
self.assertTrue(len(res) > 0)


class TestSameAliasDifferentDatabases(DBTIntegrationTest):
setup_alternate_db = True

@property
def schema(self):
return "aliases_026"

@property
def models(self):
return "models-dupe-custom-database"

@property
def project_config(self):
return {
'config-version': 2,
"macro-paths": ['macros'],
'models': {
'test': {
'alias': 'duped_alias',
'model_b': {
'database': self.alternative_database,
},
},
}
}

@use_profile('bigquery')
def test__bigquery_same_alias_succeeds_in_different_schemas(self):
results = self.run_dbt(['run'])
self.assertEqual(len(results), 2)
res = self.run_dbt(['test'])

# Make extra sure the tests ran
self.assertTrue(len(res) > 0)

0 comments on commit 20c5c4c

Please sign in to comment.