fix: honor expression on primary_key constraints in V1 materialization - #1551
Merged
Conversation
The V1 (default) persist_constraints path rendered `primary key(<cols>)` and silently dropped the constraint's `expression`, so a primary key declared with `expression: RELY` was created without it. The V2 path and V1's own foreign_key branch already honor `expression`; the primary_key branch now appends it as a trailing suffix. Adds a get_constraint_sql macro unit test covering the expression suffix.
When no explicit name is provided, append expression to the hash input so unnamed PKs with different trailing clauses (e.g. RELY) get distinct names.
Collaborator
Author
|
/integration-test |
|
Integration tests dispatched for PR #1551 by @sd-db. Track progress in the Actions tab. |
|
Integration results for PR #1551 — UC cluster ✅ success · SQL warehouse ❌ failure · All-purpose cluster ✅ success · Shard coverage ✅ success |
|
Integration results for PR #1551 — UC cluster ✅ success · SQL warehouse ✅ success · All-purpose cluster ✅ success · Shard coverage ✅ success |
jprakash-db
approved these changes
Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
On the V1 materialization path (
use_materialization_v2: false, the default), thepersist_constraintsflow renderedprimary key(<cols>)and silently dropped the constraint'sexpression. A primary key declared withexpression: RELY(or any trailing clause) was created without it — no error, no warning.The V2 path (
PrimaryKeyConstraint._render_suffix) and V1's ownforeign_keybranch already honorexpression. This change makes the V1primary_keybranch inrelations/constraints.sqlappendexpressionas a trailing suffix, gated on its presence so a PK without an expression renders identically to before.Why
PRIMARY KEY (id) RELYis valid Databricks syntax for an informational primary key the optimizer can rely on. Before this fix, the only way to attachRELYat model-build time on the default path was a post-hook.Given (default path,
contract.enforced: true):ALTER TABLE … ADD CONSTRAINT pk PRIMARY KEY (id);← RELY droppedALTER TABLE … ADD CONSTRAINT pk PRIMARY KEY (id) RELY;Tests
test_macros_get_constraint_sql_primary_key_with_expression(tests/unit/macros/relations/test_constraint_macros.py) asserting the rendered SQL carries the expression suffix. Fails without the fix, passes with it.expressionrender byte-identically, so the pre-existing constraint macro unit tests are unchanged — full unit suite green.constraints/+persist_constraints/functional suites pass on a UC SQL warehouse (39 passed).