Adds 3 parameters to unpivot: remove, field_name, value_name

[remigabillet]

This makes dbt_utils.unpivot a bit more like pandas.melt by adding the ability :

• to name the resulting columns
• to remove some columns from the operation

[remigabillet]

@drewbanin check this out, it's my first DBT contribution :-D

[drewbanin]

Hey @remigabillet - thanks for the heads up! I'll give this a look today

(cc @dwallace0723, the original author of this macro 😉)

[drewbanin]

Thanks for the PR @remigabillet! This is looking really good! Some comments below, and one test failure, but I think this is super close.

Database Error in model test_unpivot_original_api (models/sql/test_unpivot_original_api.sql)
  000904 (42000): 018cf03d-0377-c5e3-0000-0e6902c5023a: SQL compilation error: error line 19 at position 50
  invalid identifier 'FIELD_NAME'
  compiled SQL at target/compiled/dbt_utils_integration_tests/sql/test_unpivot_original_api.sql

[drewbanin]

Cool! Just kicked off a build :)

I think we'll be good to merge this when the tests pass!

[remigabillet]

hey @drewbanin it still doesn't work. Sorry to keep bothering you. I'm happy to iterate on it but I don't have snowflake so I need the CI to run automatically. Is there a way for me to do that?

[drewbanin]

Hey @remigabillet - we disabled builds on PRs to prevent nefarious users from logging out our database credentials! We've permissioned everything accordingly, but we still review PRs before we kick off CI builds. Would love to figure out a setup (dedicated redshift+snowflake+bq instances) that would let us make this CI process more friendly for contributors though.

Not sure if you saw my comment above, but i think all you need to do to get these tests to pass is lowercase the prop column returned by the unpivot macro in test_unpivot.sql when target.name == 'snowflake'. Something like:

select
    customer_id,
    created_at,
    case
        when '{{ target.name }}' = 'snowflake' then lower(prop)
        else prop
    end as prop,
    value

from (
    {{ dbt_utils.unpivot(
        table=ref('data_unpivot'),
        cast_to=dbt_utils.type_string(),
        exclude=exclude,
        remove='name',
        field_name='prop',
        value_name='val'
    ) }}
) as sbq

Once you make that change I can kick off the CI build and get this thing merged! This is also super minor, lmk if you'd prefer for me to make that particular change.

[remigabillet]

Got it re: CI.

I actually wrote the test a little differently, removing that conditional. Isn't it the same thing?

select
    customer_id,
    created_at,
    prop,
    val

from (
    {{ dbt_utils.unpivot(
        table=ref('data_unpivot'),
        cast_to=dbt_utils.type_string(),
        exclude=exclude,
        remove='name',
        field_name='prop',
        value_name='val'
    ) }}
) as sbq

[remigabillet]

I'm still unclear on snowflake's casing, do I need to add conditional around the 'prop' and 'val' arguments?

[drewbanin]

The conditional really just needs to be applied to prop. We're going to fetch the column names in the data_unpivot model from Snowflake. Snowflake will return upper-case identifiers like 'SEGMENT' and 'STATUS'. In order for the equality comparison against the expected output to pass, we need to lowercase these identifiers. We can actually remove the conditional and just always lowercase that prop column, eg:

select
    customer_id,
    created_at,
    lower(prop) as prop,
    val

but I like the idea of making it explicit that Snowflake is the odd one out here.

[remigabillet]

Thanks for the explanation @drewbanin. I think I get it now, Snowflake uppercases column names so I need to lowercase before I compare my test output with the expected results.
I agree that it's better to be explicit also! 👍
I think it'll work now.

[drewbanin]

cool! This last change LGTM - looks like there's just a tiny typo where you project the column as 'prop' instead of prop. Fix that and we'll be good to go!

[drewbanin]

thanks @remigabillet, merging!

[remigabillet]

Yes !