Consolidate date macros into timestamps.sql

[colin-rogers-dbt]

resolves #5521

Description

Checklist

• [ ] I have read the contributing guide and understand what's expected of me
• [ ] 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 opened an issue to add/update docs, or docs changes are not required/relevant for this PR
• [ ] I have run changie new to create a changelog entry

[github-actions[bot]]

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

[colin-rogers-dbt]

I like the idea of trying to also incorporate the changes I recommended in https://github.com/dbt-labs/dbt-core/issues/5521#issuecomment-1246795511, which would enable the full removal of dbt_utils.current_timestamp by dbt-utils v1.0.

Will pull those changes in as well

What do you think about deprecating date_function + sql_now()? That's the last tine in our fork of inconsistency.

I like the idea but should probably be a follow on pr

Lastly, we'll want companion PRs in all our adapter plugins, and to test each of them thoroughly. I don't think our existing suite of functional tests is sensitive enough to catch subtly breaking changes in these macros.

I'm thinking I'll add a functional test into core and we can copy that one into the adapters

[colin-rogers-dbt]

Could you implement a test that confirms that current_timestamp_in_utc yields the expected value as well as the expected data type? I'd suggest something like this.

While we certainly could I do not see a compelling case for doing so as we already have added more testing for this than other macros, if a particular adapter has this as a concern it wouldn't be difficult to add this in the adapter specific implementation.

The type of thing I'm worried about is if a user has their database timezone set to some non-UTC timezone and somehow current_timestamp_in_utc accidentally reports a timezone-naive local time that isn't actually in alignment with UTC.

This is a valid concern for redshift where the established behavior for _utc relies on it's default behavior but adding a test for utc won't actually catch this.

Do you know if the data type (or value) of current_timestamp_in_utc will be different than the implementation within dbt-utils for any adapters? (I didn't check those implementations, so I don't know.)

It will not, I'm specifically checking for that in the adapter level tests.

[colin-rogers-dbt]

Received offline feedback concerning the need to include a "backcompat" macro for current_timestamp_in_utc as we may be diverging from dbt-utils versions in a breaking manner. Digging in there are two adapter specific _utc implementations (that don't just use current_timestamp):

• Snowflake
• Postgres
• Redshift Looking at the three we aren't changing the datatype (and checking the values)

[jtcohen6]

@colin-rogers-dbt @dbeatty10 Thanks for all the great discussion in here!

I had a chance to chat with Doug earlier today, and hear his perspective on the complexity that he's been going deep on, and simmers under the surface. There's quite a lot!

To simplify matters, here are my proposed next steps:

• Split this PR into two.
• The first PR should just provide backward compatible implementations for dbt_utils.current_timestamp and dbt_utils.current_timestamp_in_utc — via dbt.current_timestamp_backcompat and dbt.current_timestamp_in_utc_backcompat, respectively — so that we can unblock dbt-utils v1.0. Exit criteria: The return types and values of these back-compat macros match exactly the return types and values of the deprecated dbt_utils macros. (I'd also be happy to see the reorganization / centralization of our existing macros.)
• The second PR should add in dbt.convert_timezone + dbt.current_timestamp_in_utc, as net-new macros that do not exist currently. I've opened a new issue outlining the spec for both: https://github.com/dbt-labs/dbt-core/issues/5969
• For future (tech debt): https://github.com/dbt-labs/dbt-core/issues/5967

In terms of timing, we only need the first PR for inclusion in v1.3, to unblock dbt-utils v1.0. The second PR could be merged for inclusion in v1.4 instead. That's my main motivation for splitting up the work into two separate PRs, which can be merged separately.

[colin-rogers-dbt]

One easy way to test the backwards compatibility is just to compare the new SQL to the old SQL. If they are the same, then we are done.

Good idea, I've added an assertion to our functional tests to validate the compiled SQL

[gshank]

Yes, we won't be implementing all adapter zone tests for all adapters. We did expect that everything in the "basic" directory would be a requirement for all adapters, but tests in the adapter zone outside of that are optional. Not all adapters implement all features.