[Django] #36165: Postgres schema editor throws away custom index deletion SQL

[Django]

#36165: Postgres schema editor throws away custom index deletion SQL
--------------------------------+--------------------------------------
Reporter: Daniel Samuels | Type: Bug
Status: new | Component: CSRF
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
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
)
}}}

https://github.com/django/django/blob/5f30fd2358fd60a514bdba31594bfc8122f30167/django/db/backends/postgresql/schema.py#L349-L353

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

https://github.com/django/django/blob/5f30fd2358fd60a514bdba31594bfc8122f30167/django/db/backends/postgresql/schema.py#L324-L330

I believe this constitutes a bug and should have an obvious and simple
fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/36165>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36165: Postgres schema editor throws away custom index deletion SQL

-------------------------------------+-------------------------------------
Reporter: Daniel Finch | Owner: Natalia
| Bidart
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 Natalia Bidart):

* component: CSRF => Database layer (models, ORM)
* has_patch: 0 => 1
* owner: (none) => Natalia Bidart
* stage: Unreviewed => Accepted
* status: new => assigned
* version: 5.1 => dev

Comment:

Hello Daniel, thank you for your report. I have spent some time reviewing
code and docs for `Index`. First of all,
[https://docs.djangoproject.com/en/5.1/ref/models/indexes/ the docs] do
not document the methods `create_sql` or `remove_sql`, so overriding these
is not necessarily supported. But, after further investigation I found:
* revno bd366ca2aeffa869b7dbc0b0aa01caea75e6dc31 added the support for the
custom `sql` in create index
* the rationale from Mariusz is still valid "''this may be helpful for
3rd-party database backends that subclass django.db.backends.postgresql. I
think it's worth adding''".

Given the above, I'm accepting this ticket and I'm pushing a branch with
fix and regression test for both methods.
--
Ticket URL: <https://code.djangoproject.com/ticket/36165#comment:1>

[Django]

#36165: Postgres schema editor throws away custom index deletion SQL
-------------------------------------+-------------------------------------
Reporter: Daniel Finch | Owner: Natalia
| Bidart
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 Natalia Bidart):

* cc: Mariusz Felisiak (added)

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

[Django]

#36165: Postgres schema editor throws away custom index deletion SQL
-------------------------------------+-------------------------------------
Reporter: Daniel Finch | Owner: Natalia
| Bidart
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* stage: Accepted => Ready for checkin

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

[Django]

#36165: Postgres schema editor throws away custom index deletion SQL
-------------------------------------+-------------------------------------
Reporter: Daniel Finch | Owner: Natalia
| Bidart

Type: Bug | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by nessita <124304+nessita@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"1f33f21fca60c3839bcfc756900fb78bcfd15cc3" 1f33f21f]:
{{{#!CommitTicketReference repository=""
revision="1f33f21fca60c3839bcfc756900fb78bcfd15cc3"
Fixed #36165 -- Made PostgreSQL's SchemaEditor._delete_index_sql() respect
the "sql" argument.

This is a follow up of bd366ca2aeffa869b7dbc0b0aa01caea75e6dc31.

Thank you Daniel Finch for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36165#comment:4>