Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#31351 closed Cleanup/optimization (fixed)

Need warning when UniqueConstraint with condition is used but supports_partial_indexes is False

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

Description

Databases that support supports_partial_indexes allow using a UniqueConstraint with a condition specified (implemented in #29547 and #30062). However, when such constraints are specified, but the database (e.g. Mysql/Mariadb or older versions of sqlite) does not support partial indexes, these constraints are silently ignored.

It would be better if this would give a warning, similar to the warning for CheckConstraints at https://github.com/django/django/blob/9358da704ea9ba55f22df912e47b54eb85d5c97e/django/db/models/base.py#L1838-L1862

I have observed this with 2.2.10, but looking at the code, this has not changed since then, so I'm setting version to master.

This is probably a fairly trivial change, but I'm not in the opportunity to provide a PR in the near future.

Change History (12)

comment:1 by Ichlasul Affan, 5 years ago

Cc: Ichlasul Affan added
Needs tests: set
Owner: changed from nobody to Ichlasul Affan
Status: newassigned
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

As mentioned by #30062, the warning should be given when using MySQL/MariaDB or SQLite version <= 3.8.

in reply to:  description comment:2 by Ichlasul Affan, 5 years ago

Replying to Matthijs Kooijman:

Databases that support supports_partial_indexes allow using a UniqueConstraint with a condition specified (implemented in #29547 and #30062). However, when such constraints are specified, but the database (e.g. Mysql/Mariadb or older versions of sqlite) does not support partial indexes, these constraints are silently ignored.

It would be better if this would give a warning, similar to the warning for CheckConstraints at https://github.com/django/django/blob/9358da704ea9ba55f22df912e47b54eb85d5c97e/django/db/models/base.py#L1838-L1862

I have observed this with 2.2.10, but looking at the code, this has not changed since then, so I'm setting version to master.

This is probably a fairly trivial change, but I'm not in the opportunity to provide a PR in the near future.

Firstly, I'm sorry I am new contributor to Django. When I explored further on the code, the warning checks for specific DBMS' ability to check constraints using connection.features.support_table_check_constraints. Unfortunately, I think that similiar type of variable would be available on some database connectors for this case (unique constraint with conditions). Would you mind if I just add the warning regardless of what kind of database users will use?

UPDATE: Oh nevermind, I can just use support_partial_indexes. The default definitions is actually available via django.db.backends.base.features.

Last edited 5 years ago by Ichlasul Affan (previous) (diff)

comment:3 by Matthijs Kooijman, 5 years ago

Yeah, I think the check can just look at supports_partial_indexes, since that is also what [the code that decides (not) to add the constraint](https://github.com/django/django/blob/57a3d96ff57abffa8e7fbe3fd972a5ee39bd0fec/django/db/backends/base/schema.py#L1097) looks at.

comment:4 by Ichlasul Affan, 5 years ago

Last edited 5 years ago by Ichlasul Affan (previous) (diff)

comment:5 by Matthijs Kooijman, 5 years ago

Has patch: set
Needs tests: unset
Patch needs improvement: set

comment:6 by Ichlasul Affan, 5 years ago

Patch needs improvement: unset

comment:7 by Mariusz Felisiak, 5 years ago

Patch needs improvement: set

comment:8 by Ian Foote, 5 years ago

Cc: Ian Foote added

comment:9 by Ian Foote, 5 years ago

Patch needs improvement: unset

I just had a look over the PR and it looks like the outstanding issues have been resolved.

comment:10 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 53d229ff:

Fixed #31351 -- Added system checks for partial indexes and unique constraints support.

comment:12 by GitHub <noreply@…>, 5 years ago

In 4c988608:

Refs #31351 -- Made partial constraints tests use required_db_features.

This will notably silence the warnings issued when running the test
suite on MySQL and MariaDB.

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