Add column for non-ISO day_of_week

[davesgonechina]

1. Make explicit that day_of_week is ISO
2. Add non-ISO column for those use cases

[clausherther]

Hi @davesgonechina, thanks for the PR! You're right that's potentially confusing. However, renaming those columns is going to break a lot of people's downstream models, including mine.
I could see the following option though:

• make day_of_week non-ISO and don't rename it. That aligns with some of the macro APIs as well where the non-specific macros like day_of_week are non-ISO (if the db supports it)
  (good thing to remember that the default for some platforms is actually ISO, so I wouldn't want to assume this is always non-ISO in the name)
• add day_of_week_iso to add an explicitly ISO option

Does that work for you?

[davesgonechina]

Hey thanks for the quick response @clausherther! Absolutely, let's not break things - but if renaming is going to break things, presumably making day_of_week non-ISO will also because currently it is ISO? Perhaps the reverse, leaving day_of_week unchanged and adding day_of_week_non_iso?

Also perhaps the new column should be last to prevent any right shift problems downstream?

[clausherther]

@davesgonechina I'm a little more comfortable with what is essentially a "fix" to day_of_week aligning with how the rest of the package thinks about ISO vs non-ISO. I'd rather not add anything called non_iso since that's not language we use anywhere else I think.
I'd ask you to call that out in CHANGELOG.md (the next version would probably be dbt-date v0.2.8) as a fix, highlighting that this is a potentially breaking logic fix (which at least would not require recoding downstream pipelines if users are ok with the new implementation).
I'll probably go through the rest of that model after your PR to also look at other instances of ISO/non-ISO in the column definitions. If so, probably warrants bumping the package to 0.3.0.

[davesgonechina]

@clausherther done. I did not move the day_of_week_iso column to the right end since it is more legible this way.

[clausherther]

@davesgonechina Awesome, looks good so far. I'll run tests tonight and merge to main then. New release probably by early next week. Thanks!

[davesgonechina]

@clausherther macros/calendar_date/day_name.sql uses day_of_week to get long day names in Snowflake which would be isoweek dependent. Changed to map short to long names instead.

[clausherther]

Looks good, thanks for making those changes!

[clausherther]

Good catch!