dbt Constraints / model contracts #341

b-per · 2022-12-05T13:41:55Z

Linked to the dbt-core issues dbt-labs/dbt-core#6079 and dbt-labs/dbt-core#6079

These changes, along with the dbt-Core ones, allow defining column types and constraints on columns in Snowflake.

Description

Supports setting the 4 Snowflake constraints described in the docs
- unique, not null, primary key and foreign key
- ⚠️ It is important to note that only the not null (and the not null property of primary key) are actually checked today in Snowflake. There rest of the constraints are purely metadata ones, not verified when inserting data
While the dbt Core implementation supports checks, I don't think that those are supported in Snowflake today
With this implementation:
- all the columns need to be listed in the YML file, in the correct order, and each of them require a correct data type
- in case that the data type is missing or incorrect, the CTA fails

Related Core/Adapter Pull Requests

Must be reviewed with passing tests

Example of behavior

The following jaffle_shop example

models:
  - name: customers
    description: This table has basic information about a customer, as well as some derived facts based on a customer's orders
    config:
      constraints_enabled: true

    columns:
      - name: customer_id
        description: This is a unique identifier for a customer
        data_type: number(38,0)
        constraint: 
          - unique
          - not null
        tests:
          - unique
          - not_null

      - name: first_name
        data_type: varchar(100)
        description: customer's first name. pii.

      - name: last_name
        data_type: varchar(5)
        description: customer's last name. pii.

      - name: first_order
        description: date (utc) of a customer's first order
        data_type: date

      - name: most_recent_order
        description: date (utc) of a customer's most recent order
        data_type: date

      - name: number_of_orders
        description: count of the number of orders a customer has placed
        data_type: number(18,0)

      - name: total_order_amount
        description: total value (aud) of a customer's orders
        data_type: number(18,0)

