-
Notifications
You must be signed in to change notification settings - Fork 103
Add support for nulls_not_distinct
option in create_constraint
and create_index
operations
#740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
nulls_not_distinct
option in create_constraint
operation for unique constraints
e1bad97
to
6ff0f9e
Compare
6ff0f9e
to
7e2b9b5
Compare
7e2b9b5
to
8ab51cf
Compare
8ab51cf
to
d0ec26c
Compare
d0ec26c
to
4d894cb
Compare
nulls_not_distinct
option in create_constraint
operation for unique constraintsnulls_not_distinct
option in create_constraint
and create_index
operations
f6c1033
to
7575377
Compare
18f1010
to
0e9be3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for the new nulls_not_distinct option used to indicate that nulls are considered equal in unique constraints and indexes when supported by PostgreSQL 15. Key changes include:
- Adding a new boolean field for nulls_not_distinct in constraint and index migration types.
- Updating SQL generation functions to optionally include the "NULLS NOT DISTINCT" clause.
- Enforcing that nulls_not_distinct is only applicable for unique constraints and updating tests and duplicate operations accordingly.
Reviewed Changes
Copilot reviewed 9 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
pkg/migrations/types.go | Adds new bool field (NullsNotDistinct) to migration types |
pkg/schema/schema.go | Adds new field in Index and UniqueConstraint structures with JSON mapping for nulls_not_distinct |
pkg/migrations/op_create_constraint.go | Passes the new nullsNotDistinct parameter and validates constraint type |
pkg/migrations/op_create_index.go | Condition to append "NULLS NOT DISTINCT" in index creation SQL |
pkg/migrations/index.go | Updates function signatures and SQL generation for unique indexes |
pkg/migrations/duplicate.go | Updates unique index duplication to include nullsNotDistinct flag |
pkg/migrations/duplicate_test.go | Adjusts test calls to include the new parameter |
pkg/migrations/op_set_unique.go | Updates call to createUniqueIndexConcurrently with false for nullsNotDistinct |
pkg/migrations/op_add_column.go | Updates call to createUniqueIndexConcurrently with false for nullsNotDistinct |
Files not reviewed (6)
- docs/operations/create_constraint.mdx: Language not supported
- docs/operations/create_index.mdx: Language not supported
- examples/44_add_table_unique_constraint.json: Language not supported
- internal/jsonschema/testdata/create-constraint-3-invalid-check-nulls.txtar: Language not supported
- pkg/state/init.sql: Language not supported
- schema.json: Language not supported
Comments suppressed due to low confidence (1)
pkg/schema/schema.go:118
- [nitpick] Consider using snake_case ("nulls_not_distinct") for the JSON tag to align with the migration types' naming convention.
NullsNotDistinct bool `json:"nullsNotDistinct"`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Left some comments.
In addition, we should also add tests in op_create_constraint_test.go
and op_create_index_test.go
to check that NULLS NOT DISTINCT
is applied correctly when set.
internal/jsonschema/testdata/create-constraint-3-invalid-check-nulls.txtar
Show resolved
Hide resolved
5764cd5
to
5de6ea7
Compare
6d8d642
to
5de6ea7
Compare
5de6ea7
to
e09788a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
We still need to add tests in op_create_constraint_test.go
and op_create_index_test.go
to check that nulls_not_distinct: true
is applied correctly when set. Currently we don't have anything to check that indexes/constraints created with nulls_not_distinct: true
actually results in a NULLS NOT DISTINCT
index/constraint.
internal/jsonschema/testdata/create-index-3-valid-unique-nulls.txtar
Outdated
Show resolved
Hide resolved
internal/jsonschema/testdata/create-constraint-5-valid-unique-nulls.txtar
Outdated
Show resolved
Hide resolved
pkg/state/state_test.go
Outdated
@@ -491,6 +494,43 @@ func TestReadSchema(t *testing.T) { | |||
}, | |||
}, | |||
}, | |||
{ | |||
name: "unique, nulls not distinct", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we also need a test for reading a unique constraint with created with NULLS NOT DISTINCT
.
524be8e
to
401d13a
Compare
Co-authored-by: Andrew Farries <[email protected]>
Co-authored-by: Andrew Farries <[email protected]>
….txtar Co-authored-by: Andrew Farries <[email protected]>
…nulls.txtar Co-authored-by: Andrew Farries <[email protected]>
401d13a
to
832935c
Compare
I am converting this to draft for now because we need to rework backfilling process significantly to get this working. |
Closing because it is outdated. |
Add support for
nulls_not_distinct
option increate_constraint
andcreate_index
operations for unique constraints and indexes.NULLS NOT DISTINCT
was introduced in PG 15, so I added some conditionals to check if we can retrieve the column frompg_index
. Without this function, we get an exception becauseindnullsnotdisctict
does not exist in PG 14.Closes #730