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.

Change History (1)

comment:1 by Alex Gaynor, 15 years ago

Resolution: duplicate
Status: newclosed

Dupe of #11267.

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