Add database-level uniqueness constraints for a couple tables/columns #405
Closed
matthewbennink
started this conversation in
General
Replies: 1 comment 2 replies
-
Agreed @matthewbennink, this makes sense to add. For user let's just have it be unique on email (not email, type combo). Notably, the same email with different capitalizations will not be caught but you can downcase in your API calls. That was actually the reason I left it off b/c I didn't want to go through the trouble of case insensitive indexes with citext. I did that on another project and it was surprisingly annoying to maintain. I agree that the credential columns should be paired with the type. Those should of had the index all along so that was just an oversight. thanks for catching! |
Beta Was this translation helpful? Give feedback.
2 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
While very unlikely with manual registration, I noticed it was possible to create multiple
Person
records with identical emails when making API calls. The scenario is two threads trying to create the samePerson
record in parallel. Is there any harm to updating the "people" index to be:The index on email will give a small boost in performance for servers with many thousands of users, but more importantly it will ensure that there can only be a single
Person
record (perpersonable_type
) with a given email. The AR validation already handles this for manual registration, and I can surely handle this case myself, but the database-level constraint feels nice to have.This could also just be
t.index ["email"], name: "index_people_on_email", unique: true
, which would match the current AR validation (validates :email, ..., uniqueness: true, ...
), and feels conceptually simpler.The same general issue affects
Credential#external_id
andCredential#oauth_email
. In the very unlikely event that multiple credentials were created with the same values but for different users, then it's uncertain who might be logged in.And the same general code change applies - adding a unique constraint to the index, though I think it would be necessary to scope by
type
in this case sincetype
will always be used when querying the STI table.Beta Was this translation helpful? Give feedback.
All reactions