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: state:modified.macros #3559

Merged
merged 7 commits into from
Aug 12, 2021
Merged

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Jul 12, 2021

resolves #2704
resolves #3278
resolves this tweet

Description

If a macro's SQL is modified, mark any node that depends on it (or depends on a macro that depends on it) as modified.

While I was here, it was trivial to make sub-selectors (finally) happen:

  • state:modified.body
  • state:modified.configs
  • state:modified.persisted_descriptions
  • state:modified.database_representations state:modified.relation
  • state:modified.macros

For review

  • I don't really know how to write recursive functions :)
  • Let's revisit some of the nomenclature here (vs. the original proposal in Subselectors for state:modified #2704). Even though it's a mouthful, I prefer persisted_descriptions vs. just descriptions, to be clear it's the subset with persist_docs enabled. On the other hand, I'm not sure about modified.database_representations vs. something shorter & sweeter like modified.relations.
  • There are a few more same_x checks (same_fqn, same_quoting, same_external) that are resource-specific and wouldn't have dedicated subselectors in this first round. I think that's ok—these aren't ones we've heard as much demand for, and they're still included in the state:modified superset—just worth calling out.

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.

@cla-bot cla-bot bot added the cla:yes label Jul 12, 2021
@jtcohen6 jtcohen6 force-pushed the feature/state-modified-macros branch from 646ee34 to 7bdba68 Compare July 29, 2021 22:03
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 29, 2021 22:03 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 29, 2021 22:03 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 29, 2021 22:03 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 29, 2021 22:03 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 29, 2021 22:03 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 29, 2021 22:03 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 29, 2021 22:10 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 29, 2021 22:10 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 29, 2021 22:10 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 29, 2021 22:10 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 29, 2021 22:10 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 29, 2021 22:10 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 29, 2021 22:17 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 29, 2021 22:17 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 29, 2021 22:17 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 29, 2021 22:17 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 29, 2021 22:17 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 29, 2021 22:17 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Postgres July 29, 2021 22:30 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 29, 2021 22:30 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake July 29, 2021 22:30 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 29, 2021 22:30 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery July 29, 2021 22:30 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 29, 2021 22:30 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift July 29, 2021 22:30 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift August 5, 2021 01:22 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift August 5, 2021 01:22 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Postgres August 5, 2021 01:22 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery August 5, 2021 01:22 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery August 5, 2021 01:22 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake August 5, 2021 01:22 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake August 5, 2021 01:22 Inactive
@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Aug 5, 2021

Thanks for the reviews @nathaniel-may @kwigley! I added some comments where the code wasn't clear.

I ended up renaming state:modified.database_representations to state:modified.relation. I think I like it better that way. Relation is a wonkier word, but it's one that dbt users should be familiar with as indicating database.schema.table.

I spent slightly too long thinking about which of these sub-selector names should be singular and which should be plural. I decided to use "per node" as the frame, such that:

  • one body per node
  • multiple configs per node
  • multiple persisted_descriptions per node (potentially relation + columns)
  • one relation per node
  • multiple macros that a node may depend on

@jtcohen6 jtcohen6 force-pushed the feature/state-modified-macros branch from 8605fc6 to 7fcc4eb Compare August 12, 2021 20:38
@jtcohen6 jtcohen6 temporarily deployed to Postgres August 12, 2021 20:38 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake August 12, 2021 20:38 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Snowflake August 12, 2021 20:38 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift August 12, 2021 20:38 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Redshift August 12, 2021 20:38 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery August 12, 2021 20:38 Inactive
@jtcohen6 jtcohen6 temporarily deployed to Bigquery August 12, 2021 20:38 Inactive
@jtcohen6 jtcohen6 merged commit b6e534c into develop Aug 12, 2021
@jtcohen6 jtcohen6 deleted the feature/state-modified-macros branch August 12, 2021 21:21
jtcohen6 added a commit that referenced this pull request Aug 12, 2021
* First cut at state:modified for macro changes

* First cut at state:modified subselectors

* Update test_graph_selector_methods

* Fix flake8

* Fix mypy

* Update 062_defer_state_test/test_modified_state

* PR feedback. Update changelog
jtcohen6 added a commit that referenced this pull request Aug 12, 2021
* First cut at state:modified for macro changes

* First cut at state:modified subselectors

* Update test_graph_selector_methods

* Fix flake8

* Fix mypy

* Update 062_defer_state_test/test_modified_state

* PR feedback. Update changelog
TeddyCr pushed a commit to TeddyCr/dbt that referenced this pull request Sep 9, 2021
* First cut at state:modified for macro changes

* First cut at state:modified subselectors

* Update test_graph_selector_methods

* Fix flake8

* Fix mypy

* Update 062_defer_state_test/test_modified_state

* PR feedback. Update changelog
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.

Use depends_on.macros as input to state:modified Subselectors for state:modified
3 participants