This changes makes the macro easier to read and workable on SQL Server

[mikaelene]

resolves #3003

Description

Checklist

• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

[jtcohen6]

Makes a lot of sense to me, @mikaelene. Thanks for this.

I know it's not directly in the scope of the problem you're trying to solve, but I see that the same duplicated logic exists for the Postgres implementation here:

https://github.com/fishtown-analytics/dbt/blob/a4280d7457c32dce072d3224190a4779abc77d77/plugins/postgres/dbt/include/postgres/macros/materializations/snapshot_merge.sql#L5-L17

I think it would be simpler as just:

    update {{ target }}
    set dbt_valid_to = DBT_INTERNAL_SOURCE.dbt_valid_to
    from {{ source }} as DBT_INTERNAL_SOURCE
    where DBT_INTERNAL_SOURCE.dbt_scd_id::text = {{ target }}.dbt_scd_id::text
      and DBT_INTERNAL_SOURCE.dbt_change_type::text in ('update'::text, 'delete'::text)
      and {{ target }}.dbt_valid_to is null;

Is that a change you'd be willing to make here as well, for greater consistency across adapters?

[mikaelene]

Makes a lot of sense to me, @mikaelene. Thanks for this.

I know it's not directly in the scope of the problem you're trying to solve, but I see that the same duplicated logic exists for the Postgres implementation here:

Is that a change you'd be willing to make here as well, for greater consistency across adapters?

Changed in Postgres as well

[jtcohen6]

@cla-bot check