generates the following SQL:

        create or replace transient table <DB>.<SCHEMA>.customers 
        
  
  (
    
      customer_id number(38,0)  unique  not null   ,
      first_name varchar(100)   ,
      last_name varchar(5)   ,
      first_order date   ,
      most_recent_order date   ,
      number_of_orders number(18,0)   ,
      total_order_amount number(18,0)   
  )
  

         as
        (with customers as (

    select * from AD_HOC.dbt_bperigaud.stg_customers

),
...

Checklist

I have read the contributing guide and understand what's expected of me
I have signed the CLA
I have run this code in development and it appears to resolve the stated issue
This PR includes tests, or tests are not required/relevant for this PR
I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR
I have run changie new to create a changelog entry

github-actions · 2022-12-05T13:42:18Z

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the dbt-snowflake contributing guide.

sungchun12 · 2022-12-05T15:43:37Z

I should have communicated better that my PR #6271 actually uses the below config.

version: 2

models:
  - name: constraints_example
    docs:
      node_color: black
    config:
      constraints_enabled: true
    columns:
      - name: id
        data_type: integer
        description: hello
        constraints: ['not null','primary key']
        check: id > 0
        tests:
          - unique
      - name: color
        data_type: string
      - name: date_day
        data_type: date

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql

sungchun12 · 2022-12-05T15:56:15Z

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql

+      {% set constraints = col['constraint'] -%}
+      {%- set checks = col['checks'] -%}
+      {%- if checks -%}
+        {{exceptions.warn("We noticed you have `checks` in your configs, these are NOT compatible with Snowflake and will be ignored")}}


Can you show me a screenshot of the warning message working in your terminal? I couldn't get it to pop up on mine?

I got it to work by copying your code to my branch. I'll figure out the pip installs later! Looks good @jtcohen6 what's your opinion on how the warning should look and where in the logs it should appear?

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql

sungchun12 · 2022-12-16T19:43:30Z

@joshuataylor keep your eyes on this when it's ready for local testing on your machine!

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql

sungchun12 · 2022-12-19T17:44:30Z

@joshuataylor try to test this out now!

sungchun12 · 2022-12-19T17:47:41Z

tests/functional/constraints/test_constraints.py

+"""
+
+
+class TestMaterializedWithConstraints:


Add a rollback to previous table test based on this: https://github.com/dbt-labs/dbt-core/pull/6271/files#diff-ace143195a2ed95e724979d70d36ca44a4177f0876f768795225994d87142da8R139

Add a DDL actual == expected test based on this: https://github.com/dbt-labs/dbt-core/pull/6271/files#diff-ace143195a2ed95e724979d70d36ca44a4177f0876f768795225994d87142da8R119

Do we really want the DDL test? I feel like any other change to create_table_as() would then require updating the tests for something unrelated.

I actually wanted to avoid a test checking the DDL.

Based on an internal slack thread, you don't need the DDL test, but the rollback test remains to be needed

Sure, I can create a rollback test. Because this PR leverages Snowflake's ability to set constraints as part of the CTAS statement I think that this use case is already covered by the existing CTAS tests but it is not too difficult for me to add another one here.

Thanks looks great!

joshuataylor · 2022-12-24T05:58:20Z

@joshuataylor try to test this out now!

Sorry I've had family issues over the past week or so, so haven't had a chance to check this out yet.

I'm planning on checking this out over the next few days, but obviously with being the holiday season I won't expect an immediate reply on feedback 🚀

joshuataylor · 2022-12-27T03:43:33Z

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql

+    {%- endfor %}
+  )
+    {%- if ns.at_least_one_check -%}
+      {{exceptions.warn("We noticed you have `check` configs, these are NOT compatible with Snowflake and will be ignored")}}


Maybe link to https://docs.snowflake.com/en/sql-reference/constraints-overview.html ?

Maybe the wording can be similar to what they have?

Snowflake supports defining and maintaining constraints, but does not enforce them, except for NOT NULL constraints, which are always enforced.

joshuataylor · 2022-12-27T03:45:44Z

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql

@@ -0,0 +1,18 @@
+{% macro snowflake__get_columns_spec_ddl() %}
+  {# loop through user_provided_columns to create DDL with data types and constraints #}
+  {% if config.get('constraints_enabled', False) %}


This looks good, nice work!

Move this if statement to adapters.sql similar to this: https://github.com/dbt-labs/dbt-core/pull/6271/files#diff-7246f19f4d12042d172ac61b3d0c4564ea27dd8825870b35425ede1ea41625dbR28

joshuataylor · 2022-12-27T03:47:54Z

This PR brings me such joy 😍.

2023 is going to be the year we stop having to cast everything in SQL (so it doesn't randomly change 😬 ) and it'll work on a YAML level. 🎉🎉🎉

sungchun12 · 2023-01-05T16:52:59Z

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql

+    {% for i in user_provided_columns -%}
+      {%- set col = user_provided_columns[i] -%}
+      {% set constraints = col['constraints'] -%}
+      {%- set ns.at_least_one_check = ns.at_least_one_check or col['check'] %}


Suggested change

{%- set ns.at_least_one_check = ns.at_least_one_check or col['check'] %}

{%- set ns.at_least_one_check = ns.at_least_one_check or col['constraints_check'] %}

Do a find and replace on all check configs in all files to be constraints_check by name.

dbt/include/snowflake/macros/adapters.sql

b-per · 2023-01-17T12:10:20Z

Tests are failing due to Snowflake columns being internally stored in uppercase.
I raise a comment on the dbt-core PR to know if we handle this on the Core side or adapter side.

Co-authored-by: Sung Won Chung <sungwonchung3@gmail.com>

Fleid · 2023-02-01T23:27:12Z

I know it's not tech_debt but I'm temporarily using the label to sort through the backlog. Don't get offended please ;)

sungchun12 · 2023-02-16T19:09:48Z

@joshuataylor It's official. You can give this a rougher test drive now :)

cla-bot bot added the cla:yes label Dec 5, 2022

sungchun12 self-requested a review December 5, 2022 15:25

sungchun12 reviewed Dec 5, 2022

View reviewed changes

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql Outdated Show resolved Hide resolved

sungchun12 reviewed Dec 5, 2022

View reviewed changes

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql Outdated Show resolved Hide resolved

sungchun12 reviewed Dec 5, 2022

View reviewed changes

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql Outdated Show resolved Hide resolved

sungchun12 reviewed Dec 5, 2022

View reviewed changes

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql Outdated Show resolved Hide resolved

This was referenced Dec 5, 2022

dbt Constraints / model contracts dbt-labs/dbt-core#6271

Merged

[CT-1613] Update redshift adapter to use constraints and checks dbt-labs/dbt-redshift#227

Closed

sungchun12 reviewed Dec 19, 2022

View reviewed changes

dbt/include/snowflake/macros/utils/get_columns_spec_ddl.sql Outdated Show resolved Hide resolved

b-per marked this pull request as ready for review December 19, 2022 17:35

sungchun12 reviewed Dec 19, 2022

View reviewed changes

b-per linked an issue Dec 20, 2022 that may be closed by this pull request

[CT-1676] [Feature] Support constraints in Snowflake following the dbt Core implementation #365

Closed

3 tasks

This was referenced Dec 20, 2022

dbt Constraints / model contracts dbt-labs/dbt-redshift#229

Merged

dbt Constraints / model contracts dbt-labs/dbt-bigquery#426

Merged

dbeatty10 mentioned this pull request Dec 22, 2022

dbt Constraints / model contracts dbt-labs/dbt-spark#574

Merged

6 tasks

joshuataylor reviewed Dec 27, 2022

View reviewed changes

sungchun12 assigned b-per Dec 27, 2022

sungchun12 reviewed Jan 5, 2023

View reviewed changes

sungchun12 reviewed Jan 13, 2023

View reviewed changes

dbt/include/snowflake/macros/adapters.sql Outdated Show resolved Hide resolved

MichelleArk mentioned this pull request Jan 26, 2023

[CT-1915] [Epic] Multi-project collaboration - Milestone 1 dbt-labs/dbt-core#6747

Closed

b-per added 2 commits February 1, 2023 10:56

Add support for the constraint config in Snowflake

b41b6f0

Add warning for checks

0bf52c6

b-per and others added 11 commits February 1, 2023 10:56

Action PR feedback

4932c70

Report one exception in case of multiple check

02fb11b

Add tests for constraints

2687b77

Add changie entry

7b3b2b0

Force dependency on specific core branch for CI

33f27a3

Add test for rollback after failed constraint

91d55ee

Fix test

a27065a

Rename check to constraints_check

0d2887c

Check columns and column order between SQL and YML

0200d94

Co-authored-by: Sung Won Chung <sungwonchung3@gmail.com>

Move constraints_enabled check to create_table_as

d14cf79

Switch to new tests

f63c639

jtcohen6 force-pushed the dbt-constraints-snowflake branch from bd8ca0d to f63c639 Compare February 1, 2023 09:56

jtcohen6 mentioned this pull request Feb 1, 2023

[CT-1918] [Discussion] Unify constraints and constraints_check configs dbt-labs/dbt-core#6750

Closed

Light cleanup

40942e6

Fleid added the tech_debt label Feb 1, 2023

Small cleanup

384a203

dbeatty10 changed the title ~~Add support for the constraint config in Snowflake~~ dbt Constraints / model contracts Feb 14, 2023

jtcohen6 and others added 2 commits February 15, 2023 16:55

Reset to dbt-core main

e18c60d

Merge branch 'main' into dbt-constraints-snowflake

fea6b44

MichelleArk approved these changes Feb 15, 2023

View reviewed changes

jtcohen6 approved these changes Feb 16, 2023

View reviewed changes

jtcohen6 merged commit 02c82a5 into main Feb 16, 2023

jtcohen6 deleted the dbt-constraints-snowflake branch February 16, 2023 11:01

mikealfare restored the dbt-constraints-snowflake branch February 17, 2023 01:29

mikealfare deleted the dbt-constraints-snowflake branch February 17, 2023 02:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dbt Constraints / model contracts #341

dbt Constraints / model contracts #341

b-per commented Dec 5, 2022 •

edited by dbeatty10

Loading

github-actions bot commented Dec 5, 2022

sungchun12 commented Dec 5, 2022

sungchun12 Dec 5, 2022

sungchun12 Dec 5, 2022

sungchun12 commented Dec 16, 2022

sungchun12 commented Dec 19, 2022

sungchun12 Dec 19, 2022

sungchun12 Dec 19, 2022

b-per Dec 19, 2022

sungchun12 Dec 19, 2022

b-per Dec 21, 2022

sungchun12 Dec 21, 2022

joshuataylor commented Dec 24, 2022

joshuataylor Dec 27, 2022

joshuataylor Dec 27, 2022

sungchun12 Jan 13, 2023

joshuataylor commented Dec 27, 2022 •

edited

Loading

sungchun12 Jan 5, 2023

b-per commented Jan 17, 2023

Fleid commented Feb 1, 2023

sungchun12 commented Feb 16, 2023

	{%- set ns.at_least_one_check = ns.at_least_one_check or col['check'] %}
	{%- set ns.at_least_one_check = ns.at_least_one_check or col['constraints_check'] %}

		"""


		class TestMaterializedWithConstraints:

dbt Constraints / model contracts #341

dbt Constraints / model contracts #341

Conversation

b-per commented Dec 5, 2022 • edited by dbeatty10 Loading

Description

Example of behavior

Checklist

github-actions bot commented Dec 5, 2022

sungchun12 commented Dec 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sungchun12 commented Dec 16, 2022

sungchun12 commented Dec 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuataylor commented Dec 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuataylor commented Dec 27, 2022 • edited Loading

Choose a reason for hiding this comment

b-per commented Jan 17, 2023

Fleid commented Feb 1, 2023

sungchun12 commented Feb 16, 2023

b-per commented Dec 5, 2022 •

edited by dbeatty10

Loading

joshuataylor commented Dec 27, 2022 •

edited

Loading