#35002 closed Bug (fixed)
nulls_distinct not honored for ALTER
Reported by: | Peter Thomassen | Owned by: | Peter Thomassen |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.0 |
Severity: | Release blocker | Keywords: | |
Cc: | 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
The SQL template for creating uniqueness constraints (BaseDatabaseSchemaEditor.sql_create_unique) does not include the nulls_distinct
field necessary to support #34701.
As a result, adding a UNIQUE NULLS NOT DISTINCT on migration will end up with standard UNIQUE constraint.
Replacing
"UNIQUE (%(columns)s)%(deferrable)s"
with
"UNIQUE%(nulls_distinct)s (%(columns)s)%(deferrable)s"
on line 114 (5.0rc1) worked for me. I'll be happy to create a PR for that.
Also, I noticed that the include
field does not appear to be rendered in this template as well, although the UniqueConstraint class does have an include
argument. I wonder if it should also be included there, as follows (inspired by the sql_create_unique_index template, which I think should only differ w.r.t. to the condition field):
"UNIQUE%(nulls_distinct)s (%(columns)s)%(include)s%(deferrable)s"
I picked "release blocker" because it's a new feature that's broken, not a random bug unrelated to this release. I hope that was right.
Change History (11)
comment:1 by , 9 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 9 months ago
Has patch: | unset |
---|
comment:3 by , 9 months ago
Peter, Do you have time today or tomorrow to work on this? The final 5.0 release is scheduled for Monday, so we're in a hurry. If you don't have time, I can handle it.
comment:6 by , 9 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 9 months ago
FYI, I am on it. -- I found that the includes
field missing in this template isn't a bug, because when that modifier is given, another template is used (sql_create_unique_index
). The patch will thus contain only the fix for sql_create_unique
.
comment:8 by , 9 months ago
Has patch: | set |
---|
Patch is here: https://github.com/django/django/pull/17567
Note the comment there regarding the second test.
comment:9 by , 9 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Definitely a release blocker thanks for testing against the release candidate.
Please submit a PR with the proposed changes and a regression test. I'd suggest making the
includes
change in a distinct commit as this feature was added in 3.2 and thus I don't think it will qualify for a backport.