search

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
478 stars 114 forks source link

[BUG] Concat should be replaced with concat_ws in sqlserver from dbtVault version 0.9.1 onwards (version 0.9.0 is working and older ones) #188

Closed koillinengit closed 3 weeks ago

koillinengit commented 1 year ago

Describe the bug From version 0.9.1 concat_ws is replaced with concat in hashed views in sqlserver. It gives an error: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The concat function requires 2 to 254 arguments.

Environment

dbt version: dbt-core 1.3.3, dbt-sqlserver 1.3.1 dbtvault version: >= 0.9.1 Database/Platform: Microsoft SQL Server Standard (64-bit) 15.0.2101.7, Debian 5.10.127-2 (2022-07-23) x86_64, Python 3.9.2

To Reproduce Steps to reproduce the behavior:

  1. Dbt run
  2. See error

Expected behavior Concat should be replaced with concat_ws to support more than 254 concanated fields

Screenshots image

Log files In progress or failed ('42000', '[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The concat function requires 2 to 254 arguments. (189) (SQLMoreResults)')

Additional context https://github.com/Datavault-UK/dbtvault/blob/master/macros/internal/metadata_processing/concat_ws.sql

Concat_ws has been in SQL Server 2017 (14.x) and later versions https://learn.microsoft.com/en-us/sql/t-sql/functions/concat-ws-transact-sql?view=sql-server-ver16

AB#5364

DVAlexHiggs commented 1 year ago

Hi! Thanks for this.

We originally implemented the concat macro using concant_ws, however we had issues with consistency across platforms, as not all platforms have concat_ws available.

Saying this, I completely agree it should still be used where supported, and the limitations on other platforms with the regular concat documented in our docs. The downside is inconsistency across platforms, however, I think this is better than the alternative of the dbtvault concat macro not working at all for large concatenations on some platforms.

We shall add this to the backlog and get it out in a release soon. Thank you

koillinengit commented 1 month ago

Is there anyway to get this implemented? I'm not able to upgrade to automaDV versions at all. I have to use dbtvault 0.9.0 with our sqlserver implementtion because of the limit of 254 catenation strings.

DVAlexHiggs commented 4 weeks ago

Is there anyway to get this implemented? I'm not able to upgrade to automaDV versions at all. I have to use dbtvault 0.9.0 with our sqlserver implementtion because of the limit of 254 catenation strings.

Hello! We've implemented and tested this and it'll be in the next release (this week!). My only advice would be that even though this fixes it, consider reviewing your model or splitting your satellites a bit more - hundreds of columns in a hashdiff or a primary key is a sign of an incorrect data model and/or a very overloaded satellite.

Thanks for your patience on this one!

DVAlexHiggs commented 3 weeks ago

Fixed and released in v0.11.0, Thank you for your patience on this one!

If the issue persists, please re-open this issue.