Quote Redshift identifiers that would lose their casing - #16282
dylanpulver wants to merge 2 commits into
Conversation
dbt-adapter has two `need_quotes`, in need_quotes.rs and format_ident.rs, with the argument order swapped. The first quotes for Snowflake (folds to uppercase) and for Redshift (folds to lowercase); the second has only the Snowflake clause, so format_ident emitted "MyTable" unquoted on Redshift and the server folded it to "mytable". Both are reachable from TypeOps, so need_quotes_for_ident and format_ident gave opposite answers for the same identifier. Adds the missing clause plus a test asserting the two agree, so they cannot drift apart again without a failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo
|
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @dylanpulver |
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Generated with changie new per AGENTS.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo
|
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @dylanpulver |
dbt-adapterhas two public functions namedneed_quotes, in different modules, with the argument order swapped. They disagree about Redshift.need_quotes.rs:format_ident.rshas the Snowflake clause and no Redshift one, soformat_ident("MyTable", AdapterType::Redshift)returnsMyTableunquoted. Redshift folds unquoted identifiers to lowercase, so the emitted SQL refers tomytableand the casing the Snowflake rule exists to preserve is lost for Redshift.Both are reachable through the same trait, which is what makes them contradict rather than merely differ.
TypeOpsinsql_types.rsexposes:On Redshift,
need_quotes_for_ident("MyTable")istruewhileformat_ident("MyTable")returns it unquoted.This adds the missing Redshift clause to
format_ident.rs, mirroring the wording of the Snowflake one, and two tests: one pinning the case-folding behaviour for both backends, and one asserting the two functions agree, so the pair cannot drift apart again silently.I kept the change to the missing clause rather than collapsing the duplication, per the "keep changes minimal and focused" rule in
AGENTS.md. If you would rather have one function and a re-export, I am happy to send that instead.cargo test -p dbt-adapter --lib format_ident: 12 passed. Reverting the clause fails the new agreement test withformat_ident and need_quotes disagree for "MyTable" on Redshift, so it is not passing vacuously.cargo fmtclean.No changelog entry:
changie newwants an issue number and I did not open one, since this looked small enough to bring straight as a PR. Say the word and I will add one.This PR was written with AI assistance (Claude Code).