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

Feat/snowflake destination #414

Merged
merged 50 commits into from
Jun 20, 2023
Merged

Feat/snowflake destination #414

merged 50 commits into from
Jun 20, 2023

Conversation

steinitzu
Copy link
Collaborator

@steinitzu steinitzu commented Jun 15, 2023

closes #22

@netlify
Copy link

netlify bot commented Jun 15, 2023

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit 12bb222
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/6491aa69ff059d00072dcaa3

dlt/destinations/snowflake/__init__.py Outdated Show resolved Hide resolved
dlt/destinations/snowflake/snowflake.py Outdated Show resolved Hide resolved
"bool": "BOOLEAN",
"date": "DATE",
"timestamp": "TIMESTAMP_TZ",
"bigint": f"NUMBER({BIGINT_PRECISION}, 0)", # Snowflake has no integer types
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing 🤷

dlt/pipeline/pipeline.py Outdated Show resolved Hide resolved
def escape_snowflake_identifier(v: str) -> str:
# Snowcase uppercase all identifiers unless quoted. Match this here so queries on information schema work without issue
# See also https://docs.snowflake.com/en/sql-reference/identifiers-syntax#double-quoted-identifiers
return v.upper()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we asked our snowflake buddies what they use typically. let's see what they say

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to take into account escaping sql keywords also, e.g. : CREATE TABLE xxx (FROM VARCHAR)
Thinking now we should both quote and upcase.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems that people mostly ignore it: use the original casing without quotes. this of course translates to upper case internally. that for example makes dbt packages to look identical for different systems. I'm still waiting for more input. sql keywords? aren't they case insensitive as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean with keywords if you have a column named from or update or something. Those need to be quoted.

So I think if we uppercase and quote in the escape function: some_column -> "SOME_COLUMN" makes things work pretty seamlessly, least resistance to how snowflake works.

Then queries with unquoted cols like SELECT some_column FROM ... work since snowflake translates it to uppercase implicitly.

caps.escape_identifier = escape_snowflake_identifier
caps.max_identifier_length = 255
caps.max_column_identifier_length = 255
caps.max_query_length = 32 * 1024 * 1024
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dlt/destinations/snowflake/configuration.py Outdated Show resolved Hide resolved
dlt/destinations/snowflake/snowflake.py Outdated Show resolved Hide resolved
dlt/common/destination/capabilities.py Outdated Show resolved Hide resolved
dlt/destinations/snowflake/configuration.py Show resolved Hide resolved
dlt/destinations/snowflake/configuration.py Outdated Show resolved Hide resolved
@rudolfix rudolfix mentioned this pull request Jun 19, 2023
@rudolfix
Copy link
Collaborator

@steinitzu for some reason snowflake tests are not running

@steinitzu steinitzu marked this pull request as ready for review June 19, 2023 21:24
@steinitzu steinitzu changed the title WIP: Feat/snowflake destination Feat/snowflake destination Jun 19, 2023
rudolfix
rudolfix previously approved these changes Jun 20, 2023
dlt/destinations/snowflake/sql_client.py Show resolved Hide resolved
@rudolfix rudolfix merged commit 7137e84 into devel Jun 20, 2023
@rudolfix rudolfix deleted the feat/snowflake-destination branch June 20, 2023 13:33
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.

[destination] implement loader for Snowflake with PUT stage
2 participants