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

Test parsing overhaul #2149

Closed
beckjake opened this issue Feb 20, 2020 · 0 comments · Fixed by #2220
Closed

Test parsing overhaul #2149

beckjake opened this issue Feb 20, 2020 · 0 comments · Fixed by #2220
Labels
enhancement New feature or request

Comments

@beckjake
Copy link
Contributor

beckjake commented Feb 20, 2020

Describe the feature

This issue is fallout from working on #2114

For models, dbt creates schema tests by looking at the test name and arguments, converting most arguments into key=str(value) pairs. Arguments that look like dbt functions (ref, doc, source, var, and env_var) are converted into key=value pairs. Then the resulting test is treated like a model: parsed and eventually compiled.

Currently (0.15.3rc1, 0.16.0b1), dbt parses and renders arguments of source tests before building the raw SQL, but doesn't render model tests. This is an oversight, and dbt shouldn't be parsing arguments for source tests either.

However, the issue exposed some deficiencies in behavior with dbt test arguments. In particular, the perfectly reasonable construction of having a test argument like from_condition: created_at::date <= '{{ var('created_at__date') }}' that you want rendered can't be expressed: You can say var('created_at_date') and concatenate the strings in your macro itself, but {{ ... }} won't be rendered.

To fix this, dbt should render each test argument using jinja's "native environment" (probably wrapped around the jinja sandbox environment). The native environment basically does jinja things, except it doesn't wrap the output in quotes if the result is a real object.

Then dbt should use the test name to look up the test macro and call that with the native-rendered arguments. We'll want to do this both at parse time and at runtime in order to collect all the refs, and the two envs (native env rendering the arguments and sandbox env rendering the call) will probably have to share a context.

Fortunately for us, parsed schema tests already carry around a test_metadata object, so that aspect is taken care of.

Describe alternatives you've considered

  • We can just keep on keepin' on!
  • We can render every argument as a string (breaks lots of tests that make use of the fact that model=ref(...) is a Relation).
  • We can render everything in the native environment (I think you'll need to use {{ this | string }} and {{ ref('model') | string }} and such everywhere. Please no!

Additional context

N/A

Who will this benefit?

Users who have expectations about dbt rendering behavior.

@beckjake beckjake added enhancement New feature or request triage labels Feb 20, 2020
@beckjake beckjake added this to the Octavius Catto milestone Feb 20, 2020
@drewbanin drewbanin removed the triage label Feb 27, 2020
beckjake added a commit that referenced this issue Mar 25, 2020
…-pass-rendered

In schema tests, pass rendered items (#2149)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants