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 relation type to get_relations_by_pattern and get_relations_by_prefix #323

Merged
merged 4 commits into from
Jan 11, 2021

Conversation

clrcrl
Copy link
Contributor

@clrcrl clrcrl commented Jan 10, 2021

This is a:

  • bug fix PR with no breaking changes — please ensure the base branch is master
  • new functionality — please ensure the base branch is the latest dev/ branch (🚨 I created this against master since it's such a tiny change)
  • a breaking change — please ensure the base branch is the latest dev/ branch

Description & motivation

I was trying to use this macro today like so:

{% macro cleanup_ci_relations(safe_mode=true) %}
{% set relations_to_drop = dbt_utils.get_relations_by_pattern(
    schema_pattern='my_schema',
    table_pattern='dbt\_%'
) %}

{% set sql_to_execute = [] %}

{% for relation in relations_to_drop %}
    {% set drop_command %}
    drop {{ relation.type }} {{ relation }} cascade;
    {% endset %}
    {% do log(drop_command, info=True) %}
    {% do sql_to_execute.append(drop_command) %}
{% endfor %}

{% endmacro %}

However this line didn't work:

    drop {{ relation.type }} {{ relation }} cascade;

Since relation.type compiled to None.

This fixes the issue, but I'm curious whether there's a smarter thing we should be doing here!

I also took the opportunity to update the docs to hint people towards using the more flexible get_relations_by_pattern (partially by listing it first)

Checklist

  • I have verified that these changes work locally on the following warehouses (Note: it's okay if you do not have access to all warehouses, this helps us understand what has been covered)
    • BigQuery
    • Postgres
    • Redshift
    • Snowflake
  • I have updated the README.md (if applicable)
  • n/a I have added tests & descriptions to my models (and macros if applicable)
  • I have added an entry to CHANGELOG.md

Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

I really like it!

This fixes the issue, but I'm curious whether there's a smarter thing we should be doing here!

These macros look more and more like dbt's built-in methods for building the relational cache at the start of an invocations. A cool way to rewrite these could be as regex-y cache lookups—if the dbs/schemas of interest overlap with the ones that dbt knows about—but that's a project for a later day.

Let's merge this in and cut v0.6.4 later today, yeah?

@clrcrl clrcrl force-pushed the add-relation-type branch from 94dd2f1 to bc1e653 Compare January 11, 2021 14:14
@clrcrl clrcrl changed the title Add relation type to get_relations_by_pattern Add relation type to get_relations_by_pattern and get_relations_by_prefix Jan 11, 2021
@clrcrl clrcrl merged commit 462e3bc into master Jan 11, 2021
@clrcrl clrcrl deleted the add-relation-type branch January 11, 2021 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants