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

[Bug] snowflake__create_table_as contract enforcing incorrect sql code #900

Open
2 tasks done
tphillip33 opened this issue Feb 6, 2024 · 3 comments
Open
2 tasks done
Labels
bug Something isn't working model_contracts support

Comments

@tphillip33
Copy link

tphillip33 commented Feb 6, 2024

Is this a new bug in dbt-snowflake?

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

Current Behavior

I have created a custom materialization which is modifying the compiled_code and removing columns for final output, then passing the modified code to create_table_as. This is erroring on the fields removed in the contracts.

  {%- set exclude_columns = config.get('exclude_columns', default=['METADATA$ACTION','METADATA$ISUPDATE','METADATA$ROW_ID']) -%}

  {%- set tmp_columns = adapter.get_columns_in_relation(tmp_relation_src) -%}
  {%- set collist = get_merge_update_columns(none, exclude_columns, tmp_columns) -%}
  {%- set tmp_relation_query =  'SELECT \n' ~ collist | join(',\n') ~ '\n FROM ' ~ tmp_relation_src -%}
...
    {%- call statement('main', language=language) -%}
      {{ create_table_as(False, target_relation, tmp_relation_query, language) }}
    {%- endcall -%} 

{% macro snowflake__create_table_as(temporary, relation, compiled_code, language='sql') -%}
is enforcing the contract on a variable "sql", which is not defined instead of passed variable named "compiled_code".
{{ get_assert_columns_equivalent(sql) }}
should be
{{ get_assert_columns_equivalent(compiled_code) }}

Expected Behavior

Contracts should be enforced on the code passed to the create_table_as() macro.

Steps To Reproduce

Create a custom materialization which modifies the compiled_code.
Create a contract on the table, excluding the columns which are listed in "exclude_columns".
Run materialization

Relevant log output

This model has an enforced contract that failed.
  Please ensure the name, data_type, and number of columns in your contract match the columns in your model's definition.   

  | column_name       | definition_type | contract_type | mismatch_reason     |
  | ----------------- | --------------- | ------------- | ------------------- |
  | METADATA$ACTION   | TEXT            |               | missing in contract |
  | METADATA$ISUPDATE | BOOLEAN         |               | missing in contract |

Environment

- OS:Windows
- Python: 3.9.9
- dbt-core: 1.7.4
- dbt-snowflake: 1.7.1

Additional Context

No response

@tphillip33 tphillip33 added bug Something isn't working triage labels Feb 6, 2024
@tphillip33
Copy link
Author

I should say, I have already overridden the code with my own version and fixed my current issue by changing the "compiled_code" variable to "sql", the same as other instances of the code.

{% macro snowflake__create_table_as(temporary, relation, sql, language='sql') -%}
...
        (
          {%- if cluster_by_string is not none -%}
            select * from (
              {{ sql }}
              ) order by ({{ cluster_by_string }})
          {%- else -%}
            {{ sql }}
          {%- endif %}
        );
 ...
     {{ py_write_table(compiled_code=sql, target_relation=relation, table_type=table_type) }}

@Fleid Fleid added support and removed triage labels Feb 22, 2024
@Fleid
Copy link
Contributor

Fleid commented Feb 23, 2024

@MichelleArk does the behavior (and resolution via using sql instead of compiled_code) matches your expectations?
If yes we should close this then.

@MichelleArk
Copy link
Contributor

Modifying sql directly makes sense as a workaround given the contracts are actually enforced on sql. sql should be the same value as compiled_code: https://github.com/dbt-labs/dbt-core/blob/main/core/dbt/context/providers.py#L1511-L1516, but they are probably passed by value in the jinja context and so modifying compiled_code does not propagate to the sql value.

It does make sense to me that the {{ get_assert_columns_equivalent(sql) }} call should be refactored to be {{ get_assert_columns_equivalent(compiled_code }} though and we could keep this open as tech debt instead of a bug.

Eventually fixing this would very likely break the workaround you've implemented though @tphillip33 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working model_contracts support
Projects
None yet
Development

No branches or pull requests

4 participants