calogica/dbt-date

Week start macro returning all nulls

jpmmcneill opened this issue · 1 comments

Currently, the week_start macro (https://github.com/calogica/dbt-date/blob/main/macros/calendar_date/week_start.sql) is returning all nulls in snowflake. I've only been able to test it there. I'm unsure if it's working in other DB environments.

It gets compiled to (i've removed aliases here):

    case
        when
            case
                when
                    date_part('dayofweek', 
                    date_trunc('week', date_day)
                    ) = 7
                    then 1
                else
                    date_part('dayofweek', 
                    date_trunc('week', date_day)
                    ) + 1
            end = 1
    then cast(

    dateadd(
        day,
        0,
        date_day
        )

 as date)
end as week_start_date

The above is a copy/paste from the get_date_dimension macro section that uses it. It's clear from the code above that there are a few control cases missing from the code. The first case statement

I'm not 100% sure what the expected behaviour of this macro is versus the iso_week_start macro is. It reads as if it wants sunday to be the start of the week. If you can confirm this @clausherther I'm happy to submit a PR to fix this :). I think we'll just need to add a few more control cases for different "day of weeks" for the date_trunc -> week.

Thanks @jpmmcneill for this issue! I think we when we first wrote this, date_part('dayofweek', date_trunc('week', date_day)) returned 0 for Sundays (vs isoweek which should be 7 for a Sunday). That case statement is meant to capture the different week_policy settings for Snowflake, but I guess it's not capturing all the cases.
I'm surprised the integration tests pass on this one, since we're testing the macros against hard coded data.
If you feel like pushing a PR for this, that'd be great! Otherwise, I think I can to this by next week. Thanks!