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

[CT-527] Unexpected macro resolution order for collect_freshness macro #5123

Closed
jtcohen6 opened this issue Apr 21, 2022 · 3 comments
Closed
Labels
bug Something isn't working stale Issues that have gone stale

Comments

@jtcohen6
Copy link
Contributor

Split off from #4525

When running dbt source freshness, why does the definition of collect_freshness in a package silently take precedence over the global/default version in dbt-core, without the end user / root project "opting into" the override?

These codepaths are in execution/adapter land:

freshness = self.adapter.calculate_freshness(

# run the macro
table = self.execute_macro(FRESHNESS_MACRO_NAME, kwargs=kwargs, manifest=manifest)

But the real question here is around the macro resolver for collect_freshness, so I think Language team is best positioned to weigh in. For all other "built-in" macros (ones defined in the global project), only the root project can override them, or explicitly opt into a dispatch order (docs) that prefers macros from packages.

Reproduction case

Create a source for a nonexistent table in your project:

version: 2

sources:
  - name: fake_source
    loaded_at_field: current_timestamp
    freshness:
      warn_after: {period: day, count: 7}
    tables:
      - name: does_not_exist

Run dbt source freshness, fails because the table doesn't exist.

Create a local package with macros/collect_freshness.sql:

{% macro collect_freshness(source, loaded_at_field, filter) %}
  {% call statement('collect_freshness', fetch_result=True, auto_begin=False) -%}

  -- postgres syntax
  select
    current_timestamp as max_loaded_at,
    current_timestamp as snapshotted_at

  {% endcall %}
  {{ return(load_result('collect_freshness').table) }}
{% endmacro %}

Add that local package in packages.yml:

packages:
  - local: local_package

Run dbt source freshness, it passes—even though the end user never opted into the override.

Expected behavior

The user could override collect_freshness in their own project, to point to the package version instead:

{% macro collect_freshness(source, loaded_at_field, filter) %}
    {{ return(fivetran_utils.collect_freshness(source, loaded_at_field, filter) }}
{% endmacro %}

Or rearrange dispatch order to prefer candidate macros from the fivetran_utils package:

dispatch:
  - macro_namespace: collect_freshness
    search_order: ['fivetran_utils', 'dbt']
@jtcohen6 jtcohen6 added bug Something isn't working Team:Language labels Apr 21, 2022
@github-actions github-actions bot changed the title Unexpected macro resolution order for collect_freshness macro [CT-527] Unexpected macro resolution order for collect_freshness macro Apr 21, 2022
@nathaniel-may
Copy link
Contributor

estimation:

  • several moving parts
  • tests may be difficult
  • actual change likely to be small once the right places in the code are found

@github-actions
Copy link
Contributor

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Oct 19, 2022
@github-actions
Copy link
Contributor

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest; add a comment to notify the maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale Issues that have gone stale
Projects
None yet
Development

No branches or pull requests

2 participants