-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use all adapter names for adapter.dispatch #2923
Labels
Milestone
Comments
jtcohen6
added
enhancement
New feature or request
adapter_plugins
Issues relating to third-party adapter plugins
labels
Dec 1, 2020
friendly, curious bump. is this still something in consideration for Margaret Mead. Relatedly, I just peeped MaterializeInc/materialize-dbt-utils. Pretty cool to see folks catching dispatch fever! |
Yes, this is still something I'd like to do for Margaret Mead! The change itself is very straightforward. We'll want to add some tests for it, and it will likely require a few changes in packages, especially dbt-utils. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Describe the feature
The
dbt-redshift
adapter plugin inherits from thedbt-postgres
adapter plugin. This reflects the fact that they share a lot of code: connection mechanism (psycopg2
), basic syntax, types, materializations, etc. (This can be obfuscated by the fact that their shared implementations of macros are also often thedefault__
dbt implementations.)Today, when dbt dispatches
package_name.macro_name
macro on Redshift, it looks for:my_project.redshift__macro_name
my_project.redshift__macro_name
package_name.postgres__concat
package_name.postgres__concat
I think it should instead look for:
my_project.redshift__macro_name
my_project.postgres__macro_name
my_project.default__macro_name
package_name.redshift__macro_name
package_name.postgres__macro_name
package_name.default__macro_name
This is a possibility we weighed in #2679, to the point that we make it possible as a one-line change:
https:/fishtown-analytics/dbt/blob/5ba5271da99bc1ef7fbad2f7d0b45634087300fa/core/dbt/context/providers.py#L104-L109
What's the risk?
There are cases today where Redshift uses the
default__
implementation and Postgres uses a custompostgres__
implementation, e.g.dbt_utils.dateadd
.I don't think this is a breaking change per se, because Redshift largely supports PostgreSQL syntax, but we may want to add implementations that explicitly preserve existing behavior:
Who will this benefit?
This will enable maintainers of similar-yet-slightly-different plugins, e.g.
dbt-sqlserver
anddbt-synapse
, who can then "share" macro implementations between parent and child adapters. See microsoft/dbt-synapse#18.It's possible to imagine many more adapter inheritances in the future:
dbt-databricks
ordbt-delta
that inherits fromdbt-spark
dbt-hive
that inherits fromdbt-spark
ordbt-presto
dbt-materialize
that inherits fromdbt-postgres
The text was updated successfully, but these errors were encountered: