Datavault-UK / automate-dv

A free to use dbt package for creating and loading Data Vault 2.0 compliant Data Warehouses (powered by dbt, an open source data engineering tool, registered trademark of dbt Labs)
https://www.automate-dv.com
Apache License 2.0
492 stars 121 forks source link

[BUG] pit PKs being generated in mixed case post 0.9.0 #163

Closed TeeWallz closed 1 year ago

TeeWallz commented 1 year ago

Describe the bug macros/tables/snowflake/pit.sql is providing mixed case PK and LDTS column names. i.e. instead of "SAT_SATELLITE_PK" we get "sat_satellite_PK" in pits

The names were converted to upper in 0.8.2: https://github.com/Datavault-UK/dbtvault/blob/ca533eafff10978036d60545f109169074494125/macros/tables/snowflake/pit.sql#L146

This means that when an upper case column already exists, the materialisation is unable to insert into a preexisting table due to the error similar to "SQL compilation error: error line x at position x invalid identifier 'SAT_SATELLITE_PK".

If an uppercase sat name is provided in the satellites key in the YAML, the relation generated to the sat fails, and so needs to be lower case.

Environment

dbt version: 1.1 dbtvault version: 0.9.0 Database/Platform: Snowflake / DBT Cloud

To Reproduce Steps to reproduce the behavior:

  1. Already have a dbtvault.pit model/table created using a pit_incremental Materialisation from before 0.9.0. This table will have an upper case "SAT_SATELLITE_PK" and "SAT_SATELLITE_LDTS"
  2. Upgrade to dbt vault 0.9.0
  3. Compile the pit model and the PK now has a mixed column name of "sat_satellite_PK"
  4. When run, the model is unable to find "SAT_SATELLITE_PK" and fails a run.

Expected behavior macros/tables/snowflake/pit.sql should generate upper case names for PK and LDTS fields on the following lines: https://github.com/Datavault-UK/dbtvault/blob/8379d10af3f80f6f500f90d4a2aef2e806fa1235/macros/tables/snowflake/pit.sql#L133 https://github.com/Datavault-UK/dbtvault/blob/8379d10af3f80f6f500f90d4a2aef2e806fa1235/macros/tables/snowflake/pit.sql#L139 https://github.com/Datavault-UK/dbtvault/blob/8379d10af3f80f6f500f90d4a2aef2e806fa1235/macros/tables/snowflake/pit.sql#L145

Screenshots Compiled SQL image

Resulting run image

Compile after changing the macro to run AS {{ dbtvault.escape_column_names("{}_{}".format((sat_name | upper), sat_pk_name)) }},

Result image

Additional context I would like to provide a PR, however I'm unsure if it would be the way you'd want it to be done

DVAlexHiggs commented 1 year ago

Hi thanks for this report. It's a duplicate (see #157) so closing it. Thanks

DVAlexHiggs commented 1 year ago

I am going to re-open this for visibility. Thanks

DVAlexHiggs commented 1 year ago

0.9.1 Released which addresses this issue. Thanks! Please close if this issue is resolved, or let me know if not!

DVARymer commented 1 year ago

Datavault have double checked this in 0.9.1 and looks resolved. Thank you!

image

DVAlexHiggs commented 1 year ago

Closing as this should now be fixed in 0.9.1. Please re-open if not! Thanks for reporting 😄

TeeWallz commented 1 year ago

Thank you very much for fixing this!