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

Generated test CTE names should be lowercase per style guide #3027

Closed
NiallRees opened this issue Jan 24, 2021 · 1 comment · Fixed by #3028
Closed

Generated test CTE names should be lowercase per style guide #3027

NiallRees opened this issue Jan 24, 2021 · 1 comment · Fixed by #3028
Labels
enhancement New feature or request

Comments

@NiallRees
Copy link
Contributor

Describe the feature

Firstly, this feels like the most nit-picky thing ever. The motivation comes from development of SQLFluff with the dbt templater. We have to lint pure SQL currently as opposed to jinja-SQL for parsing reasons.

In the case of dbt tests where we're linting the compiled, wrapped query, I've noticed that the CTE name being used has inconsistent capitalisation, and per the dbt coding conventions should be lowercase. At the minute it makes SQLFluff fail.

Compiled dbt test:


with dbt__CTE__INTERNAL_test as (

select * from schema.table
where x != 0
)select count(*) from dbt__CTE__INTERNAL_test

Describe alternatives you've considered

We are now able to map from jinja-sql to post-templated SQL to match the error lines correctly, but not yet able to exclude linting errors which only occur in the compiled SQL (like this one). We'll do that at some point, which will make this change unnecessary from a SQLFluff POV.

Who will this benefit?

Anyone who wants to use SQLFluff with the dbt templater or who likes looking at compiled dbt tests and feels extraordinarily passionate about the dbt coding conventions.

Are you interested in contributing this feature?

Sure

@jtcohen6
Copy link
Contributor

  1. I'm in favor of this change from a code-style perspective
  2. We've already done the legwork to avoid breaking changes:
  3. It's likely that in v0.20.0 we'll be making subtle changes to data test compilation, so that all SQL is wrapped by Jinja macros, rather than generated by python string logic. In the meantime, thanks for opening Make generated CTE test names lowercase to match style guide #3028, I'll check it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants