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

Fix unquoted database name #899

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix unquoted database name #899

wants to merge 3 commits into from

Conversation

skion
Copy link

@skion skion commented May 28, 2024

resolves #900

Description & motivation

This fixes get_relations_by_prefix() when the database name contains e.g. a hyphen on Postgres.

Checklist

  • This code is associated with an Issue which has been triaged and accepted for development.
  • I have verified that these changes work locally on the following warehouses (Note: it's okay if you do not have access to all warehouses, this helps us understand what has been covered)
    • BigQuery
    • Postgres
    • Redshift
    • Snowflake
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my models (and macros if applicable)

@dbeatty10 dbeatty10 added bug Something isn't working case_sensitivity Issues related to case-sensitivity behavior quoting Issues related to quoting behavior labels Jun 4, 2024
@skion skion marked this pull request as ready for review June 5, 2024 15:34
@dbeatty10
Copy link
Contributor

Thanks for putting this together @skion !

Taking an initial look, your proposal is simple and aligns with all the other macros. Ideally, we'd add a test case that includes a "mIxEdCaSe" database name so that we can validate that all the data warehouse we test work in this scenario (and also to prevent regressions in the future). But that might prove to be a bit tricky / impractical. So we might need to fall back on some manual testing for this. I'm doing to do some manual testing with Postgres, Snowflake, and Databricks both with and without the changes in your PR. I'll report back later with those results.

@skion
Copy link
Author

skion commented Jul 11, 2024

Any luck on this @dbeatty10 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working case_sensitivity Issues related to case-sensitivity behavior quoting Issues related to quoting behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quoting of database name appears to be broken in get_tables_by_pattern_sql
2 participants