-
Notifications
You must be signed in to change notification settings - Fork 105
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 generate_source
macro return case-sensitive names
#136
Conversation
@@ -44,7 +44,7 @@ | |||
{% endif %} | |||
|
|||
{% for table in tables %} | |||
{% do sources_yaml.append(' - name: ' ~ table | lower ) %} | |||
{% do sources_yaml.append(' - name: ' ~ (table if case_sensitive_tables else table | lower)) %} |
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.
Would it make sense to have the name still as lower case but add the uncased as identifier
?
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.
Ah, I never knew about the distinction between table
and identifier
in sources. Makes sense to use identifier
!
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.
Actually, I believe this is assigning the relation's identifier
to the table
variable, so no change would be necessary to pick up the logic you're suggesting. Does that make sense?
dbt-codegen/macros/generate_source.sql
Line 10 in af12570
{% set table_list= tables | map(attribute='identifier') %} |
Hey @jgillies -- thanks for your patience with this, we were a bit short staffed while I was on medical leave for several months. This looks awesome and definitely should be added, thank you! 🙏🏻 Couple things for consideration:
Lastly, as you opened this quite a while ago, if your capacity has changed and you don't have the time to get into the above, no worries at all, just let me know and I can see about pushing some more commits to this branch. Would prefer to help you get this across the finish line yourself though if you're in for it! Here to help with anything or any questions, just @ me. |
Also, I went ahead and resolved the conflicts with main that have cropped up in the months this was waiting, so one less hurdle to worry about. |
Hi @gwenwindflower! I'm not able to jump back into this at the moment, so if someone else can push it across the finish line I'm totally OK with that. Otherwise, I can spend some time on it within the next couple months. |
All good @jgillies -- thanks for letting us know, we've got it! Appreciate the work, patience, and thought to get it to here! ❤️ |
@jgillies @gwenwindflower glad to see this PR! I've just run into the same issue. Is there anything I can help with to get this over the finish line? I'm happy to contribute! 😄 |
hey @pnadolny13 👋🏻 ! that would be super welcome! take a look here for how package development works and if that all makes sense, then the things remaining are:
very happy to answer any questions! |
@gwenwindflower I created a new PR #168 that should have all the remaining items, let me know if you have any feedback! Thanks @jgillies for creating this PR! If its ok with you then I think we can close this and get #168 across the finish line 😄 . |
Closed because of #168 |
resolves #112
This is a:
All pull requests from community contributors should target the
main
branch (default).Description & motivation
For organizations with strict standards around the letter case of their tables and columns. The existing behavior of the
generate_source
macro is that it returns all table and column names in lowercase. This makes it less useful for the previously mentioned organizations.This adds the parameters
case_sensitive_tables
andcase_sensitive_cols
to thegenerate_source
macro. Both default tofalse
to preserve existing behavior.Checklist