dbt-core icon indicating copy to clipboard operation
dbt-core copied to clipboard

[CT-894] [Feature] Migrate array cross-db macros from dbt-utils

Open joellabes opened this issue 2 years ago • 4 comments

Is this your first time opening an issue?

Describe the Feature

A follow-on to #4813 and its follow-ons - https://github.com/dbt-labs/dbt-utils/pull/595 added some new array-y macros:

  • array_append
  • array_concat
  • array_construct
  • cast_array_to_string

These have missed the bus for inclusion in Core v1.2, but should be added for 1.3.

FYI: My ongoing plan for any future macros is detailed here - in short, if someone need a new cross-db macro, they add it to their own package until the next minor version of dbt Core, at which point they can swap to the official one. If you have any pushback on that strategy, please let me know!

Describe alternatives you've considered

  • Instead of being added to dbt Core, they migrate back to the https://github.com/dbt-labs/dbt-project-evaluator project. They seem useful and applicable in multiple contexts so I don't think this is the right move.
  • They could stay in utils forever, but it sorta defeats the purpose of moving all of their siblings out.

Who will this benefit?

No response

Are you interested in contributing this feature?

I'm sorta hoping Doug might 😅

Anything else?

No response

joellabes avatar Jul 25 '22 02:07 joellabes

FYI: My ongoing plan for any future macros is https://github.com/dbt-labs/dbt-utils/discussions/487#discussioncomment-3185750 - in short, if someone need a new cross-db macro, they add it to their own package until the next minor version of dbt Core, at which point they can swap to the official one. If you have any pushback on that strategy, please let me know!

This strategy makes sense to me. I think low-level "cross-db" macros should skip over dbt-utils entirely. We may decide that a macro is operating at a higher level of complexity / utility, and that dbt-core isn't the right place for it, at which point we can try to find another long-term home.

I think our requirements for a cross-db macro to make its way into dbt-core should be:

  • It must be implemented on the five adapter plugins that dbt Labs maintains
  • It must have functional testing

Given that adding one macro requires opening PRs across several adapter plugin repos, it is nicer to batch up many at once. In theory, though, these could be good opportunities for external contribution.

I've just created a new label, utils, to keep track of this and other follow-on work from the cross-db macro migration.

jtcohen6 avatar Jul 25 '22 09:07 jtcohen6

@dbeatty10 let's sync if this is something you have bandwidth for and would like to do. We could even get you paired with an Adapters person to do it together if that makes sense.

leahwicz avatar Jul 26 '22 17:07 leahwicz

Count me in @leahwicz

I can get the PRs opened and the macros moved over. The thing that I would welcome the most help with:

  • robust testing

dbeatty10 avatar Jul 26 '22 18:07 dbeatty10

Thanks @dbeatty10! I'm going to assign this to you then and then once you start working on this and looking for testing help, let me know and we can coordinate

leahwicz avatar Jul 26 '22 20:07 leahwicz