include project macros in the manifest the adapter stores locally

[beckjake]

resolves #2301

Description

Macros defined in core and called via the adapter's execute_macro method will now accept local overrides.

Previously, if you wanted to override a macro in dbt that was used for internal purposes (listing schemas, creating schemas, dropping schemas, getting columns in a relation, ...), the only way to do so was in an adapter plugin itself. This is because the adapter uses a special internal manifest that doesn't require any non-macros. Previously, the adapter's internal manifest only looked at macros defined in dbt core and any active adapter plugins. Now, we search the entire project (including dependencies) for macros and use that for the manifest.

Checklist

• 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 updated the CHANGELOG.md and added information about my change to the "dbt next" section.

[beckjake]

@drewbanin this fixes an issue I bet exists in the 0.17.x RPC server as well: if you change a config and reload, it won't be reflected on the adapter level. I think that means quoting changes in dbt_project.yml might not be honored after a SIGHUP.

[jtcohen6]

Nice! This works as I'd expect.

Following on from the dispatch work, I decided to try something: If I'm running on postgres, and define in my local project:

• get_columns_in_relation: overrides
• postgres__get_columns_in_relation: overrides
• default__get_columns_in_relation: does not override

That's true whether I call it as {{ get_columns_in_relation }} or {{ adapter.get_columns_in_relation }}. I'm fine with this behavior, as long as it doesn't surprise you.

[beckjake]

Yeah. I don't think it's what I would dream of, but I also think it's probably one of the least-bad solutions right now.

If you define a macro with the name, dbt will just look that up (get_columns_in_relation).
If you define a macro without the name, dbt will look up get_columns_in_relation. That will call adapter.dispatch(), which will look for adaptertype__get_columns_in_relation in any package, followed by default__get_columns_in_relation in any package.

If you've defined adaptertype__get_columns_in_relation locally, that will win the lookup. but if you only define default__get_columns_in_relation locally, we'll find dbt_adaptertype.adaptertype__get_columns_in_relation first.

This is all a consequence of the fact that adapter-specific macros are just regular macros + how dbt builds the context.

I think the way to get this to behave how you'd expect is to have the adaptertype part of the macro definition (like materializations), and not put adaptertype macros into the context. In that world, default__get_columns_in_relation and adaptertype__get_columns_in_relation wouldn't be visible to the jinja context. They'd only see get_columns_in_relation, which would be resolved ahead of time to the appropriate underlying macro.

The takeaway here from a behavioral perspective is: don't try to override the things adapter.dispatch would call in your project, instead override the macro that would call adapter.dispatch.

[jtcohen6]

Got it! Thanks for the explanation. It's much more important that this override be possible, from the root project, in one way or another. And indeed:

The takeaway here from a behavioral perspective is: don't try to override the things adapter.dispatch would call in your project, instead override the macro that would call adapter.dispatch.

I don't see a need for packages to override adapter macros for use in the root project. As such, I don't feel passionately that we need all adapter macros to work with the level of rigor that materializations do. There may be a subset (e.g. schema tests) for which we decide it's necessary, someday, to wrap the adaptertype more tightly into the definition.