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

[CT-572] Two tmp tables are created if insert_overwrite is the strategy and on_schema_change is True due to incorrect parameter. #171

Closed
robomill opened this issue Apr 28, 2022 · 2 comments · Fixed by #172
Labels
good_first_issue Good for newcomers type:bug Something isn't working

Comments

@robomill
Copy link
Contributor

robomill commented Apr 28, 2022

Describe the bug

If the materialisation strategy is insert_overwrite and on_schema_change != ignore then two tmp tables will be created due to an incorrect parameter passed to the bq_insert_overwrite macro. Instead of tmp_relation_exists, on_schema_change is passed to the macro, which will always be None, and causes the first if in the following control statement to always evaluate as True.

      {% if not tmp_relation_exists %}
        {{ declare_dbt_max_partition(this, partition_by, sql) }}

        -- 1. create a temp table
        {{ create_table_as(True, tmp_relation, sql) }}
      {% else %}

Steps To Reproduce

run an incremental_overwrite model with on_schema_change != 'ignore' then check the query history in BQ, you will notice two tmp_table statments, first as a seperate job and then again in the merge statement.

Expected behavior

a single tmp table should be created.

Screenshots and log output

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

System information

The output of dbt --version:

Core:
  - installed: 1.1.0-rc1
  - latest:    1.0.6     - Ahead of latest version!

Plugins:
  - bigquery: 1.1.0b1 - Ahead of latest version!

The operating system you're using:
OSX

The output of python --version:
python 3.10.2

@robomill robomill added type:bug Something isn't working triage:product labels Apr 28, 2022
@github-actions github-actions bot changed the title Two tmp tables are created if insert_overwrite is the strategy and on_schema_change is True due to incorrect parameter. [CT-572] Two tmp tables are created if insert_overwrite is the strategy and on_schema_change is True due to incorrect parameter. Apr 28, 2022
@jtcohen6 jtcohen6 added good_first_issue Good for newcomers S3 and removed triage:product labels May 3, 2022
@jtcohen6
Copy link
Contributor

jtcohen6 commented May 3, 2022

@robomill You're totally right! Thanks for the catch, and for the PR :)

It is a bit weird that on_schema_change returns None, rather than throwing an error, since it isn't really set within the scope of the bq_generate_incremental_build_sql macro. Maybe a weird effect of scoping for Jinja variables within materializations...?

@robomill
Copy link
Contributor Author

robomill commented May 4, 2022

@robomill You're totally right! Thanks for the catch, and for the PR :)

It is a bit weird that on_schema_change returns None, rather than throwing an error, since it isn't really set within the scope of the bq_generate_incremental_build_sql macro. Maybe a weird effect of scoping for Jinja variables within materializations...?

No problemo @jtcohen6!

Yup I don't really understand that either. I tried a very simple jinja snippet to test what the hell was going on and it just seems that instead of raising an error when an undefined variable is templated, it just evaluates the undefined variable as None.

If I have a template like:

{% macro test_2() -%}
    {{ test }}
{%- endmacro %}

{% macro test_1() -%}
    {% set test = "testytesty" %}
    {{ test_2() }}
{%- endmacro %}

{{ test_1() }}

and run template.render() it throws no errors and just returns white space. So it seems like unexpected default behaviour by Jinja. You can even pass a variable that doesn't get defined anywhere and no error is raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good_first_issue Good for newcomers type:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants