Skip to content

Generate checks for declared primary keys - #1404

Merged
jschoedl merged 2 commits into
datacontract:mainfrom
DMZ22:primary-key-checks
Jul 27, 2026
Merged

Generate checks for declared primary keys#1404
jschoedl merged 2 commits into
datacontract:mainfrom
DMZ22:primary-key-checks

Conversation

@DMZ22

@DMZ22 DMZ22 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Closes #1220.

create_checks.py handled required and unique but had no primaryKey branch, so a declared primary key generated nothing. On a CSV whose order_id is both duplicated and null:

before:  🟢 data contract is valid. Run 4 checks.
after:   🔴 data contract is invalid, found the following errors:
         1) order_id Check that primary key field order_id has no missing values:
            Actual missing_count(order_id) was 1, expected = 0
         2) order_id Check that primary key field order_id has no duplicate values:
            Actual duplicate_count(order_id) was 1, expected = 0

Composite keys

This is the part that makes it more than a copy of the unique branch. Emitting _duplicate_count_check per key column would require each member of a composite key to be individually unique, which is wrong and would fail nearly every composite-PK contract. So a composite key gets one model-level check over the tuple, using the multi-column DUPLICATE_COUNT support that CheckSpec.columns and ibis_check_execute already have, ordered by primaryKeyPosition:

# key (order_id, line_no); order_id repeats but each tuple is unique
A1,1 / A1,2 / A2,1   ->  🟢 valid. Run 6 checks.

# same contract, duplicate tuple
A1,1 / A1,1 / A2,1   ->  🔴 Check that primary key (order_id, line_no) has no
                            duplicate values: Actual duplicate_count(items) was 1

De-duplication

Contracts routinely declare required, unique and primaryKey on the same column — tests/fixtures/schema-evolution/odcs-datacontract-cities-version-1.yaml does exactly that on city_id. The primary-key checks are skipped where required or unique has already emitted an equivalent one, so those contracts see no new rows and no near-duplicate output. Keys stay distinct (field_primary_key_required / field_primary_key_unique / <model>__primary_key_unique) since _collect_failed_samples resolves checks by key.

Two things for you to rule on

  1. Check naming. I used field_primary_key_required / field_primary_key_unique for single columns and primary_key_unique for the composite, mirroring field_required / field_unique. Easy to rename if you'd prefer something else.
  2. This is a behaviour change, not only a bug fix — contracts that pass today will start failing if their data violates a declared primary key. That is the point of the issue, but it will surface in existing pipelines on upgrade, so it may deserve a note in the release notes beyond the changelog line.

Verification

  • Full suite is unchanged: 30 failed / 948 passed / 41 errors both before and after (the failures and collection errors are optional backends — Trino, SQL Server, S3 — that need extras I don't have locally).

  • tests/test_test_schema_evolution.py, test_create_checks_physical_type.py, test_create_checks_nested_type.py, test_data_contract_checks.py: 97 passed.

  • New tests/test_create_checks_primary_key.py covers single keys, composite tuples, physicalName mapping, position ordering, and the de-duplication guard. Reverting only create_checks.py while keeping the tests fails 3 of the 5; the other 2 are over-generation guards and pass either way by design.

  • End-to-end runs above were real datacontract test invocations against local CSV through duckdb, not unit fixtures.

  • Tests pass (uv run pytest)

  • Code formatted (uv run ruff check --fix && uv run ruff format)

  • Docs updated (if relevant) — no user-facing doc lists generated checks

  • CHANGELOG.md entry added

create_checks handled `required` and `unique` but ignored `primaryKey`, so
a contract could declare a primary key and `datacontract test` would report
the data valid while the column contained both duplicates and nulls.

A primary key now produces a not-null check per key column plus a
uniqueness check. A composite key is unique as a tuple rather than column
by column, so its members are checked together in a single model-level
check using the existing multi-column DUPLICATE_COUNT support, ordered by
primaryKeyPosition.

Columns that also declare `required` or `unique` keep only the existing
check, so declaring all three does not emit near-duplicate rows.
@jschoedl

Copy link
Copy Markdown
Collaborator

Thank you for the PR, LGTM! I think the Changelog entry is fine - you're right that this change will make some currently-passing contracts fail, but only in cases where this should be the expected behavior anyways.

@jschoedl
jschoedl merged commit e593297 into datacontract:main Jul 27, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PrimaryKey is not auto-checked

2 participants