[Django] #29622: Indexes on JSONB paths fail system checks in 2.1
Django
------------------------------------------------+------------------------
Reporter: James Howe | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: 2.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 |
------------------------------------------------+------------------------
I am using PostgreSQL and have some JSONField-type fields in my models.
I had added some indexes to these, and they worked fine previously.
{{{#!python
class Meta:
indexes = [
models.Index(fields=['json_field__property'], name='my_idx'])
]
}}}
However, having just upgraded to Django 2.1, the checks on startup now
fail:
{{{
ERRORS:
myapp.MyModel: (models.E012) 'indexes' refers to the nonexistent field
'json_field__property'.
}}}
My tests (run with pytest-django) all still work, so it seems to only be
the system check that's the problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/29622>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: James Howe | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: 2.1
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 Tim Graham):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
The system check is new in Django 2.1 (#28714).
--
Ticket URL: <https://code.djangoproject.com/ticket/29622#comment:1>
Django
Reporter: James Howe | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
The check probably needs to be adjusted to use `get_lookups`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29622#comment:2>
Django
Reporter: James Howe | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Josh Schneier):
How did this ever work & what version of Django are you upgrading from?
`set_name_with_model` uses `._meta.get_field` which would fail but I see
you've worked around that by explicitly naming your index; however
`create_sql` uses the same logic.
I was able to get the system check to pass & the migration to generate by:
splitting on `LOOKUP_SEP`, checking if it's a `JSONField`, and ensuring
that none of the lookup paths is in `.get_lookups`. Attempting to apply
the migration failed because of the aforementioned issues in
`.create_sql`. Obviously that also required importing `JSONField` which is
a no-go.
I think one approach to fixing this is an `Index` subclass that works
specifically for `JSONField` since it is pretty different, happy to work
on it if that sounds reasonable but I'll let someone else more familiar
with the system take a look . We'd have to call this previous behavior
unsupported I think.
--
Ticket URL: <https://code.djangoproject.com/ticket/29622#comment:3>
Django
Reporter: James Howe | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: 2.1
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by James Howe):
> How did this ever work
Ah, I also have a custom SQL migration so I could make it partial.
Adding {{{'models.E012'}}} to {{{SILENCED_SYSTEM_CHECKS}}} should give
exactly the same behaviour as in 2.0?
--
Ticket URL: <https://code.djangoproject.com/ticket/29622#comment:4>
Django
Reporter: James Howe | Owner: nobody
Type: Bug | Status: new
Component: Core (System checks) | Version: 2.1
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 Simon Charette):
* severity: Release blocker => Normal
Comment:
This is not a release blocker in this case.
@Josh I don't think we should ship a special index for this case. #26167
will add support for functional indices where the above will be
expressible using `KeyTransform`s.
Not sure if this ticket should be closed as duplicate of #26167 or a new
ticket should be opened (or this one renamed) to add support for string
transforms to `Index` to reduce the boiler plate required to define a
functional index.
e.g.
- `Index('join_date__year')` instead of `Index(ExtractYear('join_date'))`
- `Index('json_field__property')` instead of
`Index(KeyTransform('property', 'json_field'))`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29622#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: James Howe | Owner: nobody
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* cc: Hannes Ljungberg (added)
* component: Core (System checks) => Database layer (models, ORM)
* version: 2.1 => master
* type: Bug => New feature
* stage: Accepted => Someday/Maybe
Comment:
I agree with Simon, this ticket is an improvement to the #26167.
Functional indexes are complicated even without this, so we should wait
for #26167 to land.
--
Ticket URL: <https://code.djangoproject.com/ticket/29622#comment:6>
Django
-------------------------------------+-------------------------------------
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage:
| Someday/Maybe
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => duplicate
Comment:
`F()` expressions support transforms since
8b040e3cbbb2e81420e777afc3ca48a1c8f4dd5a, so now it's a duplicate of
#26167.
--
Ticket URL: <https://code.djangoproject.com/ticket/29622#comment:7>