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

split quoting behaviour for Snowflake between column identifiers and other identifiers #194

Closed

Conversation

dhwdeca
Copy link

@dhwdeca dhwdeca commented Dec 22, 2023

Closes #141

Default dbt-unit-testing behaviour is to uppercase every identifier on Snowflake.
For dbt configuration this is typically the behaviour we want, but for column identifiers this constraint is too strict.

I suggest keeping the case for column identifiers on Snowflake unchanged, as otherwise the include-missing-columns functionality doesn't work when a database query returns lowercase columns.

This does require a little more diligence in naming the columns in the unit tests themself, but I believe that to be an acceptable requirement.

@maplechori
Copy link

What about identifiers with quotes that are also reserved words such as "CASE" in Snowflake?

@dhwdeca
Copy link
Author

dhwdeca commented Jan 19, 2024

What about identifiers with quotes that are also reserved words such as "CASE" in Snowflake?

Those will remain working, as every identifier still gets quoted. The only change is that column identifiers get quoted in the case they are provided (either inside the mock statement, or returned by the database), rather than getting uppercased by default.

@cdiniz cdiniz added the safe to test Label used to identify prs safe to test on the integration environments label Apr 16, 2024
@github-actions github-actions bot removed the safe to test Label used to identify prs safe to test on the integration environments label Apr 16, 2024
@cdiniz cdiniz added the safe to test Label used to identify prs safe to test on the integration environments label Apr 18, 2024
@github-actions github-actions bot removed the safe to test Label used to identify prs safe to test on the integration environments label Apr 18, 2024
Copy link
Collaborator

@psousa50 psousa50 left a comment

Choose a reason for hiding this comment

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

Good call!
Unfortunately, the code evolved a bit and we can't merge this now. I'll make the changes and mention your contribution in our next release.
Thank you!

@psousa50
Copy link
Collaborator

This has been included in release 0.4.2.

Once again, thank you very much for your help @dhwdeca!

@psousa50 psousa50 closed this Apr 19, 2024
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.

Snowflake SQL compilation error with quoted column names
5 participants