dbt-msft / tsql-utils

dbt-utils for the dbt-msft family of packages
MIT License
25 stars 25 forks source link

decide API for dbt-utils macros that work without issue in TSQL #23

Closed dataders closed 3 years ago

dataders commented 3 years ago

consider dbt_utils.star() which works without issue in TSQL. But, because it isn't currently implemented in tsql-utils, you have to call dbt_utils.star() to use it... which is overly complicated?

The simplest solution IMHO involves https://github.com/fishtown-analytics/dbt/issues/2923 where if a user calls tsql_utils.star, it would first look for a star macro in the tsql-utils package and if it can't find one, then it would go look in dbt-utils and find dbt-utils.star. However, that's a medium- or long-term fix.

In the short-term, @jtcohen6 do you think it makes the most sense to have 100% dbt-utils macro cover in tsql-utils, but macros that do not need modification are simply dispatched back to dbt-utils, like below? Otherwise, I think it might be challenging for users to have to remember to call dbt-utils for some macros and tsql-utils for others....

{% sqlserver__star(from, relation_alias=False, except=[]) %}
    {% do return( dbt_utils.star(from, relation_alias, except) ) %}
{% endmacro %}

{% synapse_star(from, relation_alias=False, except=[]) %}
    {% do return( sqlserver.star(from, relation_alias, except) ) %}
{% endmacro %}
jtcohen6 commented 3 years ago

Users should always call macros from the dbt_utils namespace. That's the magic of dispatch. When I call dbt_utils.star, so long as I have both dbt-utils + tsql-utils packages installed, dbt figures out (based on my current adapter and the available macros) which implementation to use behind the scenes, and it "just works."

So if both SQLServer + Synapse can use the default__ implementation of a macro defined in dbt_utils, you shouldn't need to define it at all.

The only time users should reference macros from the tsql_utils namespace in their root projects is if you've created net-new utility macros, exclusive to this package, that do not map to something defined in dbt-utils.

dataders commented 3 years ago

color me embarrassed! looks like I couldn't see the forest for the mycorrhiza...

facepalm