-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Make generated CTE test names lowercase to match style guide #3028
Make generated CTE test names lowercase to match style guide #3028
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @NiallRees! Just a comment about the changelog, then this is good to go.
@@ -298,7 +298,7 @@ def filter_null_values(input: Dict[K_T, Optional[V_T]]) -> Dict[K_T, V_T]: | |||
|
|||
|
|||
def add_ephemeral_model_prefix(s: str) -> str: | |||
return '__dbt__CTE__{}'.format(s) | |||
return '__dbt__cte__{}'.format(s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For clarity, this method is no longer used, since #2712 swapped it out in favor of adapter.add_ephemeral_prefix
.
@@ -246,7 +246,7 @@ def _inject_ctes_into_sql(self, sql: str, ctes: List[InjectedCTE]) -> str: | |||
return str(parsed) | |||
|
|||
def _get_dbt_test_name(self) -> str: | |||
return 'dbt__CTE__INTERNAL_test' | |||
return 'dbt__cte__internal_test' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably not be hard-coded within the compiler, and should instead do something like adapter.add_ephemeral_prefix('internal_test')
, but that's a problem for another day. We're planning to clean up some inconsistencies around data tests for the v0.20.0 release, anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be nice to see 🙂
Co-authored-by: Jeremy Cohen <jtcohen6@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Niall!
Resolves #3027
Description
Auto-generated dbt test CTE's have lowercase names to comply with dbt coding conventions. Makes SQLFluff happy which has to lint post-templated SQL. See #3027 for more
Checklist
CHANGELOG.md
and added information about my change to the "dbt next" section.