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

is_incremental() evaluates to True when switching from a view to an incremental model #1292

Closed
clrcrl opened this issue Feb 13, 2019 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@clrcrl
Copy link
Contributor

clrcrl commented Feb 13, 2019

Steps to reproduce

  1. Create a view model, models/my_model.sql (this SQL is on Redshift):
{{
    config(
        materialized='view'
    )
}}

select
  1 as id,
  getdate() as _dbt_updated_at
  1. dbt run:
$ dbt run -m my_model
Found 8 models, 12 tests, 0 archives, 0 analyses, 95 macros, 2 operations, 0 seed files

10:44:19 | Concurrency: 1 threads (target='dev')
10:44:19 |
10:44:19 | 1 of 1 START view model jaffle_shop_dev.my_model..................... [RUN]
10:44:19 | 1 of 1 OK created view model jaffle_shop_dev.my_model................ [CREATE VIEW in 0.41s]
10:44:19 |
10:44:19 | Finished running 1 view models in 0.99s.

Completed successfully
  1. Change the model to incremental
{{
    config(
        materialized='incremental',
        unique_key='id'
    )
}}

select
  1 as id,
  getdate() as _dbt_updated_at

{% if is_incremental() %}
    {{ log("This is an incremental run", info=True) }}
    where _dbt_updated_at >= (select max(_dbt_updated_at) from {{this}} )
{% endif %}
  1. dbt run to get this:
$ dbt run -m my_model
Found 8 models, 12 tests, 0 archives, 0 analyses, 95 macros, 2 operations, 0 seed files

10:46:03 | Concurrency: 1 threads (target='dev')
10:46:03 |
10:46:03 | 1 of 1 START incremental model jaffle_shop_dev.my_model.............. [RUN]
This is an incremental run
10:46:03 | 1 of 1 ERROR creating incremental model jaffle_shop_dev.my_model..... [ERROR in 0.21s]
10:46:03 |
10:46:03 | Finished running 1 incremental models in 0.69s.

Completed with 1 errors:

Database Error in model my_model (models/staging/my_model.sql)
  relation "jaffle_shop_dev.my_model" does not exist
  compiled SQL at target/compiled/jaffle_shop/staging/my_model.sql

Done. PASS=0 ERROR=1 SKIP=0 TOTAL=1
  1. dbt run again... and it works...:
$ dbt run -m my_model
Found 8 models, 12 tests, 0 archives, 0 analyses, 95 macros, 2 operations, 0 seed files

10:47:11 | Concurrency: 1 threads (target='dev')
10:47:11 |
10:47:11 | 1 of 1 START incremental model jaffle_shop_dev.my_model.............. [RUN]
10:47:26 | 1 of 1 OK created incremental model jaffle_shop_dev.my_model......... [SELECT in 15.46s]
10:47:26 |
10:47:26 | Finished running 1 incremental models in 16.08s.

Completed successfully

Done. PASS=1 ERROR=0 SKIP=0 TOTAL=1

I have the logs for this as well (I can't attach text files here, and I'm not keen to paste 680 lines of output in this issue).

System info

$ dbt debug
dbt version: 0.12.2
python version: 3.7.2
@drewbanin
Copy link
Contributor

Thanks @clrcrl - can you DM me the logs? I can check it out

@clrcrl clrcrl changed the title is_incremental() evaluates to True when switching from a view to a table is_incremental() evaluates to True when switching from a view to an incremental model Feb 13, 2019
@drewbanin
Copy link
Contributor

Ok - this is just happening because is_incremental() checks that the relation exists, but does not confirm that the type of the relation is a table.

dbt is smart enough to avoid inserting into the view (it drops the view first), but then is_incremental() does indeed evaluate to true and the incremental filter gets included in the model sql. The problem is that is_incremental() is evaluated before dbt gets to the materialization, so is_incremental() returns true in response to the existing view, which is then dropped immediately in the materialization.

The fix here is probably to make is_incremental() check that the target relation is of type table. That might look like:

{% macro is_incremental() %}
    {#-- do not run introspective queries in parsing #}
    {% if not execute %}
        {{ return(False) }}
    {% else %}
        {% set relation = adapter.get_relation(this.database, this.schema, this.table) %}
        {{ return(relation is not none and relation.type == 'table' and not flags.FULL_REFRESH) }}
    {% endif %}
{% endmacro %}

@drewbanin drewbanin added this to the Stephen Girard milestone Feb 13, 2019
@drewbanin drewbanin added the bug Something isn't working label Feb 13, 2019
@drewbanin drewbanin self-assigned this Feb 13, 2019
drewbanin added a commit that referenced this issue Feb 13, 2019
If the target relation is a non-table (probably a view) then dbt
should return False from the is_incremental() macro. The
materialization will drop this relation before running the model
code as a `create table as` statement, so the incremental filter
would likely be invalid.
@drewbanin
Copy link
Contributor

Thanks for the report @clrcrl - fixed in #1293 and going out in 0.13.0

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

No branches or pull requests

2 participants