Opened 5 years ago

Closed 4 years ago

#30916 closed New feature (fixed)

Added support for functional constraints.

Reported by: Safwan Rahman Owned by: Hannes Ljungberg
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: constraint functional
Cc: Ian Foote, Hannes Ljungberg Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Safwan Rahman)

Case insensitive unique constraint is needed for various purposes like storing emails. In postgresql, its possible to add a functional unique index with `Lower` and it will work as a case insensitive unique constraint. I have tried to implement that functional unique index/constraint in django, but looking at the source it seems that it is not possible to add functional unique index/constraint in django. PR 11929 is created for adding functional index, but it seems like it is not possible to create unique functional index with it.

I think there should be a option like case_insensitive=True that can be passed to UniqueConstraint so the UniqueConstraint create a functional unique index with Lower.

I know, its possible to store the value as lower case always and mitigate this issue, but it does not provide DB level constrains safety from anomaly.

Change History (26)

comment:1 by Safwan Rahman, 5 years ago

Description: modified (diff)

comment:2 by Safwan Rahman, 5 years ago

Description: modified (diff)

comment:3 by Simon Charette, 5 years ago

Keywords: constraint functional added; UniqueConstraint removed
Summary: Add support for case insensitive unique constraintAdded support for functional constraints
Triage Stage: UnreviewedAccepted

Should we close this one as a duplicate of #26167.

I'll at least rename the issue to more generic name.

comment:4 by saadalsaad, 5 years ago

Owner: changed from nobody to saadalsaad
Status: newassigned

comment:5 by Mariusz Felisiak, 5 years ago

Summary: Added support for functional constraintsAdded support for functional constraints.
Version: 2.2master

Probably some or most of logic can be reused from functional indexes (#26167) (e.g. with a mixin) but I think we should treat this as a separate feature.

comment:6 by Carlos_Mir_de_Souza, 5 years ago

Has patch: set

Looking the history in github it seems that was closed because the code was merged, but after that it had improvement/modifications, it would be interesting if the author can clarify the status of the PR, if the discussion is about the same topic or about other things.

Last edited 5 years ago by Carlos_Mir_de_Souza (previous) (diff)

comment:7 by Mariusz Felisiak, 5 years ago

Has patch: unset

CarlosMirdeSouza, this ticket doesn't have a patch.

Last edited 5 years ago by Mariusz Felisiak (previous) (diff)

comment:8 by Carlos_Mir_de_Souza, 5 years ago

He references the PR 11929, it doesn't count has a patch?
https://github.com/django/django/pull/11929

comment:9 by Carlos_Mir_de_Souza, 5 years ago

I was confused, my bad, sorry for the mistake, it references the PR for other ticket.

Last edited 5 years ago by Carlos_Mir_de_Souza (previous) (diff)

comment:10 by Mariusz Felisiak, 5 years ago

PR11929 is about functional indexes, not functional constraints.

comment:11 by Carlos_Mir_de_Souza, 5 years ago

Thanks for the explanation, i will have more attention for next time.

comment:12 by saadalsaad, 5 years ago

from my understanding, we need to support for functional constraints by adding case_insensitive option to UniqueConstraint ?
https://github.com/django/django/pull/12115

comment:13 by Mariusz Felisiak, 5 years ago

saadalsaad, no. This ticket is about adding support for functional constraints not only about case insensitive check constraints. It's more generic.

comment:14 by saadalsaad, 5 years ago

felixxm, since this ticket is generic for functional constraints, can we move case_insensitive option to a sperate ticket?

comment:15 by Mariusz Felisiak, 5 years ago

saadalsaad, not really we don't want to add such specific parameters.

comment:16 by saadalsaad, 5 years ago

Replying to Safwan Rahman:

Case insensitive unique constraint is needed for various purposes like storing emails. In postgresql, its possible to add a functional unique index with `Lower` and it will work as a case insensitive unique constraint. I have tried to implement that functional unique index/constraint in django, but looking at the source it seems that it is not possible to add functional unique index/constraint in django. PR 11929 is created for adding functional index, but it seems like it is not possible to create unique functional index with it.

I think there should be a option like case_insensitive=True that can be passed to UniqueConstraint so the UniqueConstraint create a functional unique index with Lower.

I know, its possible to store the value as lower case always and mitigate this issue, but it does not provide DB level constrains safety from anomaly.

Ticket description suggested adding case_insensitive option to be added in UniqueConstraint to create a functional unique index with Lower
since we are not going to add such option, seems we need to modify ticket description
felixxm, do you mean supporting functional constraints (supporting expressions in class based constraints ?) similar to indexes in #26167 ?

Thanks

in reply to:  16 comment:17 by Mariusz Felisiak, 5 years ago

Replying to saadalsaad:

felixxm, do you mean supporting functional constraints (supporting expressions in class based constraints ?) similar to indexes in #26167 ?

Thanks

Yes exactly, see comment.

comment:18 by Sanskar Jaiswal, 5 years ago

Was a common consensus reached regarding this? I would love to give this a shot if I could understand what all we expect from this feature.

comment:19 by Ian Foote, 5 years ago

Cc: Ian Foote added

comment:20 by Hannes Ljungberg, 4 years ago

Cc: Hannes Ljungberg added

comment:21 by Hannes Ljungberg, 4 years ago

Owner: changed from saadalsaad to Hannes Ljungberg

I intend to create a patch for this after #26167 is merged.

Last edited 4 years ago by Mariusz Felisiak (previous) (diff)

comment:23 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:24 by Hannes Ljungberg, 4 years ago

Patch needs improvement: unset

comment:25 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:26 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 3aa5452:

Fixed #30916 -- Added support for functional unique constraints.

Thanks Ian Foote and Mariusz Felisiak for reviews.

Note: See TracTickets for help on using tickets.
Back to Top