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

[CT-1522] [Bug] Easy : Can't Create lowercase model on SnowFlake (double quotes conflict) #330

Closed
2 tasks done
jugodfroy opened this issue Nov 17, 2022 · 5 comments · Fixed by #399
Closed
2 tasks done
Labels
bug Something isn't working good_first_issue Good for newcomers python_models

Comments

@jugodfroy
Copy link

jugodfroy commented Nov 17, 2022

Is this a new bug in dbt-snowflake?

  • I believe this is a new bug in dbt-snowflake
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

When trying to create a python model with lowercase name, the name of the model is under double quotes which lead to a conflit of double quote in the function save_as_table().

image

Expected Behavior

The expected behavior is the good creation of the model in lowercase in Snowflake

Steps To Reproduce

In dbt_project.yml specifiy the quoting settings :
image

Then try to run a python model. The error should be reproduced.

Relevant log output

![image](https://user-images.githubusercontent.com/79590825/202451775-681be1e0-8d03-4082-8c94-1229c88dde52.png)

Environment

- OS:Windows 10
- Python:
- dbt-core:1.3.0
- dbt-snowflake:

Ran with a Job

Additional Context

Idea To fix : in file : dbt-snowflake/dbt/include/snowflake/macros/materializations/table.sql

line 52 (double to simple quote) :
replace df.write.mode("overwrite").save_as_table("{{ target_relation }}", create_temp_table={{temporary}})
by df.write.mode("overwrite").save_as_table('{{ target_relation }}', create_temp_table={{temporary}})

@jugodfroy jugodfroy added bug Something isn't working triage labels Nov 17, 2022
@github-actions github-actions bot changed the title [Bug] Easy : Can't Create lowercase model on SnowFlake (double quotes conflict) [CT-1522] [Bug] Easy : Can't Create lowercase model on SnowFlake (double quotes conflict) Nov 17, 2022
@jtcohen6
Copy link
Contributor

jtcohen6 commented Nov 17, 2022

@jugodfroy Thanks for opening - I think you've got the right idea!

line 52 (double to simple quote)

This is definitely the most expedient. For consistency in output code, we might prefer to keep it as a double-quoted string, with escapes for any quotes inside. This looks awkward, but something like it might get the job done:

target_relation = {{ target_relation }}
df.write.mode("overwrite").save_as_table("{{ target_relation | replace('"', '\"' }}", create_temp_table={{temporary}})

@ChenyuLInx Any preference between those two?

@jtcohen6
Copy link
Contributor

jtcohen6 commented Dec 1, 2022

Let's go for the escape route... just in case the quoted relation name contains a single quote (gross)

@jugodfroy Is this something you'd be interested in contributing?

@jtcohen6 jtcohen6 added the good_first_issue Good for newcomers label Dec 1, 2022
@jugodfroy
Copy link
Author

I would love to !
Tell me how can I contribute @jtcohen6

@jtcohen6
Copy link
Contributor

jtcohen6 commented Dec 6, 2022

@jugodfroy Check out the contributing guide!

We'll just want an additional test case here, which has a Python model with a lowercase name + quoting config enabled.

@jugodfroy
Copy link
Author

Actually it will be quicker if someone else fix this one :)

jtcohen6 added a commit that referenced this issue Jan 16, 2023
jtcohen6 added a commit that referenced this issue Jan 20, 2023
* Respect quoting in dbt-py models #330

* Add changelog entry

* Switch dbt-core back to main

Co-authored-by: Chenyu Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good_first_issue Good for newcomers python_models
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants