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

this.is_view and this.is_table not working in BigQuery inside a hook #3529

Closed
1 of 5 tasks
jmriego opened this issue Jul 1, 2021 · 3 comments
Closed
1 of 5 tasks
Labels
bug Something isn't working wontfix Not a bug or out of scope for dbt-core

Comments

@jmriego
Copy link
Contributor

jmriego commented Jul 1, 2021

Describe the bug

I'm trying to use the .is_table and .is_view methods but they don't seem to be working in BigQuery

Steps To Reproduce

Create a sample project with dbt init and modify my_second_dbt_model.sql to this:

{{
  config(
    post_hook=['ALTER {{"view" if this.is_view else "table"}} {{ this }} SET OPTIONS (description="second model...")'],
  )
}}
-- Use the `ref` function to select from other models

select *
from {{ ref('my_first_dbt_model') }}
where id = 1

Run this model and you'll get an error. Actually, both this.is_view and this.is_table return False in Jinja

Expected behavior

I would expect this to run successfully, but it doesn't render as expected.

Screenshots and log output

System information

Which database are you using dbt with?

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

The output of dbt --version:

installed version: 0.20.0-rc1
   latest version: 0.19.2

Your version of dbt is ahead of the latest release!

Plugins:
  - postgres: 0.20.0rc1
  - snowflake: 0.20.0rc1
  - redshift: 0.20.0rc1
  - bigquery: 0.20.0rc1

The operating system you're using:
Ubunt4 20.04

The output of python --version:
Python 3.8.5

Additional context

Nothing else I can think of

@jmriego jmriego added bug Something isn't working triage labels Jul 1, 2021
@jtcohen6
Copy link
Contributor

jtcohen6 commented Jul 2, 2021

@jmriego Thanks for opening!

This is a sort of known issue (mentioned in #2938 as well): {{ this }} always returns a relation with relation_type none. Technically, dbt doesn't know the relational type of the current model's corresponding relational object in the database... it just knows its materialization and representation (database/schema/identifier). Either of those things could be used to infer the type with reasonable effectiveness.

Following that, you've got two options:

  1. Use config.materialized as a proxy for what the relation type ought to be. Since it's a post-hook, it shouldn't ever be wrong:
{{
  config(
    post_hook=['ALTER {{"view" if model.config.materialized == "view" else "table"}} {{ this }} SET OPTIONS (description="second model...")'],
  )
}}
-- Use the `ref` function to select from other models

select *
from {{ ref('my_first_dbt_model') }}
where id = 1
  1. Grab the relation type from the database / relational cache, via get_relation.
-- macros/alter_this.sql

{% macro alter_this(this) %}

    {% set this_typed = adapter.get_relation(this.database, this.schema, this.name) %}

    ALTER {{"view" if this_typed.is_view else "table"}} {{ this_typed }}
        SET OPTIONS (description="second model...")

{% endmacro %}
-- models/my_model.sql
{{
  config(
      post_hook=["{{ alter_this(this) }}"],
  )
}}
-- Use the `ref` function to select from other models

select *
from {{ ref('my_first_dbt_model') }}
where id = 1

Note that the macro call must be nested in quotes + another set of curlies, in order to be re-parsed at execute time (see #2370).

The question is, should dbt use one of those approaches by default to set this.relation_type? The former approach feels much better to me than the latter (we can't know all the possible values of materialized), but it's still tricky—by only being available at execute time, and not at parse time, it doesn't support the exact formulation you're using in the original issue.

I'm going to close this particular issue for now as a wontfix. That said, it will be worth revisiting hook parsing and model/materialization context construction (including this) in the future.

@jtcohen6 jtcohen6 added wontfix Not a bug or out of scope for dbt-core and removed triage labels Jul 2, 2021
@jtcohen6 jtcohen6 closed this as completed Jul 2, 2021
@jmriego
Copy link
Contributor Author

jmriego commented Jul 2, 2021

sure @jtcohen6 that all makes sense. Thanks for the explanation and the workaround!
If you don't mind, I'm trying to grasp what you meant at:

by only being available at execute time, and not at parse time, it doesn't support the exact formulation you're using in the original issue.

does this mean that the post_hook config is parsed before the model has run and then that sql is ran at the end?

@jtcohen6
Copy link
Contributor

jtcohen6 commented Jul 2, 2021

does this mean that the post_hook config is parsed before the model has run and then that sql is ran at the end?

This is the confusing behavior described in #2370:

  • When a model is being parsed, its post_hook config is Jinja-rendered, and the result is stored. At parse time, execute is set to False, and any macros that require the adapter cache or connection (e.g. get_relation) return None.
  • When the model is being run, its post_hook config is then Jinja-rendered again, this time with execute = True and adapter-dependent macros switched on.

Because of that double-rendering, these yield different results:

post_hook=alter_this(this),
post_hook="{{ alter_this(this) }}",

In the first case, alter_this(this) is rendered at parse time (when get_relation returns None), and its string result is stored as a string inside the post_hook config:

"post-hook": [
    {
        "sql": "\n\n    \n\n    ALTER table None\n        SET OPTIONS (description=\"second model...\")\n\n",
        "transaction": true,
        "index": null
    }
],

In the second case, parse-time Jinja rendering just removes the first set of curlies, leaving the main macro call in place to be re-rendered during execution:

"post-hook": [
    {
        "sql": "{{ alter_this(this) }}",
        "transaction": true,
        "index": null
    }
],

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix Not a bug or out of scope for dbt-core
Projects
None yet
Development

No branches or pull requests

2 participants