Opened 9 months ago
Closed 9 months ago
#36165 closed Bug (fixed)
Postgres schema editor throws away custom index deletion SQL
| Reported by: | Daniel Finch | Owned by: | Natalia Bidart |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Mariusz Felisiak | 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
If I wanted to create a Postgres database index type that used custom SQL for both creation and deletion, the custom SQL would only apply in the create case.
Here's some example code:
from django.db.models import Index
class UniqueIndex(Index):
# Adjusted version of DatabaseSchemaEditor.sql_create_index that works with materialized views
sql_create_index = "CREATE UNIQUE INDEX %(name)s ON %(table)s%(using)s (%(columns)s);"
sql_delete_index = "DROP INDEX IF EXISTS %(name)s;"
def create_sql(self, model, schema_editor, using="", **kwargs):
kwargs.setdefault("sql", self.sql_create_index)
return super().create_sql(model, schema_editor, using, **kwargs)
def remove_sql(self, model, schema_editor, **kwargs):
kwargs.setdefault("sql", self.sql_delete_index)
return super().remove_sql(model, schema_editor, **kwargs)
The call to create_sql would do the right thing, and use the SQL supplied - as seen in the underlying code:
sql = sql or (
self.sql_create_index
if not concurrently
else self.sql_create_index_concurrently
)
However, the same cannot be said for the delete code, which simply ignores the provided SQL value:
sql = (
self.sql_delete_index_concurrently
if concurrently
else self.sql_delete_index
)
I believe this constitutes a bug and should have an obvious and simple fix.
Change History (4)
comment:1 by , 9 months ago
| Component: | CSRF → Database layer (models, ORM) |
|---|---|
| Has patch: | set |
| Owner: | set to |
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
| Version: | 5.1 → dev |
comment:2 by , 9 months ago
| Cc: | added |
|---|
comment:3 by , 9 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Note:
See TracTickets
for help on using tickets.
Hello Daniel, thank you for your report. I have spent some time reviewing code and docs for
Index. First of all, the docs do not document the methodscreate_sqlorremove_sql, so overriding these is not necessarily supported. But, after further investigation I found:sqlin create indexGiven the above, I'm accepting this ticket and I'm pushing a branch with fix and regression test for both methods.