Snapshots aren't correctly handling multiple unique_key values

[ZacharyMcGuire]

Issue

When configuring a snapshot with multiple unique_key values the adapter does not correctly handle the columns created by the get_or_create_relation macro. This results in the 2nd execution adding dbt_unique_key_1, dbt_unique_key_2, etc. to the snapshot table. On a 3rd execution the get_or_create_relation macro produces invalid SQL by using select * in addition to aliasing the unique keys again as dbt_unique_key_1, dbt_unique_key_2, etc.

Root Cause Analysis

I've tracked down the cause to be this section of code, which is working on the assumption unique_key is a single value. As such, the length of missing_columns is greater than 0 and the create_columns macro is executed.

dbt-sqlserver/dbt/include/sqlserver/macros/materializations/snapshot/snapshot.sql

Lines 58 to 74 in bcf4ac9

	{% set missing_columns = adapter.get_missing_columns(staging_table, target_relation)
	| rejectattr('name', 'equalto', 'dbt_change_type')
	| rejectattr('name', 'equalto', 'DBT_CHANGE_TYPE')
	| rejectattr('name', 'equalto', 'dbt_unique_key')
	| rejectattr('name', 'equalto', 'DBT_UNIQUE_KEY')
	| list %}
	{% if missing_columns|length > 0 %}
	{{log("Missing columns length is: "~ missing_columns|length)}}
	{% do create_columns(target_relation, missing_columns) %}
	{% endif %}
	{% set source_columns = adapter.get_columns_in_relation(staging_table)
	| rejectattr('name', 'equalto', 'dbt_change_type')
	| rejectattr('name', 'equalto', 'DBT_CHANGE_TYPE')
	| rejectattr('name', 'equalto', 'dbt_unique_key')
	| rejectattr('name', 'equalto', 'DBT_UNIQUE_KEY')
	| list %}

Suggested Solution

I suggest updating that section similar to how the snapshot materialisation is implemented in dbt-fabric, by creating a list of columns to ignore and appending each unique_key value if it happens to be a list.

Testing

I've monkey-patched this locally and it appears to work as expected with some very limited testing.
For clarity, I've included a comparison of the file with my patch applied.

---

I'd be happy to also raise this as a PR, once I'm not on the work PC, if it makes things easier.

[ZacharyMcGuire]

Upon further review it appears the 1.9 adapter update doesn't add any support for the 1.9 dbt-core updates to snapshot configuration.

This creates further issues, for instance with the merge SQL. Again, the 1.9 update for dbt-fabric appears to appropriately resolve these in a way that's compatible with SQL Server.

[Benjamin-Knight]

I believe this is resolved by #628.

Due to this adapter handing off a lot of functional testing to the dbt-fabric adapter and implementing only a selection of its own tests the 1.9 upgrade showed the functionality as passing even though it was as above broken.

I had to fix this particular issue along with the incorrect handling of snapshot configurations to get all the copied over versions of the functional tests.