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

Please do not use using #79

Merged
merged 2 commits into from
Feb 8, 2023
Merged

Please do not use using #79

merged 2 commits into from
Feb 8, 2023

Conversation

epapineau
Copy link
Contributor

Add guidance to the style guide for writing explicit join instructions with an on clause instead of implicit join instructions with a using clause.

The data team has experienced drift in our internal project based on personal preferences and would like to add this rule to the style guide to use for reference when we request changes on a using clause.

dbt_style_guide.md Outdated Show resolved Hide resolved
@patkearns10
Copy link
Contributor

I was gonna say, I thought this was always in the style guide! https://github.com/dbt-labs/corp/pull/58/files

Co-authored-by: Brandon Thomson <brandon.thomson@dbtlabs.com>
@epapineau
Copy link
Contributor Author

@patkearns10 I thought it was as well ??

Copy link
Contributor

@bthomson22 bthomson22 left a comment

Choose a reason for hiding this comment

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

It definitely was! I believe it was removed when it was determined that Snowflake resolved the bugs around using - that being said, I'm a-ok with being explicit about not using using, since it is harder to comprehend, and can theoretically cause errors in multi-join CTEs.

Copy link
Contributor

@bthomson22 bthomson22 left a comment

Choose a reason for hiding this comment

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

That was meant to be an approval

@epapineau epapineau merged commit a1da364 into main Feb 8, 2023
@epapineau epapineau deleted the epapineau/clarify_using_style branch February 9, 2023 01:38
@b-per
Copy link
Contributor

b-per commented Feb 20, 2023

We actually removed this explicitly when we reviewed the guide a few months back. I think that the initial comment was that there was performance reason with using which don't exist anymore.

In my mind, using is great because it forces you to name your columns correctly upstream.

@patkearns10
Copy link
Contributor

This is my gripe with using, if you do not fully qualify your tables
https://getdbt.slack.com/archives/CBSQTAPLG/p1626478828173900?thread_ts=1626439665.129500&cid=CBSQTAPLG

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.

None yet

4 participants