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

Respect Snowflake Quoting #930

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

Conversation

ludwig-solita
Copy link

Problem

Quoting isn't respected when altering Snowflake tables with non-ansi-sql compliant names. This throws an error when syncing columns in incremental models. I didn't create a new issue since this already seems to recognize the problem
dbt-labs/dbt-adapters#250

I created a new PR since I messed up the commit history with an unverified Github Account. Original PR: #923
@VersusFacit

image
image
image

Solution

Instead of just using column.name for the alter statements I've added the adapter.quote to properly respect the quoting configuration in yaml-files

Tests

I haven't written any tests in this project before so any help would be appreciated. The below code would essentially be the test.

Configure project.yml to use quoting for identifiers

quoting:
  identifier: true

Create the table with CTAS

{{
    config(
        materialized="incremental",
        on_schema_change='sync_all_columns'
    )
}}
SELECT 'Hello world!' as original_column

Add new columns

{{
    config(
        materialized="incremental",
        on_schema_change='sync_all_columns'
    )
}}
SELECT 'Hello world!' as original_column
,'Greetings. I come from a non-ansi compliant planet' as "NoN AnSi CoMpLiAnT CoLuMn"
,'Greetings. I come from an ansi compliant planet' as ANSI_COMPLIANT_COLUMN

Checklist

  • I have read the contributing guide and understand what's expected of me

  • I have run this code in development and it appears to resolve the stated issue
    image

  • This PR includes tests, or tests are not required/relevant for this PR
    It's a quite minor change.

  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX
    It does affect the adapter. I've had discussions with @ernestoongaro regarding the problem.

@cla-bot cla-bot bot added the cla:yes label Mar 15, 2024
@dataders
Copy link
Contributor

dataders commented Mar 21, 2024

@ludwig-solita is there an open dbt-snowflake issue for this? If not can you please open one? thanks! I couldn't find one on dbt-labs/dbt-core#923 either

@ludwig-solita
Copy link
Author

Quoting isn't respected when altering Snowflake tables with non-ansi-sql compliant names. This throws an error when syncing columns in incremental models. I didn't create a new issue since this already seems to recognize the problem

Found this one in the core repo. dbt-labs/dbt-adapters#250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants