Set Person.Trn to be non-nullable [DO NOT MERGE] #2688
Conversation
stusherwin
force-pushed
the
make-trn-non-nullable
branch
from
15a76d2 to
5a79dd1
Compare
stusherwin
changed the title
stusherwin
force-pushed
the
make-trn-non-nullable
branch
2 times, most recently
from
3968eb0 to
f1e3888
Compare
| builder.HasIndex(p => p.DqtContactId).HasFilter("dqt_contact_id is not null").IsUnique(); | ||
| builder.HasIndex(p => p.MergedWithPersonId).HasFilter("merged_with_person_id is not null"); | ||
| builder.HasIndex(p => p.Trn).HasFilter("trn is not null").IsUnique(); | ||
| builder.HasIndex(p => p.Trn).IsUnique(); |
There was a problem hiding this comment.
Probably want an .IsCreatedConcurrently() on this otherwise we might have timeout issues deploying on prod.
There was a problem hiding this comment.
Oh right - just looked at postgres docs and it says
Another difference is that a regular CREATE INDEX command can be performed within a transaction block, but CREATE INDEX CONCURRENTLY cannot
Is that going to be an issue?
There was a problem hiding this comment.
It's best to put the CreateIndex into its own migration to mitigate that
There was a problem hiding this comment.
Given the difficulty with this, it might be simpler to leave the index as-is
There was a problem hiding this comment.
Having potential timeouts on prod seems worrying though?
There was a problem hiding this comment.
Oh you mean leave the filter in place. Sure can do that
stusherwin
force-pushed
the
make-trn-non-nullable
branch
from
f1e3888 to
a1e0e0a
Compare
DO NOT MERGE UNTIL DELETE JOB HAS BEEN RUN ON ALL ENVIRONMENTS
Otherwise the migration will fail due to null TRNs
Environments to run job on first
Context
Once we’ve run the job to delete records without a TRN we should make the TRN column on person non-nullable to ensure no more records are created without a TRN. (It’ll also tidy up our nullable annotations and prevent the need for !s everywhere where we know we have a TRN.)
Changes proposed in this pull request
Checklist