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

[Regression] Incorrect CTE name in unit tests using incremental mode for versioned incremental models #10763

Closed
2 tasks done
b-per opened this issue Sep 17, 2024 · 6 comments · Fixed by #10755
Closed
2 tasks done
Labels
bug Something isn't working incremental Incremental modeling with dbt model_versions regression unit tests Issues related to built-in dbt unit testing functionality

Comments

@b-per
Copy link
Contributor

b-per commented Sep 17, 2024

Is this a new bug?

  • I believe this is a new bug
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

When defining a unit test for the incremental load on a versioned incremental model, the generated SQL is invalid. The name of the CTE "injected" at the top contains the version name, when this is replaced by the CTE name, but without the version name.

e.g., with the versioned model my_model we get

with  __dbt__cte__my_model_v1 as (
...
)

select * from ...
where my_date > (select max(my_date) from __dbt__cte__my_model)

This is not valid SQL

Expected Behavior

this is replaced by the CTE name that includes the version number

Steps To Reproduce

  1. create an incremental model
  2. add a version to the model
  3. add a unit test to the model with is_incremental: true and some rows for this
  4. run the unit test

Relevant log output

Not relevant

Environment

- dbt-adapters:1.6.1

Additional Context

No response

@b-per b-per added bug Something isn't working triage labels Sep 17, 2024
@amychen1776 amychen1776 added unit tests Issues related to built-in dbt unit testing functionality and removed triage labels Sep 17, 2024
@dbeatty10 dbeatty10 self-assigned this Sep 17, 2024
@dbeatty10 dbeatty10 added incremental Incremental modeling with dbt model_versions labels Sep 17, 2024
@dbeatty10
Copy link
Contributor

dbeatty10 commented Sep 18, 2024

Reprex

Create these files:

models/my_upstream.sql

select 2 as id, 1 as event_time

models/my_model.sql

{{
    config(
        materialized='incremental'
    )
}}

select id, event_time

from {{ ref("my_upstream") }}

{% if is_incremental() %}

where event_time >= (select coalesce(max(event_time), 0) from {{ this }} )

{% endif %}

models/_properties.yml

models:
  - name: my_model
    latest_version: 1
    versions:
      - v: 1

unit_tests:
  - name: dbt_adapters_309
    model: my_model
    overrides:
      macros:
        is_incremental: false
    given:
      - input: ref("my_upstream")
        rows:
          - {id: 3, event_time: 2}
      - input: this
        rows:
          - {id: 2, event_time: 1}
    expect:
      rows:
          - {id: 3, event_time: 2}

Then run these commands:

dbt run --empty --full-refresh
dbt build --select my_model

✅ Everything should run just fine.

Then change is_incremental: false to be is_incremental: true.

Then re-run this command:

dbt build --select my_model

💥 It will fail with the following output:

�[0m18:48:05  Running with dbt=1.8.6
�[0m18:48:06  Registered adapter: duckdb=1.8.3
�[0m18:48:06  Found 1 model, 410 macros, 1 unit test
�[0m18:48:06  
�[0m18:48:06  Concurrency: 1 threads (target='dev')
�[0m18:48:06  
�[0m18:48:06  1 of 2 START unit_test my_model::dbt_adapters_309_v1 ........................... [RUN]
�[0m18:48:06  1 of 2 ERROR my_model::dbt_adapters_309_v1 ..................................... [�[31mERROR�[0m in 0.17s]
�[0m18:48:06  2 of 2 SKIP relation feature_456.my_model ...................................... [�[33mSKIP�[0m]
�[0m18:48:06  
�[0m18:48:06  Finished running 1 unit test, 1 incremental model in 0 hours 0 minutes and 0.39 seconds (0.39s).
�[0m18:48:06  
�[0m18:48:06  �[31mCompleted with 1 error and 0 warnings:�[0m
�[0m18:48:06  
�[0m18:48:06    Runtime Error in unit_test dbt_adapters_309 (models/_properties.yml)
  An error occurred during execution of unit test 'dbt_adapters_309'. There may be an error in the unit test definition: check the data types.
   Runtime Error
    Catalog Error: Table with name __dbt__cte__my_model does not exist!
    Did you mean "feature_456.my_model_v1"?
    LINE 29: ...version": "1.8.6", "profile_name": "duckdb", "target_name": "dev", "node_id": "unit_test.my_project.my_model.dbt_adapters_309_v1"} */
    
        
      
        
        
    
        create temporary table
          "dbt_adapters_309_v1__dbt_tmp20240918124806893647"
      
        as (
          select * from (
            
    
    with __dbt__cte__my_model_v1 as (
    
    -- Fixture for my_model
    select 
        
        cast(2 as INTEGER)
     as id, 
        
        cast(1 as INTEGER)
     as event_time
    ) select 2 as id, 1 as event_time
    
    
    
    where event_time >= (select coalesce(max(event_time), 0) from __dbt__cte__my_model )
                                                       ^
�[0m18:48:06  
�[0m18:48:06  Done. PASS=0 WARN=0 ERROR=1 SKIP=1 TOTAL=2

As @b-per mentioned, the difference is __dbt__cte__my_model (actual) vs. __dbt__cte__my_model_v1 (expected) in the portion of the logic that is only executed in incremental mode (like from this example).

@dbeatty10 dbeatty10 removed their assignment Sep 18, 2024
@dbeatty10 dbeatty10 changed the title [Bug] Incorrect CTE name referenced when testing versioned incremental models in incremental mode [Bug] Incorrect CTE name in unit tests using incremental mode for versioned incremental models Sep 19, 2024
@graciegoheen graciegoheen transferred this issue from dbt-labs/dbt-adapters Sep 23, 2024
@MichelleArk
Copy link
Contributor

This will likely be a fix in dbt-core, as that's where we set up what 'this' should alias. Here:

if input.strip() == "this":
original_input_node = tested_node

@MichelleArk
Copy link
Contributor

MichelleArk commented Sep 23, 2024

I think the issue is we're not using the 'latest' version on the tested_node, could perhaps need some updates in find_tested_model_node

@dbeatty10
Copy link
Contributor

@MichelleArk I wonder if it is fixed by #10755?

@dbeatty10
Copy link
Contributor

@MichelleArk I tried it out, and #10755 looks like it fixes this issue too.

@dbeatty10
Copy link
Contributor

Labeling this as a regression since this worked with dbt-core==1.8.5 dbt-adapters==1.4.0 but stopped working with dbt-core==1.8.6 dbt-adapters==1.4.1.

@dbeatty10 dbeatty10 changed the title [Bug] Incorrect CTE name in unit tests using incremental mode for versioned incremental models [Regression] Incorrect CTE name in unit tests using incremental mode for versioned incremental models Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working incremental Incremental modeling with dbt model_versions regression unit tests Issues related to built-in dbt unit testing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants