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

model test compilation context does not have the var method in scope #2114

Closed
1 of 5 tasks
amaruf-pagantis opened this issue Feb 10, 2020 · 4 comments · Fixed by #2150
Closed
1 of 5 tasks

model test compilation context does not have the var method in scope #2114

amaruf-pagantis opened this issue Feb 10, 2020 · 4 comments · Fixed by #2150
Labels
bug Something isn't working

Comments

@amaruf-pagantis
Copy link

Describe the bug

The model test compilation context does not have the var method in scope, but this does work correctly in sources.
In the following example, I have 2 yml files under model/staging/user.

  1. source_models.yml
  2. models.yml
model/staging/user:
|
|-  source_models.yml:
|     - schema: my_source_schema
|     - name: user_properties
|       columns:
|         - name: user_id
|           tests:
|             - relationships_where:
|                 to: source('user', 'all_users')
|                 field: user_id
|                 from_condition: created_at::date <= '{{ var('created_at__date') }}'
|
|
|-  models.yml:
|     - name: staging_user_properties
|       columns:
|         - name: user_id
|           tests:
|             - relationships_where:
|                 to: ref('stg_users')
|                 field: user_id
|                 from_condition: created_at::date <= '{{ var('created_at__date') }}'
|
|
|- stg_users.sql

If I compile the project with dbt compile --vars '{"created_at__date": "2020-01-01"}' I get:

Compilation Error in test relationships_where_staging_user_properties_user_id__user_id__created_at__date_var_created_at__date___ref_stg_users_ (models/staging/user/user_models.yml)
    'var' is undefined`

var is not available in models.yml but it's available in source_models.yml

var should have same scope in both.

Steps To Reproduce

  1. create a source schema test with a dynamic variable as parameter (in the above example, it's created_at__date)
  2. create a model test with the same dynamic variable

compile the project with --vars {"key": "value"}

Compilation will fail for model test with the error:
'var' is undefined

Expected behavior

It should compile successfully for both source and model tests with the given parameters

sources and models (and other resource types) should all have access to the var method

Screenshots and log output

If applicable, add screenshots or log output to help explain your problem.

System information

Which database are you using dbt with?

  • postgres
  • redshift
  • bigquery
  • snowflake
  • other (specify: ____________)

The output of dbt --version:

installed version: 0.13.1
   latest version: 0.15.2

The operating system you're using:
macOS Mojave
Version: 10.14.6

The output of python --version:
Python 3.7.0

Additional context

https://getdbt.slack.com/archives/C2JRRQDTL/p1581354620304600

@amaruf-pagantis amaruf-pagantis added bug Something isn't working triage labels Feb 10, 2020
@drewbanin drewbanin removed the triage label Feb 11, 2020
@drewbanin drewbanin added this to the Barbara Gittings milestone Feb 11, 2020
@cmcarthur
Copy link
Member

this might be fixed in 0.16 -- @beckjake to check

@beckjake
Copy link
Contributor

beckjake commented Feb 18, 2020

Not fixed in 0.16!

I'm not sure of the exact cause yet, but there is some very fishy rendering code in dbt.compilation.compile_node that generates what I'd consider a bad-looking context. I'm also not sure why, but source test args appear to be eagerly evaluated while model test args are lazily evaluated - this probably comes down to parser things and which compile_node we call?

@drewbanin - what's the desired behavior here? I kind of think args should be rendered at compile time for both sources and models, in the same context that will render the test itself (otherwise, how would ref work?).

Alternatively, we could fix how tests work, removing the whole idea of the wrapped_sql field. I would be pretty interested in that, but it's not a small project.

@beckjake
Copy link
Contributor

Phew, so this one's fun!
What happens on schemas is that they run through ConfigRenderer.render_schema_source, which models don't do. Ultimately this renders the test block of a source declaration using the target context, but not the test block of a model.

I think for parity and sanity, we should make the following changes:

  • config rendering should skip test blocks when it's rendering sources
  • arguments should not be rendered until runtime
  • at runtime, arguments should be rendered in the same context as the test itself

That will allow ref to work, which seems... mostly right. I think it'll be complicated.

@beckjake
Copy link
Contributor

This problem is a bit more complicated, and has some design/behavior implications.

The reason this works for sources is because dbt renders the field during source parsing (it renders everything except descriptions), and var is in scope.

Models don't do that, so this doesn't work.

However, on model tests (and source tests!) we have an explicit is_function check where we look at a test kwarg and decide if it's probably a function. If it is, we render the kwarg as {key}={value}. If it isn't, we render the kwarg as {key}={repr(value)}.

Because the strings here don't match the regex (and even have curly braces), the test parsing logic passes 'created_at::date <= '{{ var('created_at__date') }}' along as a function argument:

test_relationships_where(
  to=source('user', 'all_users'),
  field="user_id",
  from_condition="created_at::date <= '{{ var('created_at__date') }}'"
)

and the from_condition kwarg is never rendered.

I still think the thing that's actually wrong here is sources being rendered. Unfortunately, with the way dbt is currently designed, you aren't really supposed to be able to do this - the from_condition would have to be only var('created_at_date') and you'd have to write a new test macro to do the created_at::date <= interpolation, or something. I'm not sure that's how things ought to be, we can change that now if we like.

So the important questions that need to be answered before we proceed are:

  • should test args be rendered?
  • If args should be rendered, in what context?
  • if args should be rendered, what happens to the existing source(...) or var(...) syntax? Is it still supported?

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.

4 participants