[CT-3372] [Bug] Syntax error in `is_type` macro within test suite (only when test fails)

[benc-db]

Is this a new bug in dbt-core?

• [X] I believe this is a new bug in dbt-core
• [X] I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

So I'm trying to adopt missing functional adapter tests. When I got to column_types, once I used model_sql that was compatible with my adapter (Databricks), I failed the test due to a syntax error in the macro:

      select 'int_col' as bad_column
      union all

      select 'numeric_col' as bad_column
      union all

      select 'string_col' as bad_column

      select * from (select 1 limit 0) as nothing
------^^^

Looking at the macro definition, I think this hasn't been an issue because there is no syntax error if there are no failures. Here's the offending code:

...
    {% do log('bad columns: ' ~ bad_columns, info=True) %}
    {% for bad_column in bad_columns %}
      select '{{ bad_column }}' as bad_column
      {{ 'union all' if not loop.last }}
    {% endfor %}
      select * from (select 1 limit 0) as nothing

Expected Behavior

Failure of the test due to the dbt tests failing (as opposed to syntax failure). In my project, I have replaced the code above with:

    {% if bad_columns %}
        {% do log('bad columns: ' ~ bad_columns, info=True) %}
        {% for bad_column in bad_columns %}
        select '{{ bad_column }}' as bad_column
        {{ 'union all' if not loop.last }}
        {% endfor %}
    {% else %}
      select * from (select 1 limit 0) as nothing
    {% endif %}

Steps To Reproduce

Tweak the definition of model_sql so that the test would fail. See that the failure in the logs is due to syntax error.

Relevant log output

No response

Environment

- OS:
- Python:
- dbt:

Which database adapter are you using with dbt?

other (mention it in "Additional Context")

Additional Context

No response

[dbeatty10]

Thanks for reaching out @benc-db !

So basically are you suggesting that we replace this:

https://github.com/dbt-labs/dbt-core/blob/e24f9b3da722328e24ee10a06fa2e8d2ca73b3e6/tests/adapter/dbt/tests/adapter/column_types/fixtures.py#L75-L80

with this instead?

    {% if bad_columns %}
        {% do log('bad columns: ' ~ bad_columns, info=True) %}
        {% for bad_column in bad_columns %}
        select '{{ bad_column }}' as bad_column
        {{ 'union all' if not loop.last }}
        {% endfor %}
    {% else %}
      select * from (select 1 limit 0) as nothing
    {% endif %}

If so, would you be interested in raising a PR, by any chance?

[benc-db]

After I posted this, I looked at what dbt-spark has for a very similar case, and they have a different macro that might be better: https://github.com/dbt-labs/dbt-spark/blob/main/tests/functional/adapter/seed_column_types/fixtures.py

[dbeatty10]

So are you thinking replacing this in dbt-core with this from dbt-spark instead?

[benc-db]

I haven't validated it personally, but yes, that is my proposal. I'm not sure if there is anything spark specific in the macro, as I don't have an environment to validate against your typical use case (Postgres I think?)

[dbeatty10]

@benc-db Makes sense to me 👍

We can validate during implementation whether or not there is anything spark-specific within this macro from dbt-spark.

Acceptance criteria

Within dbt-core:

• If the model_sql or column_map is tweaked here in order to make the pytest fail, then the pytest failure output should show a failing dbt test rather than a SQL syntax failure.

Optional bonus criteria

Within dbt-spark:

• This code can inherit some (or all) of the definitions from dbt-core rather than needing an adapter-specific override