[Django] #34949: Fails to create unique constraints

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman | Owner: nobody
Odaisky |
Type: Bug | Status: new
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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` on `a`
* `index_b_include_c` on `b`
* `unique_d` on `d`
* 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:
1. 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.
2. 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
[https://docs.djangoproject.com/en/dev/ref/models/constraints/#baseconstraint
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 =
1. Under no circumstances should Django quietly decline to create a unique
constraint.
2. 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)),
)
}}}
3. 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.
4. 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.

--
Ticket URL: <https://code.djangoproject.com/ticket/34949>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------

Reporter: Roman Odaisky | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by David Sanders):

* stage: Unreviewed => Accepted

Comment:

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.)

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:1>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by David Sanders):

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?

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:2>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Roman Odaisky):

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"])`?

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:3>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jordan Bae):

I think he intended that models.UniqueConstraint(fields=["e"],
include=["f"], name="unique_e_include_f") makes 'unique_e_include_f' on
'e' like models.Index. and UniqueConstraint with nulls_distinct act
ignored like deferrable so need to update documentation. If you are okay,
can i handle this?

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:4>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------

Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jordan Bae):

* owner: nobody => Jordan Bae
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:5>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Roman Odaisky):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:6>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jordan Bae):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:7>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jordan Bae):

* cc: Jordan Bae (added)
* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:8>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:9>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:10>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jordan Bae):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:11>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Roman Odaisky):

>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?

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:12>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

If you want to create a cross-database constraint you should use options
support by all databases.

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:13>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:14>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan
| Bae
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jordan Bae):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:15>

[Django]

#34949: Fails to create unique constraints
-------------------------------------+-------------------------------------
Reporter: Roman Odaisky | Owner: Jordan

Type: | Bae
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* type: Bug => Cleanup/optimization
* component: Database layer (models, ORM) => Documentation

--
Ticket URL: <https://code.djangoproject.com/ticket/34949#comment:16>