dbt-core icon indicating copy to clipboard operation
dbt-core copied to clipboard

[Constraints] Add parse-time validation that ensures only a single primary_key constraint is defined

Open MichelleArk opened this issue 1 year ago • 0 comments

Housekeeping

  • [X] I am a maintainer of dbt-core

Short description

Currently, no parse-time error is raised when dbt sees a contract that defines a primary_key at both the model-level and the column-level. Additionally, no parse-time error is raised when multiple primary_keys are defined at the column-level.

In both cases, this will lead to a database error across many (probably all) data warehouses. Validated here: https://github.com/dbt-labs/jaffle_shop/pull/106

Additionally, our documented guidance in dbt is to define tests/constraints that work across multiple columns at the model-level. If a warehouse supports defining multiple primary keys at the column-level, it is really working by coincidence from the perspective of dbt.

Acceptance criteria

We raise an error at parse-time if:

  1. a primary_key constraint is defined at both the model-level and column-level
  2. a primary_key constraint is defined on multiple columns within a model

Suggested Tests

  1. Scenario where a primary key is defined across model and column level constraints
  2. scenario where a primary key is defined on multiple columns
  3. same as (1) and (2), but for versioned models

Impact to Other Teams

N/A

Will backports be required?

No.

Context

Additionally, I believe we only parse + serialize constraints if the contract is enforced currently. So if config.contract is False, we don't even store the constraints as provided. This was probably done to avoid raising errors related to warehouse-specific errors related to constraint prerequisites at parse time. However, we should probably still be parsing the constraints (storing constraints defined in project spec onto internal node + serializing them) and running any parse-time validation against them at least.

MichelleArk avatar Feb 15 '24 19:02 MichelleArk