dbt-snowflake
dbt-snowflake copied to clipboard
[Feature] Support pre-flight checks for dbt model contacts on dynamic tables
Is this a new bug in dbt-snowflake?
- [X] I believe this is a new bug in dbt-snowflake
- [X] I have searched the existing issues, and I could not find an existing issue for this bug
Current Behavior
When I deploy models materialized as table or view - the contract is verified and in case of discrepancy I receive the expected error
Compilation Error in model your_model (models\your_model_v1.sql) 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.
When I deploy models materialized as dynamic_table - the contract verification does not seem to happen, so can I release things that have not been vetted and can violate the contract.
This behavior is completely unexpected and when unnoticed will have nasty consequences discovered only on target environment (as was my case).
Expected Behavior
The models deployed as dynamic_tables should have the contract enforced, similarly to other materializations.
At minimum - there should be some warning emitted that this is not the case for dynamic tables
Steps To Reproduce
- take a simple model with enforced contract
- break the contract, eg by renaming one of the columns in model
- observe that the deployment fails properly when materialization is set to 'table'
- observe that it passes without error when materialization is set to 'dynamic_table' no matter if you use --full-refresh or not
Relevant log output
No response
Environment
- OS:Windows 10
- Python:3.10.5
- dbt-core: 1.7.4
- dbt-snowflake: 1.7.1
Additional Context
No response
Dynamic tables do not currently support model contracts. Please see the docs for more information. I'll reclassify this as an enhancement in the meantime. Hopefully this is a feature that we can take on soon.
The typical flow of working with contract would be that you start working with View or Table and only then switch to Dynamic Table (especially that it's not GA in Snowflake yet), so that limitation mentioned in Contract documentation is something that I missed completely.
Ideally it could be mentioned here as well https://docs.getdbt.com/reference/resource-configs/snowflake-configs#limitations
Ideally it could be mentioned here as well https://docs.getdbt.com/reference/resource-configs/snowflake-configs#limitations
That's fair. I'll open a docs ticket for that. This ticket (888) will pertain to the actual implementation of of model contracts for DTs.
We also want to incorporate sanity testing cases into completing this issue
Closing this because we did update the docs
@amychen1776 - not sure if closing is the right thing - updating docs (saying it's not working) was a separate issue (https://github.com/dbt-labs/docs.getdbt.com/issues/4869), but ultimately the contract verification should be implemented for Dynamic Tables as well, especially that its GA on Snowflake already.
Ah you're right - I was skimming and misread the above message. Will reopen now
@awal11 I just dug into this and we had a conversation with the Snowflake team about this. Contract verification is not supported for Dynamic tables. While I tried to reproduce on snowsight, I get this error message
Which the Snowflake DT confirmed is expected. Do you have information otherwise?
@amychen1776 - hmm, now I am confused.... By contract verification I understand the dbt functionality described as:
dbt will run a "preflight" check to ensure that the model's query will return a set of columns with names and data types matching the ones you have defined.
and to my understanding this is rather an internal dbt check against metadata, similar to what already happens with views or tables, so I really do not understand how your screenshot is related to it...
Okay, @dbeatty10 dug into this and validated that we can do pre-flight checks separately from applying model constraints despite the fact that they're defined in the same place. We don't have capacity in the short term to tackle this but we will scope this out for our next quarter.
@colin-rogers-dbt a note for when we refine this - we should scope this out to support MVs on the other adapters.