Opened 15 years ago
Closed 15 years ago
#11266 closed (duplicate)
ForeignKey/OneToOneField attribute names should be valid kwargs in queries
Reported by: | dstora | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.0 |
Severity: | Keywords: | foreign key ForeignKey OneToOneField query filter | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently, only the field names are available as kwargs for queries.
However, it makes (even more) sense that attribute names are supported too for ForeignKey/OneToOneField fields.
Let's take the following example:
- I initially have a table "Payment" with a "ccy_id" field designating a 3-letter currency code.
My model definition looks like:
from django.db import model class Payment(models.Model): # ... ccy_id = models.CharField(max_length=3)
In my python code I can retrieve payments by currency id as follows:
payment_list = list(Payment.objects.filter(ccy_id="USD"))
- Later, I decide to actually create a "Currency" table to hold some information about currencies.
And I also decide to define a foreign key constraint from "Payment" to "Currency".
My model now looks like:
from django.db import model class Currency(models.Model): ccy_id = models.CharField(max_length=3, primary_key=True) #... class Payment(models.Model): # ... ccy = models.ForeignKey(Currency, to_field="ccy_id", db_column="ccy_id")
The problem here is that my existing Python code is broken, because "ccy_id" is not accepted anymore as kwarg for "Payment" queries.
Django now only accepts "ccy" instead.
Based on the principle that defining things like foreign keys should only add functionality (and not remove any) it seems quite important that on top of the field names queries also support attribute name kwargs.
Looking at the code, I can see two ways of achieving this.
In both cases, the implementation of this feature is pretty small and fully backward-compatible:
- The first way is to register fields in options.Option against both their name and attname if these differ (SVN diff against trunk below)
Index: django/db/models/options.py =================================================================== --- django/db/models/options.py (revision 10924) +++ django/db/models/options.py (working copy) @@ -329,6 +329,8 @@ cache[f.name] = (f, model, True, True) for f, model in self.get_fields_with_model(): cache[f.name] = (f, model, True, False) + if f.attname != f.name: + cache[f.attname] = (f, model, True, False) if self.order_with_respect_to: cache['_order'] = OrderWrt(), None, True, False if app_cache_ready():
- The second way is at query level to always enable a logic currently marked as a hack that looks at attname if no field is found with the requested name (SVN diff against trunk below)
Index: django/db/models/sql/query.py =================================================================== --- django/db/models/sql/query.py (revision 10924) +++ django/db/models/sql/query.py (working copy) @@ -1678,8 +1678,7 @@ self.used_aliases = used_aliases def setup_joins(self, names, opts, alias, dupe_multis, allow_many=True, - allow_explicit_fk=False, can_reuse=None, negate=False, - process_extras=True): + can_reuse=None, negate=False, process_extras=True): """ Compute the necessary table joins for the passage through the fields given in 'names'. 'opts' is the Options class for the current model @@ -1715,10 +1714,7 @@ field, model, direct, m2m = opts.get_field_by_name(name) except FieldDoesNotExist: for f in opts.fields: - if allow_explicit_fk and name == f.attname: - # XXX: A hack to allow foo_id to work in values() for - # backwards compatibility purposes. If we dropped that - # feature, this could be removed. + if name == f.attname: field, model, direct, m2m = opts.get_field_by_name(f.name) break else: @@ -2026,8 +2022,7 @@ try: for name in field_names: field, target, u2, joins, u3, u4 = self.setup_joins( - name.split(LOOKUP_SEP), opts, alias, False, allow_m2m, - True) + name.split(LOOKUP_SEP), opts, alias, False, allow_m2m) final_alias = joins[-1] col = target.column if len(joins) > 1:
As a note, for ForeignKey/OneToOneField fields queries can be passed either objects or ids.
For example, if I want all the payments in a given currency, I can pass a Currency object or a currency id to "Payment.objects.filter".
However, in practice, I can't think of a reason why one would pass an object.
Indeed, if there's a foreign key from Payment to Currency, then given a Currency object "c" I can get to all the corresponding Payment objects by using the backward reference "c.payment_set".
Hence the reason why I think that it makes even more sense for queries to support the attribute name as query kwarg rather than the ForeignKey/OneToOneField field name.
Dupe of #11267.