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

Case sensitive generate source #168

Merged

Conversation

pnadolny13
Copy link
Contributor

resolves #112

This builds off of and should replace #136. I didnt know how to continue working off @jgillies 's fork so I made my own. In comparison to that existing PR this adds:

  • database and schema case sensitive options
  • an integration test to assert the changes

This is a:

  • documentation update
  • bug fix with no breaking changes
  • new functionality
  • a breaking change

All pull requests from community contributors should target the main branch (default).

Description & motivation

Checklist

  • This code is associated with an issue which has been triaged and accepted for development.
  • I have verified that these changes work locally
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my models (and macros if applicable)
  • I have added an entry to CHANGELOG.md

Notes:

  • I dont know of a way to test database casing in the current testing set up. Let me know if theres a way to do that! cc @gwenwindflower

@gwenwindflower
Copy link
Contributor

awesome work @pnadolny13! this looks great. :shipit:

qq: i recently rewrote the README for contribution, how did that land with you? not sure if you'd already contributed to packages before, so perhaps you didn't need it/read it, but if you did, would love any feedback on any areas that might have been confusing or could be expanded. thanks!

@pnadolny13
Copy link
Contributor Author

@gwenwindflower the docs were great!

I didnt have any issues getting the tests set up and running other than that the cimg/postgres:9.6 image in the docker compose was amd and my machine expected an arm image. I updated it to the latest cimg/postgres:15.6 and it all worked.

@gwenwindflower
Copy link
Contributor

ah awesome thanks @pnadolny13 -- i'll take a look at that i'm sure it's a quite old part of the coedebase that probably hasn't been updated in years, we definitely should probably update to a more recent postgres by default!

@gwenwindflower gwenwindflower merged commit 9347477 into dbt-labs:main Apr 1, 2024
4 checks passed
@jgillies
Copy link

jgillies commented Apr 1, 2024

@gwenwindflower and @pnadolny13 thanks for pushing this through! I hope I can get back to more dbt work soon and actually benefit from it!

@mickaelandrieu
Copy link

Hi @gwenwindflower , would you mind to release a new version with this improvement please ?

I was also looking for that feature 👍

Regards,

PS: thanks @jgillies !

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.

Names are automatically casted to lower-case, regardless of capitalisation in database
4 participants