Skip to content
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

Update comment re: private method rename: _add_ctes #2887

Closed
jtcohen6 opened this issue Nov 12, 2020 · 0 comments · Fixed by #2966
Closed

Update comment re: private method rename: _add_ctes #2887

jtcohen6 opened this issue Nov 12, 2020 · 0 comments · Fixed by #2966
Labels
bug Something isn't working

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Nov 12, 2020

Describe the bug

In #2834, it looks like we switched the name of the internal method from _insert_ctes to _add_ctes. That's fine, we should just also update the comment that serves as a hint for plugin authors:

https://github.com/fishtown-analytics/dbt/blob/3af02020ff697cbc34b7bf7d538e99ccd1979afd/core/dbt/compilation.py#L353-L358

Context

Adapter plugin authors for databases that don't support nested CTEs should have a clear point of intervention to alter dbt's behavior when constructing data tests and ephemeral models:

If we have a little extra time, I'd like to prove it's actually possible to override this behavior for data tests and ephemeral models by overriding _add_ctes in an adapter plugin.

@jtcohen6 jtcohen6 added the bug Something isn't working label Nov 12, 2020
@jtcohen6 jtcohen6 added this to the Kiyoshi Kuromiya milestone Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant