dbt-labs / dbt-adapters

Apache License 2.0
28 stars 38 forks source link

[CT-2178] Handling column names having `/` while altering the table #63

Open preethi8p opened 1 year ago

preethi8p commented 1 year ago

Is this a new bug in dbt-snowflake?

Current Behavior

We had few columns with "/" in the column names. During incremental runs, when dbt runs the alter statement to either add or drop a column it is failing with the below error in the global macro columns.sql ERROR_MESSAGE SQL compilation error: syntax error line 3 at position 12 unexpected '/'. image

Fix Modified global macro to add proper quoting to the column names during alter table statement.

Expected Behavior

We expect that the alter statements to add or drop columns, must run successfully without any failure. Basically if we have added a column in the previous layer(say load layer), and during incremental load in the next layer(raw layer) it should run the alter statement and add the column to the raw table. Please note it failed because it has a character "/" in the column name.

Steps To Reproduce

  1. Add a column to a table in load layer with a name like "KOND/CC" (with a /)
  2. Do an incremental run so that this column should be added in the next layer which uses this table as source.
  3. Alter statement to add the column is triggered and it fails with error mentioned above
  4. As a fix, we need to include the column within double quotes in the macro "snowflake__alter_relation_add_remove_columns" as we are using Snowflake.
  5. Upon fix it will run the alter statement successfully and add that column to the table.

Relevant log output

Log output with error:
2023-02-16T17:23:47.3708425Z 17:23:47  Database Error in (models/raw/cust.sql  
2023-02-16T17:23:47.3714555Z 17:23:47    001003 (42000): SQL compilation error:
2023-02-16T17:23:47.3720917Z 17:23:47    syntax error line 3 at position 12 unexpected '/'.
2023-02-16T17:23:47.3727341Z 17:23:47    syntax error line 3 at position 16 unexpected '/'.

Log output after fix:
2023-02-20T12:39:08.9562443Z 12:39:08  121 of 1023 OK created incremental model qa_raw.cust .......... [SUCCESS 0 in 193.62s]

Environment

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

Additional Context

No response

jtcohen6 commented 1 year ago

Linking:

preethi8p commented 1 year ago

Have raised a Pull Request for the issue - Update columns.sql dbt-labs/dbt-core#7025

github-actions[bot] commented 1 year ago

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

preethi8p commented 1 year ago

I hope this was accepted as a valid issue, so is this PR completed? How do I track? If yes, then please provide me the link to verify the same else please let me know the next step.

dbeatty10 commented 9 months ago

This was reported multiple times and covering a plurality of adapters:

Acceptance criteria

Implementation detail was originally described here: https://github.com/dbt-labs/dbt-core/pull/7025#issuecomment-1606152211

In short:

  1. Use {{ adapter.quote() }} as described in dbt-labs/dbt-adapters#160 (comment)
  2. Add test case(s) to the on_schema_change tests for a column that contains a special character (/, ", etc.) or is a reserved keyword, e.g. from.
  3. Inherit these test cases within each adapter (dbt-snowflake, etc).

More detail

After doing a git grep "column\.name" dbt/, here are three places I noticed in the code base to update:

  1. https://github.com/dbt-labs/dbt-adapters/blob/154ca610f67906b0affea9a1e85d29126721cafd/dbt/include/global_project/macros/adapters/columns.sql#L126
  2. https://github.com/dbt-labs/dbt-adapters/blob/154ca610f67906b0affea9a1e85d29126721cafd/dbt/include/global_project/macros/adapters/columns.sql#L130
  3. https://github.com/dbt-labs/dbt-adapters/blob/154ca610f67906b0affea9a1e85d29126721cafd/dbt/include/global_project/macros/materializations/snapshots/helpers.sql#L11

Since the 3rd one is related to snapshots, ideally there would be at least one snapshot-specific functional test that needs a quoted identifier to handle a special character or reserved keyword.

github-actions[bot] commented 1 month ago

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

preethi8p commented 1 month ago

Its been too long I have opened this issue, now I don't even remember where to track back. Now I am not clear on all the comments, so please let me know if this can be published or not ? And what should be my next step towards this? TIA.