[CT-314] [Feature] Migrate cross-db functions from dbt-utils to definition in Core, implementation in adapters

[joellabes]

Is there an existing feature request for this?

• I have searched the existing issues

Describe the Feature

As discussed in dbt-labs/dbt-utils#487, it makes more sense for the cross-db macros (length, dateadd, etc...) to be defined as Core constructs and implemented by the various adapters.

Why?

• Reduces the number of packages that need a dependency on dbt-utils, which reduces the likelihood of conflicts
• This is largely stable code, which means the relatively rapid iteration pace of the rest of utils is actually a risk if all you need is a compatible way of getting the current timestamp
• As the number of adapters continues to grow, vendors and community members have to ship a "shim" version of dbt utils anyway; if they can implement that inside of their adapter's code then it's easier for them as well.

Describe alternatives you've considered

Doing nothing

Who will this benefit?

Package developers and maintainers

Are you interested in contributing this feature?

Sure am!

Anything else?

I've build a list of the different macros and which ones will need to be custom implemented by different adapters here: https://docs.google.com/spreadsheets/d/1xF_YwJ4adsnjFkUbUm8-EnEL1r_C-9_OI_pP4m4FlJU/edit#gid=1062533692

Also interested in whether there's any better ways of implementing _is_ephemeral and _is_relation inside of Core as opposed to the current approach!

[jtcohen6]

Also interested in whether there's any better ways of implementing _is_ephemeral and _is_relation inside of Core as opposed to the current approach!

Really good callouts, I had forgotten about these. Let's aim to rewrite these as Python class or context methods instead. We already have an is_ephemeral check for nodes, though not for relations (result of ref). Maybe the latter wants to be a type check: isinstance of BaseRelation.

[joellabes]

Halfway through building that spreadsheet, I discovered that some macros that we had to make cross-compatible are now globally implemented (e.g. SF and BQ both now support right, where I assume they didn't in 2019). It makes sense to abandon those hacky versions, so I'll have to take another pass over these and check against the documentation. I've checked the docs and that was an outlier 🦢

One question I still have from a product perspective as opposed to implementation: How many functions do we want to proactively implement, compared to just waiting for them to be necessary? Some examples:

• I added bool_or last week for the metrics package. There's also bool_and which I don't need yet but wouldn't be unreasonable to include
• we have right, why not left?

As we add more adapters, it becomes more necessary to have more of them available to deliver a good experience. E.g. all of the core 4 support concat in the same way, but it's dispatched for t-sql's benefit. Do we just wait for those to come up?

A thought experiment: if we'd already moved these out of utils, and I had needed bool_or to be available, what would have happened?

• Are we willing to add additional functions in patch releases to Core + adapters so that they're quickly available for other adapters to implement?
• Or would I have shipped my own macro which covered the core 4 adapters, and other adapters would have to add the metrics package to the list of dispatch candidates?

[leahwicz]

Hey Joel thanks for pointing that out (this mismatches)! That isn't a good experience if you have one option but not the other (i.e. right but not left lol). IMO we should definitely fix that.

Are we willing to add additional functions in patch releases

To answer this question, I would only target the minor releases and not the patch releases for the additional functionality. If that means we have to span it over 1.1 and 1.2 then so be it and we should prioritize what functionality we want in each release.

[jtcohen6]

Agreed! We'd only add new functions in minor releases, since this also represents a change to the adapter interface (even if not a breaking one).

In the meantime, the existence of dispatch means that users could shim/extend the metrics package to a new, not-previously-supported adapter. In the next minor version of that adapter, we/they could seek to support it by default.

[joellabes]

OK! Sounds good to me 👍

[jtcohen6]

Early June update

We have open PRs to migrate the vast majority of "cross-db" macros from dbt-utils into dbt-core + plugins:

• Lift + shift for cross-db macros dbt-utils#597
• Lift + shift for cross-db macros #5298
• Lift + shift for cross-db macros dbt-redshift#120
• Lift + shift for cross-db macros dbt-snowflake#162
• Lift + shift for cross-db macros dbt-bigquery#192

Immediate next steps

There are two tricky areas that we're splitting out into separate units of work:

• Rewrite type_* macros to use built-in adapter capabilities dbt-utils#598
• [CT-895] [Spike] Consolidate current_timestamp & associates #5521

There's follow-on work in other plugins, to mirror the migration by moving these macros from shim packages → primary plugin. For us, that's dbt-spark (+ spark-utils):

• Initialize lift + shift for cross-db macros dbt-spark#359
• Lift + shift for cross-db macros spark-utils#25

Down the line

We've also opened "tech debt" tickets for shortcomings in the core adapter interface, uncovered during this work:

• [CT-705] Review dbt_utils "helper" methods: _is_relation + _is_ephemeral #5316
• [CT-706] Consolidate data type methods in adapter interface #5317
• [CT-707] Expose check_relations_equal + consolidate with dbt-utils generic tests #5318

[jtcohen6]

Closing this issue as resolved, since we've got the follow-up issues noted above to scope our more targeted work moving forward