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 in 1.7.2rc2 - WITH inside custom tests #470

Closed
matsonj opened this issue Feb 1, 2024 · 6 comments
Closed

Regression in 1.7.2rc2 - WITH inside custom tests #470

matsonj opened this issue Feb 1, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@matsonj
Copy link

matsonj commented Feb 1, 2024

When invoking custom tests in the previous version, the generated SQL looks like this:

select
      
      count(*) as failures,
      case when count(*) != 0
        then 'true' else 'false' end as should_warn,
      case when count(*) != 0
        then 'true' else 'false' end as should_error
    from (
      
      select *
      from "db_name"."dbo_dbt_test__audit"."custom_test_name"
  
    ) dbt_internal_test
    

however, in 1.7.2rc2, it generates SQL like this:

select
      
      count(*) as failures,
      case when count(*) != 0
        then 'true' else 'false' end as should_warn,
      case when count(*) != 0
        then 'true' else 'false' end as should_error
    from (
      
      <test_definition.sql>
  
    ) dbt_internal_test
    

Since it is dropping the raw SQL query into the executed SQL query, this breaks any custom tests that use CTEs.

Workaround: Don't use CTEs in custom tests.

@schlich schlich added the bug Something isn't working label Feb 15, 2024
@pl-pr
Copy link

pl-pr commented Feb 15, 2024

@matsonj
If you want to use CTEs in custom tests, start your code with WITH. Then it works correctly. Perhaps you have a comment in SQL format /*.... */ or --...., then change it to the Jinja format {#.... #}.

https://github.com/microsoft/dbt-fabric/blob/main/dbt/include/fabric/macros/materializations/tests/helpers.sql#L3

@matsonj
Copy link
Author

matsonj commented Feb 15, 2024

The custom test appropriately uses as CTE. The problem is in 1.4 (previous version), the test was first added as a view and then executed. In 1.7, the test is tested directly with a subquery, which means you cannot use CTEs inside of custom tests.

@matsonj
Copy link
Author

matsonj commented Feb 15, 2024

@pl-pr I will provide more notes here shortly. Thanks for the feedback

This was referenced Feb 15, 2024
@G14rb
Copy link

G14rb commented May 15, 2024

@matsonj unfortunately it's because of the dependency to dbt-fabric. This, like many other errors, is fixed in the version 1.8.0
microsoft/dbt-fabric@57f2aa6
Well, there is another error related to the version 1.8.0
microsoft/dbt-fabric#168
And also I found that if you have comments at the beginning of the sql it won't work since it checks if the sql starts with 'with'
main_sql.strip().lower().startswith('with')

@cody-scott
Copy link
Collaborator

I've pushed a changed to swap it to using a regex search instead of the with statement. If you'd like to test on your models please feel free.
#518

@cody-scott
Copy link
Collaborator

Should be closed by #518

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

5 participants