Opened 11 years ago
Closed 11 years ago
#21863 closed New feature (fixed)
Consider adding get_transform() to supplement get_lookup()
Reported by: | Anssi Kääriäinen | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.7-alpha-1 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently the API for get_lookup() is get_lookup(lookup_name). However, there are situations where a field will need more information to decide if it needs to return a lookup or a transform. Consider HStoreField. A candidate API for HStoreField is that:
hstore_field__exact=val
=> exact lookup against the hstore_fieldhstore_field=val
=> also exact lookup against the hstore_fieldhstore_field__exact__exact=val
=> get key exact from hstore, do an exact lookup against thathstore_field__nonlookup=val
=> get key nonlookup from hstore, do an exact lookup against that
This is similar to how fk__exact
and fk__exact__exact
work currently if the model pointed by fk has field named exact.
In addition, we want to support transforms in .values() etc calls in the future. So hstore_field__exact
should always resolve to transform if used in values().
Currently this is impossible to do. The get_lookup() method doesn't know if there are still further lookups in the path. It knows just the current lookup. If we are resolving path exact__exact
get_lookup() should return transform for the first exact
, lookup for the second exact
. If we are resolving just exact
, then lookup should be returned for the first 'exact'. When resolving exact
for .values('hstorefield__exact')
call in later releases, the exact
should resolve to transform.
If we add get_transform() method we could change the lookup resolution code in the ORM to do:
- for non-final lookups: always use get_transform()
- for final lookup in filter() calls: try get_lookup(lookup_name). If that doesn't return anything, use get_transform(lookup_name) + get_lookup('exact').
- for any lookup in .values() etc calls: always use get_transform()
The above examples would be resolved as follows:
hstorefield__exact=val
: get_lookup('exact') -> match, so use exact lookup against hstorefieldhstorefield=val
: get_lookup('exact') -> same as abovehstorefield__exact__exact=val
: get_transform('exact'), get_lookup('exact') -> match, so use exact lookup against key exact of hstorefieldhstorefield__nonlookup=val
: get_lookup('nonlookup') -> no match, so try get_transform('nonlookup') + get_lookup('exact') -> exact lookup against key nonlookup
The values() call will use always get_transform() so .values('hstorefield__exact
)` will work correctly.
There might also be need to add register_transform() method. If we have only register_lookup() but methods get_lookup() and get_transform() it might lead to some confusion.
Adding register_transform() will break the API, but that is hopefully OK during alpha.
Change History (4)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 11 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
comment:3 by , 11 years ago
Looks good to me for a first glance. I've got an idea to cover the foo-bar
problem - at least for hstore. I think we should also support hstorefield__contains={'foo-bar': 'value'}
as well. This is a "more than one way to do it", but there are some reasons why that would be a cleaner API with unknown values (possibly also with nested JSON structures), and it does make logical sense. This means you've always got this to fall back on if the "shorthand" of hstorefield__foobar='value'
does not work, as well as the more explicit hstorefield__foobar__exact='value'
.
I will try to update the docs tomorrow, but it doesn't need too much mention IMO above the examples in the ticket description. Let's get it in before the beta.
comment:4 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I've created a patch for this, see https://github.com/akaariai/django/compare/ticket_21863. Docs missing, but it should be OK otherwise.
I think we can easily add this still before beta, the change isn't that big (only those who override get_lookup() will be affected), and this allows for better implementations of nested fields (JSONField for example). It is notable that even with this change using JSONField or HStoreField on existing database schemas might be hard. What if you have a key like 'foo-bar'? That is not allowed as a keyword lookup with Django. We might need ability to use strings instead of kwarg=value for lookups (say,
Q('jsonfield__foo-bar__exact', value)
). Then the only problematic case would be__
in keys, but that seems really minor corner case. Another way to tackle this problem is addition of.annotate(foo_bar=JSONExtract('jsonfield', 'foo-bar'))
(that is, allow annotating non-aggregates to query). After doing the annotation one could just doqs.filter(foo_bar__exact=value)
.