Opened 6 years ago

Closed 3 years ago

#29622 closed New feature (duplicate)

Support string transforms in functional indexes.

Reported by: James Howe Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: jmh205@…, Hannes Ljungberg Triage Stage: Someday/Maybe
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

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.

Change History (7)

comment:1 by Tim Graham, 6 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

The system check is new in Django 2.1 (#28714).

comment:2 by Simon Charette, 6 years ago

The check probably needs to be adjusted to use get_lookups.

comment:3 by Josh Schneier, 6 years ago

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.

comment:4 by James Howe, 6 years ago

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?

comment:5 by Simon Charette, 6 years ago

Severity: Release blockerNormal

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

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

comment:6 by Mariusz Felisiak, 4 years ago

Cc: Hannes Ljungberg added
Component: Core (System checks)Database layer (models, ORM)
Summary: Indexes on JSONB paths fail system checks in 2.1Support string transforms in functional indexes.
Triage Stage: AcceptedSomeday/Maybe
Type: BugNew feature
Version: 2.1master

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.

comment:7 by Mariusz Felisiak, 3 years ago

Resolution: duplicate
Status: newclosed

F() expressions support transforms since 8b040e3cbbb2e81420e777afc3ca48a1c8f4dd5a, so now it's a duplicate of #26167.

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