Skip to content

fix: Primary key declaration#648

Open
vrkirillov wants to merge 12 commits into
ClickHouse:mainfrom
vrkirillov:primary-key
Open

fix: Primary key declaration#648
vrkirillov wants to merge 12 commits into
ClickHouse:mainfrom
vrkirillov:primary-key

Conversation

@vrkirillov

@vrkirillov vrkirillov commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Added support for using PRIMARY KEY in MergeTree family table definitions in place of ORDER BY
  • Added support lists of columns in PRIMARY KEY declarations

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG

Note

Medium Risk
Changes default DDL for MergeTree tables without order_by (PK-only vs ORDER BY (tuple())), which can affect physical layout and query performance for existing models.

Overview
Adds MergeTree primary_key model config (string or column list) so DDL can emit PRIMARY KEY (...) for table, distributed_table, and seed materializations, with docs and YAML/SQL examples in the changelog.

Refactors shared DDL helpers in table.sql: make_columns, renamed order_by_clause / partition_by_clause, and primary_key before order by / partition by. When only primary_key is set (no order_by), the adapter no longer forces default ORDER BY (tuple()), matching ClickHouse’s ability to rely on the primary key for sorting.

Integration tests assert system.tables primary_key and sorting_key for models with PK-only vs PK + explicit order_by.

Reviewed by Cursor Bugbot for commit c8c860d. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread dbt/include/clickhouse/macros/materializations/seed.sql
Comment thread dbt/include/clickhouse/macros/materializations/table.sql Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 25ec4af. Configure here.

vrkirillov

This comment was marked as resolved.

vrkirillov

This comment was marked as resolved.

@vrkirillov vrkirillov changed the title Primary key declaration fix: Primary key declaration May 27, 2026

@vrkirillov vrkirillov left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix recommendations in Cursor

vrkirillov

This comment was marked as resolved.

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.

1 participant