dbt-msft / dbt-sqlserver

dbt adapter for SQL Server and Azure SQL
MIT License
212 stars 99 forks source link

Regression in 1.7.2rc2 - WITH inside custom tests #470

Closed matsonj closed 1 month ago

matsonj commented 9 months ago

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.

pl-pr commented 8 months ago

@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 commented 8 months ago

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 commented 8 months ago

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

G14rb commented 5 months ago

@matsonj unfortunately it's because of the dependency to dbt-fabric. This, like many other errors, is fixed in the version 1.8.0 https://github.com/microsoft/dbt-fabric/commit/57f2aa60ef9f60f5bf2a065fd508c5c3335b6ea1 Well, there is another error related to the version 1.8.0 https://github.com/microsoft/dbt-fabric/issues/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 commented 2 months ago

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. https://github.com/dbt-msft/dbt-sqlserver/pull/518

cody-scott commented 1 month ago

Should be closed by https://github.com/dbt-msft/dbt-sqlserver/pull/518