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

Single Quotes for COLLATE statements (New PR) #358

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

Conversation

YYYasin19
Copy link

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    This solves the issue described in Single quotes for the COLLATE statement #186, unfortunately, there was no issue referenced there.

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding new credentials
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

The initial problem description can be viewed here.
TL;DR:
When creating tables with columns that have collation in sqlalchemy

table = sqlalchemy.Table("table1", Column("char_column", String(collation="en")))

the resulting SQL statements use double quotes

CREATE TABLE table1 (char_column VARCHAR COLLATE "en")

which is allowed in other database engines but does not work in snowflake.

This PR fixes that issue by overwriting the rendering in the SQLAlchemy statement Compiler for text columns and adding at least one new test.

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@YYYasin19
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@YYYasin19
Copy link
Author

My initial idea was to write a test that validates if the column is sorted according to the specified collation.
I think, however, that that is something snowflake itself should have to test. Therefore I am just validating that the information was passed correctly.
Previously, before this feature, it was not even possible to create tables with collation information so that is tested implicitly as well.

@YYYasin19 YYYasin19 marked this pull request as ready for review November 9, 2022 13:12
@YYYasin19
Copy link
Author

@sfc-gh-mkeller do you have some time or know someone from snowflake who could have a look at this?

@toobeyonewing
Copy link

Just checking to see if there is an update on this issue.

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.

2 participants