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

Feature/drop relation/ct 2581 #7626

Merged
merged 4 commits into from
May 15, 2023
Merged

Conversation

mikealfare
Copy link
Contributor

resolves #7625

Description

Allow for a more configurable drop_relation() macro; preserve backwards compatibility.

  • create adapters/drop_relation.sql
  • update drop_relation() to condition on relation.type and dispatch to the appropriate macro
  • create drop_table(), drop_view(), and drop_materialized_view()

Checklist

@mikealfare mikealfare requested a review from a team as a code owner May 15, 2023 16:34
@mikealfare mikealfare self-assigned this May 15, 2023
@mikealfare mikealfare requested a review from a team as a code owner May 15, 2023 16:34
@mikealfare mikealfare requested review from colin-rogers-dbt, aranke and davidbloss and removed request for a team May 15, 2023 16:34
@cla-bot cla-bot bot added the cla:yes label May 15, 2023
Copy link
Contributor Author

@mikealfare mikealfare May 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved all of the drop_relation() macros into their own file because now there's several macros that are all related to each other. However, I could also see an argument for keeping drop_relation() and default__drop_relation() in adapters/relation.sql to act as an interface, and then putting macros like drop_view() in a file adapter/view.sql along with similar implementations of other macros (e.g. create_view(), rename_view(), etc.). In the latter pattern, we'd also have adapters/table.sql and adapters/materialized_view.sql files.

Copy link
Member

@aranke aranke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mikealfare mikealfare merged commit 47e7b1c into main May 15, 2023
@mikealfare mikealfare deleted the feature/drop-relation/ct-2581 branch May 15, 2023 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-2581] [Feature] Allow for more flexible drop statements in drop_relation()
2 participants