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

Snowflake: Add expression parameter to columns #275

Merged
merged 13 commits into from
Apr 19, 2024

Conversation

kyleburke-meq
Copy link
Contributor

@kyleburke-meq kyleburke-meq commented Apr 4, 2024

resolves: #140

Adds the ability for custom expressions and identifiers to external sources in snowflake.

Description & motivation

All credit goes to @ipear3 and his original PR: #209
I am just resolving a merge conflict and want to see the code implemented.

Checklist

  • I have verified that these changes work locally
  • I have updated the README.md (if applicable)
  • I have added an integration test for my fix/feature (if applicable)

@cakkinep
Copy link
Contributor

cakkinep commented Apr 5, 2024

depending on the order of the merges, we will have to resolve conflicts with #257 , i am adding column alias functionality there.

@kyleburke-meq
Copy link
Contributor Author

@dataders is there something preventing this PR from being merged?

@the-davidsn
Copy link

+1
This feature is pretty helpful - especially when trying to handle timestamps w/ parquet files :)

@kyleburke-meq
Copy link
Contributor Author

@dataders I have updated the integration test so it is now passing. I also resolved the conflict with #257.

Would you kindly review this again?

@cakkinep
Copy link
Contributor

@kyleburke-meq thanks for resolving the conflicts and adding the integration test for the changes.

Copy link
Collaborator

@dataders dataders left a comment

Choose a reason for hiding this comment

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

thanks @kyleburke-meq (and @jpear3)!

@dataders dataders merged commit d318127 into dbt-labs:main Apr 19, 2024
3 checks passed
{%- set column_quoted = adapter.quote(column.name) if column.quote else column.name %}
{%- set column_alias -%}
{%- if 'alias' in column and column.quote -%}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this change introduced the bug - #315

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is a bug. left a reply in that issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah actually I think I see the issue now. We should be using column_quoted instead of column_alias here I think?:

{%- set col_id = 'value:c' ~ loop.index if is_csv else 'value:' ~ column_alias -%}

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.

Snowflake External Tables - Ability to add in custom col_expression
4 participants