add macro to get columns

[patkearns10]

This is a:

• bug fix PR with no breaking changes — please ensure the base branch is main
• new functionality — please ensure the base branch is the latest dev/ branch
• a breaking change — please ensure the base branch is the latest dev/ branch

Description & motivation

Similar to dbt_utils.get_column_values, except this pulls column names, rather than the values within a column. Like dbt_utils.star, but as a list, to be iterated over.

This is basically dbt_utils.star, but removing some of the last steps.

Unsure what the purpose of the top couple lines of star macro are:

{% macro star(from, relation_alias=False, except=[], prefix='', suffix='') -%}
    {{ return(adapter.dispatch('star', 'dbt_utils')(from, relation_alias, except, prefix, suffix)) }}
{% endmacro %}

{% macro default__star(from, relation_alias=False, except=[], prefix='', suffix='') -%}
    {%- do dbt_utils._is_relation(from, 'star') -%}
    {%- do dbt_utils._is_ephemeral(from, 'star') -%}

So I left those out. But I assume maybe that is important?

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 followed guidelines to ensure that my changes will work on "non-core" adapters by:
  • dispatching any new macro(s) so non-core adapters can also use them (e.g. the star() source)
  • using the limit_zero() macro in place of the literal string: limit 0
  • using dbt_utils.type_* macros instead of explicit datatypes (e.g. dbt_utils.type_timestamp() instead of TIMESTAMP
• I have updated the README.md (if applicable)
• I have added tests & descriptions to my models (and macros if applicable)
• I have added an entry to CHANGELOG.md

[joellabes]

Hey @patkearns10, thanks for opening this!

Unsure what the purpose of the top couple lines of star macro are:

{% macro star(from, relation_alias=False, except=[], prefix='', suffix='') -%}
    {{ return(adapter.dispatch('star', 'dbt_utils')(from, relation_alias, except, prefix, suffix)) }}
{% endmacro %}

{% macro default__star(from, relation_alias=False, except=[], prefix='', suffix='') -%}
    {%- do dbt_utils._is_relation(from, 'star') -%}
    {%- do dbt_utils._is_ephemeral(from, 'star') -%}

The first chunk is to enable multiple dispatch: https://docs.getdbt.com/reference/dbt-jinja-functions/dispatch. In short, they allow other adapters to provide an override version of this macro, if they need to.

The is_relation and is_ephemeral blocks then check that the thing you passed in is a real ref/source object, and that it's not ephemeral (because CTEs can't be used with this, because they don't go into the info schema).

So you do need all parts of that 😬

I'd be very interested in whether the star macro could call this new macro directly, instead of having two copies of the exclude code etc. I haven’t looked at whether that's possible or not, but it feels reasonable - would love if you had a go as an extra for experts!

[patkearns10]

Okay I moved some things around!
the star macro now references the get_columns macro.

tested it works locally, by recreating the macros and removing the dispatch logic. How do I test the dispatch logic?

get_columns macro:

call get_columns:

call get_columns with except:

replace within local star macro:

call local star macro:

call local star macro with except:

[joellabes]

This is looking great :chefs_kiss: Just a couple of naming/doc tweaks for clarity. As for how to test the dispatched stuff, you'd want to add a dbt Singular test which validates the behaviour you expect.

Something along similar lines to how codegen is tested: https://github.com/dbt-labs/dbt-codegen/blob/main/integration_tests/tests/test_generate_source.sql - I don't know if there's a way to compare two jinja lists' contents, or whether you'd have to turn it into a comma-separated string and compare that to an expected value 🤷

[patkearns10]

Added tests, documentation, and a new (opt-in) argument for lowercase output:

star UPPER:

star lower:

get_filtered_columns UPPER:

get_filtered_columns lower:

integration tests:

[joellabes]

Big fan overall! My one complaint is the customisable lowercasing - since you can just pass it into the lower filter, I don't think adding two extra params adds enough value. I talk about that more on one of the specific comments - lmk what you think

[joellabes]

This should be {{ limit_zero() }} - not all databases support limit clauses (notably T-SQL)

[patkearns10]

Done!
@joellabes -- this is no longer required, as I switched to using the seed/model method. Should I remove this file then?

[joellabes]

I think that makes sense - the Core team are working on a wider testing suite that I hope will also cover packages - if that wasn't in the pipeline then I'd suggest we kept this around because "we’d need it one day" but I think we'll have a more long term solution soon.

[patkearns10]

Removed lowercase and swapped to different testing method.

Curious some other tests were failing 🤔 but I didn't touch those and don't really want to dig into that.

[joellabes]

OK I looked into this, and you're off the hook for the tests - they're checking target.name for snowflake instead of target.type so aren't lowercasing the properties 😬

https://github.com/dbt-labs/dbt-utils/blob/main/integration_tests/models/sql/test_unpivot.sql#L18-L21

And the test_get_column_values_use_default one I assume came from running dbt build or something - it has a post-hook which drops the seed after it's created which is a bit finnicky unless it's done as seed/run/test.

These won't break on the CI jobs, (idk why that isn't running 🤔) so I think we're good to merge! Thanks for getting this done 🥳

[joellabes]

😍 😍