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

Use static analyzer to extract unrendered_config from config() #3680

Closed
jtcohen6 opened this issue Aug 3, 2021 · 4 comments
Closed

Use static analyzer to extract unrendered_config from config() #3680

jtcohen6 opened this issue Aug 3, 2021 · 4 comments
Labels
enhancement New feature or request stale Issues that have gone stale state Stateful selection (state:modified, defer)

Comments

@jtcohen6
Copy link
Contributor

jtcohen6 commented Aug 3, 2021

The ideas in #2714 set the course for a lot of the static analysis we've pursued in the time since. We've managed to do some of the things enumerated there, with the biggest coups being:

  • a static analyzer that can, in <1 ms, return the refs, sources, and configs of a simple model
  • a complete and reliable depends_on.macros for macros

We pursued many of those things for performance reasons, though there are spinoff benefits for state:modified as well. One enticing possibility: extracting and storing the unrendered_configs based on the arguments within a Jinja config() macro. For instance:

{{ config(
  materialized = ('table' if target.name == 'prod' else 'view')
) }}

select 1 as id

Today, dbt can only store the rendered version of that config, 'table' or 'view', captured during render_update. If a user switches targets between dev and prod, dbt will detect that as a modification (a false positive).

Imagine instead: Our tree-sitter grammar and extractor could return 'table' if target.name == 'prod' else 'dev' as node.unrendered_config.materialized, then pass that expression in for Jinja rendering, and store the rendered value as node.config.materialized. When it comes time to compare modifications, we can compare the unmodified, unrendered Jinja expressions.

I think this will be a much more fruitful endeavor than trying to parse the Jinja AST, which we found unpleasant in our work on dispatch backwards compatibility. At a conceptual level, however, the guidance from #2714 still apply:

We'll probably have to come up with some restrictions. We can do this pretty simply as a first pass, and fall back to the existing behavior (figuring out config by evaluating the jinja) when we reach the boundaries of our ability to figure it out. As a first pass, I think we can pretty safely say that any/all of these are ok to punt on for this pass (optimization fences, sort of):

  • var and env_var calls: {{ config(materialized=var('materialization_type')) }}
  • calling macros in config: {{ config(materialized=my_macro()) }}
  • macros that call config: {% macro my_macro() %} {{ config(materialized='view') {% endmacro %}
  • using variables in config: {% set materialized_value = 'view' %} {{ config(materialized=materialized_value) }}
  • using ** in config arguments {{ config(**kwargs) }}
  • combinations/indirect calls: {% set materialization_macro = my_macro %}{{ config(materialized=my_macro()) }}
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Oct 6, 2022
@github-actions
Copy link
Contributor

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest; add a comment to notify the maintainers.

Copy link
Contributor

github-actions bot commented Jul 4, 2024

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please comment on the issue or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Jul 4, 2024
Copy link
Contributor

Although we are closing this issue as stale, it's not gone forever. Issues can be reopened if there is renewed community interest. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stale Issues that have gone stale state Stateful selection (state:modified, defer)
Projects
None yet
Development

No branches or pull requests

2 participants