Slugify for snowflake

[fivetran-catfritz]

resolves #

This is a:

• documentation update
• bug fix with no breaking changes
• new functionality
• a breaking change

All pull requests from community contributors should target the main branch (default).

Description & motivation

Snowflake does not allow columns to begin with a number, so to preserve the number, the changes adds an underscore to the beginning of the string if the target is Snowflake.

Checklist

• This code is associated with an Issue which has been triaged and accepted for development.
• 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.type_* macros instead of explicit datatypes (e.g. dbt.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

Cc: @crowemi thanks so much for collaborating with us on this. I would love to have your feedback!

[joellabes]

Hey @fivetran-catfritz! Happy Hackathon and thanks for opening this.

It's not just Snowflake that has this problem, right? From some quick Googling, Postgres and BigQuery's columns must start with a letter or underscore, and Redshift can't start with a number or $ unless quoted (so basically the same rule).

I think this change can be applied across the board! Which also makes things cleaner for cross-db purposes - I don't love the idea of different column names based on the warehouse this runs on.

[joellabes]

Also, could you please change the target to the utils-v1 branch? I'd love to get this in as part of that (so next week-ish), but even if we don't you'll have a much easier merge if you're already up to date with all the changes that have happened on that branch.

[fivetran-catfritz]

Hi @joellabes - thanks for your feedback on this. I agree you're totally right about the same naming requirements for the other databases. I have made the updates and changed the target branch. Just let me know if you need any further changes!

[joellabes]

@fivetran-catfritz thanks for this! Sorry for the delay getting back to you. Looks good - only thing left would be to add a test that makes sure it works correctly.

The easiest thing to do would be to copy https://github.com/dbt-labs/dbt-utils/blob/main/integration_tests/tests/jinja_helpers/test_slugify.sql but make a new version that starts with a number

The slightly nicer option would be along the lines of

with comparisons as (
  select '{{ dbt_utils.slugify("!Hell0 world-hi") }}' as output, 'hell0_world_hi' as expected
  union 
  select '{{ dbt_utils.slugify("0Hell0 world-hi") }}' as output, '_0hell0_world_hi' as expected
)
select * 
from comparisons
where output != expected

but I haven't tested that code so don't know if it'll work perfectly!

[fivetran-catfritz]

@joellabes Thanks again! The test you suggested worked great--just had to change to a "union all" for bigquery. It should be good to go now! 😃

[joellabes]

🎉