#34949 closed Cleanup/optimization (fixed)
Clarify when unique constraints are ignored.
Reported by: | Roman Odaisky | Owned by: | Jordan Bae |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Jordan Bae | 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
Consider the following code:
class SomeModel(models.Model): a = models.IntegerField() b = models.IntegerField() c = models.IntegerField() d = models.IntegerField() e = models.IntegerField() f = models.IntegerField() g = models.IntegerField(null=True) class Meta: indexes = [ models.Index(fields=["a"], name="index_a"), models.Index(fields=["b"], include=["c"], name="index_b_include_c"), ] constraints = [ models.UniqueConstraint(fields=["d"], name="unique_d"), models.UniqueConstraint(fields=["e"], include=["f"], name="unique_e_include_f"), models.UniqueConstraint(fields=["g"], nulls_distinct=True, name="unique_g_nulls_distinct"), ]
Expected Behavior
From the documentation:
include
is ignored for databases besides PostgreSQL.nulls_distinct
is ignored for databases besides PostgreSQL 15+.
One would imagine that if we run this on SQLite we’ll end up with five indexes on one field each as though the unsupported parameters were not present.
Actual Behavior
Instead we get:
index_a
ona
index_b_include_c
onb
unique_d
ond
- and no other constraints!
The documentation and the behavior are clearly at odds, and the behavior is very confusing. The intention behind indexes like unique_e_include_f
is to enforce a constraint and to enable an optimization on top of that, Django is unable to provide the optimization so it doesn’t enforce the constraint either?
Workflows Affected
It seems to me there are two important workflows, none of which Django currently supports:
- A project for which the DB has been chosen long ago, and a migration to a different one is very unlikely. The developer would like to get the most out of the DB and is willing to use DB-specific features. In this case Django should raise errors if the specific features requested are unavailable.
- A DB-agnostic app intended to be included in other projects. The developer would like Django to create indexes that best match the performance requirements as appropriate for whatever DB the app may end up running on.
For example, in the second case, anticipating a lot of SomeModel.objects.filter(b__range=...).values("b", "c")
the developer would like to create an Index(fields=["b"], include=["c"])
but with a fallback to Index(fields=["b", "c"])
if covering indexes aren’t supported. The interface to create custom indexes is in theory documented but very sparsely (it doesn’t even say what the arguments to create_sql are) so expecting app developers to use this route is unrealistic.
Note that while in many other cases failing to perform an optimization is considered a graceful fallback, indexes are explicitly created for purposes of optimization and failing to achieve that is an error that should be signaled.
Suggested Resolution
- Under no circumstances should Django quietly decline to create a unique constraint.
- Index and UniqueConstraint classes should have extra parameters to specify what to fall back to if requested features are unavailable. Perhaps something along the lines of
models.Index( fields=["b"], include=["c"], fallback=models.Index(fields=["b", "c"]), # where does the name go though? ) models.UniqueConstraint( fields=["b"], include=["c"], fallback=[ models.UniqueConstraint(fields=["b"]), models.Index(fields=["b", "c"]), ], ) models.UniqueConstraint( fields=["x"], nulls_distinct=True, fallback=models.UniqueConstraint(Coalesce(F("x"), 0)), )
- There should be a setting to make it a warning or an error if Django encounters an index that it can’t create exactly and that has no user-specified fallback.
- A perfect solution would also provide a way of upgrading indexes if a later version of the DB adds support for the features that were previously missing.
Here’s my attempt to approximate a solution:
class FallbackIndex(models.Index): def __init__(self, *a, requires=None, fallback=None, **k): super().__init__(*a, **k) self.requires = requires self.fallback = fallback def create_sql(self, model, schema_editor, using="", **kwargs): return ( super() if getattr(schema_editor.connection.features, self.requires) else self.fallback ).create_sql(model, schema_editor, using=using, **kwargs) def deconstruct(self): path, expressions, kwargs = super().deconstruct() kwargs["requires"] = self.requires kwargs["fallback"] = self.fallback return path, expressions, kwargs indexes = [ FallbackIndex( fields=["b"], include=["c"], name="index_b_include_c", requires="supports_covering_indexes", fallback=models.Index(fields=["b", "c"], name="index_b_include_c"), ), ]
except it should be possible to determine the required features automatically.
Change History (19)
comment:1 by , 12 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 12 months ago
Here's the forum discussion I raised: https://forum.djangoproject.com/t/request-for-comments-about-a-ticket-index-vs-uniqueconstraint-inconsistency/25097
Only got 1 response but I agree with them so that's 2 for changing the behaviour 🤷♂️
Roman, would you like to prepare a patch?
comment:3 by , 12 months ago
A patch that would do exactly what? By what means do we make it possible for UniqueConstraint(fields=["a"], include=["b"])
to fall back to UniqueConstraint(fields=["a"])
+ Index(fields=["a", "b"])
?
comment:4 by , 10 months ago
comment:5 by , 10 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 10 months ago
Perhaps the easiest way of implementing this would be something along the lines of
class Meta: constraints = [ models.UniqueConstraint(fields=["e"], include=["f"], if_has_features=["supports_covering_indexes"]), models.UniqueConstraint(fields=["e"], unless_has_features=["supports_covering_indexes"]), ] indexes = [ models.Index(fields=["e", "f"], unless_has_features=["supports_covering_indexes"]), ]
The only question that would remain would be, what if the DB gains support for a feature it previously lacked.
comment:7 by , 10 months ago
Thanks for sharing ur thought. Currently I am trying to update two changes.
when i check 4options related this issue with below code
class Ticket34949(models.Model): a = models.IntegerField() b = models.IntegerField() c = models.IntegerField() d = models.IntegerField() e = models.IntegerField() f = models.IntegerField() class Meta: constraints = [ models.UniqueConstraint(fields=['a'], name='unique_a'), models.UniqueConstraint(fields=['b'], name='unique_b_c', include=['c']), models.UniqueConstraint(fields=['d'], name='unique_d_e', deferrable=models.Deferrable.DEFERRED), models.UniqueConstraint(fields=['e'], name='unique_e', nulls_distinct=False), models.UniqueConstraint(fields=['f'], name='unique_f', opclasses=['varchar_pattern_ops']) ]
Django makes only a,f unique constraints in MySQL.
I think opclasses, include are option for optimization. so it needs to make unique constraints as David said.
And nulls_distinct, deferrable are related to unique constraints behavior. so, Django shouldn't make unique constraints. so, my actions are below two items.
1) fix includes
2) update documents for distinguishing between opclasses, include and nulls_distinct, deferrable.
comment:8 by , 10 months ago
Cc: | added |
---|---|
Has patch: | set |
comment:9 by , 10 months ago
I'm not really sure what do we want to achieve in this ticket.
Under no circumstances should Django quietly decline to create a unique constraint.
It doesn't. Django has system checks for all unsupported options models.W036
-> models.W040
so none of them is silently omitted.
comment:10 by , 10 months ago
Patch needs improvement: | set |
---|
comment:11 by , 10 months ago
Patch needs improvement: | unset |
---|
comment:12 by , 10 months ago
I'm not really sure what do we want to achieve in this ticket.
Suppose I want to write a reusable Django app that others will include in their projects. A model in the app wants an index that’s best described as UniqueConstraint(fields=["a", "b"], include=["c", "d"])
. How do I make Django create constraints and/or indexes that best approximate this on whatever DB the user will run their project?
comment:13 by , 10 months ago
If you want to create a cross-database constraint you should use options support by all databases.
comment:14 by , 10 months ago
Patch needs improvement: | set |
---|
comment:15 by , 10 months ago
Patch needs improvement: | unset |
---|
comment:16 by , 10 months ago
Component: | Database layer (models, ORM) → Documentation |
---|---|
Type: | Bug → Cleanup/optimization |
comment:17 by , 10 months ago
Summary: | Fails to create unique constraints → Clarify when unique constraints are ignored. |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Hey thanks for the report!
Accepting though I don't know whether this will end up being a documentation change or behaviour change. I agree the behaviour is inconsistent between indexes & constraints, though there may be a few factors to consider here and other team members will weigh in on whether making the behaviour consistent is a desirable outcome. (If we do change behaviour this ticket may need to be split in 2 as
nulls_distinct
is a new Django 5.0 feature.)