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

Add project name to default search packages #4114

Merged
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
- Make finding disabled nodes more consistent ([#4069](https://github.com/dbt-labs/dbt-core/issues/4069), [#4073](https://github.com/dbt-labas/dbt-core/pull/4073))
- Remove connection from `render_with_context` during parsing, thereby removing misleading log message ([#3137](https://github.com/dbt-labs/dbt-core/issues/3137), [#4062](https://github.com/dbt-labas/dbt-core/pull/4062))
- Wait for postgres docker container to be ready in `setup_db.sh`. ([#3876](https://github.com/dbt-labs/dbt-core/issues/3876), [#3908](https://github.com/dbt-labs/dbt-core/pull/3908))
- Prefer macros defined in the project over the ones in a package by default ([#4106](https://github.com/dbt-labs/dbt-core/issues/4106), [#4114](https://github.com/dbt-labs/dbt-core/pull/4114))

Contributors:
- [@sungchun12](https://github.com/sungchun12) ([#4017](https://github.com/dbt-labs/dbt/pull/4017))
- [@matt-winkler](https://github.com/matt-winkler) ([#4017](https://github.com/dbt-labs/dbt/pull/4017))
- [@NiallRees](https://github.com/NiallRees) ([#3625](https://github.com/dbt-labs/dbt/pull/3625))
- [@rvacaru](https://github.com/rvacaru) ([#3908](https://github.com/dbt-labs/dbt/pull/3908))
- [@JCZuurmond](https://github.com/jczuurmond) ([#4114](https://github.com/dbt-labs/dbt-core/pull/4114))

## dbt-core 1.0.0b1 (October 11, 2021)

Expand Down
10 changes: 5 additions & 5 deletions core/dbt/context/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def dispatch(
elif isinstance(namespace, str):
search_packages = self._adapter.config.get_macro_search_order(namespace)
if not search_packages and namespace in self._adapter.config.dependencies:
search_packages = [namespace]
search_packages = [self.config.project_name, namespace]
else:
# Not a string and not None so must be a list
raise CompilationException(
Expand All @@ -162,10 +162,10 @@ def dispatch(
macro = self._namespace.get_from_package(
package_name, search_name
)
except CompilationException as exc:
raise CompilationException(
f'In dispatch: {exc.msg}',
) from exc
except CompilationException:
# Only raise CompilationException if macro is not found in
# any package
macro = None

if package_name is None:
attempts.append(search_name)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{% macro postgres__get_columns_in_relation(relation) %}
{{ return('a string') }}
{% endmacro %}
58 changes: 58 additions & 0 deletions test/integration/016_macro_tests/test_macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,64 @@ def test_postgres_overrides(self):
self.run_dbt()


class TestMacroOverridePackage(DBTIntegrationTest):
"""
The macro in `override-postgres-get-columns-macros` should override the
`get_columns_in_relation` macro by default.
"""

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

@property
def models(self):
return 'override-get-columns-models'

@property
def project_config(self):
return {
'config-version': 2,
'macro-paths': ['override-postgres-get-columns-macros'],
}

@use_profile('postgres')
def test_postgres_overrides(self):
# the first time, the model doesn't exist
self.run_dbt()
self.run_dbt()


class TestMacroNotOverridePackage(DBTIntegrationTest):
"""
The macro in `override-postgres-get-columns-macros` does NOT override the
`get_columns_in_relation` macro because we tell dispatch to not look at the
postgres macros.
"""

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

@property
def models(self):
return 'override-get-columns-models'

@property
def project_config(self):
return {
'config-version': 2,
'macro-paths': ['override-postgres-get-columns-macros'],
'dispatch': [{'macro_namespace': 'dbt', 'search_order': ['dbt']}],
}

@use_profile('postgres')
def test_postgres_overrides(self):
# the first time, the model doesn't exist
self.run_dbt(expect_pass=False)
self.run_dbt(expect_pass=False)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the second time be expect pass True ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a check in the model that raises an explicit error if the macro is overridden, so I believe it fails both times:

{% set result = adapter.get_columns_in_relation(this) %}
{% if execute and result != 'a string' %}
{% do exceptions.raise_compiler_error('overriding get_columns_in_relation failed') %}
{% endif %}
select 1 as id

Copy link
Contributor Author

@JCZuurmond JCZuurmond Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained in the class docstring, the run in this test should fail because the override does not happen. We need the override to overrule the raise compile error that @jtcohen6 showed.

    The macro in `override-postgres-get-columns-macros` does NOT override the
    `get_columns_in_relation` macro because we tell dispatch to not look at the
    postgres macros.



class TestDispatchMacroOverrideBuiltin(TestMacroOverrideBuiltin):
# test the same functionality as above, but this time,
# dbt.get_columns_in_relation will dispatch to a default__ macro
Expand Down