ForeignKey/OneToOneField attribute names should be valid kwargs in queries

[dstora]

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:
  

[dstora]

Patch 1

[dstora]

Patch 2

[Alex Gaynor]

I'd like to wontfix this, as I don't see any reduction in functionality, you've changed the field name. However, I'm going to leave it open for a committer's opinion.

[dstora]

Indeed, I've changed the field name from "ccy_id" to "ccy" when making it a ForeignKey field.
I did it so that the id value can still be referenced to as "ccy_id":

p = Payment()
#...
if p.ccy_id == "USD":
    #...

So basically, the requirements so that one can add foreign key constraint without breaking existing code are:

• To name the ForeignKey/OneToOneField field differently and be able to decide the name Django uses for the id attribute (see ticket 11265)
  
• To be able to use the id attribute name in queries
  

[dstora]

Hi guys,

I patched the Django install in my corporate production environment a while ago now and it's been running absolutely fine.

The business case for this ticket is quite real - it comes from concrete real life working environment constraints - but I'm open to discussion if I haven't succeeded in making my point clear enough.

Could you please have a further look into it?

Thanks,

Dan

[Russell Keith-Magee]

Alex's original intuition is on the money. This is a wontfix for me, for two reasons:

• As the Zen of Python directs, there should only be one way to do it. We don't need two ways to construct queries on a foreign key.
• Django's ORM attempts to hide the details of the underlying database implementation. The database column name is an implementation detail that shouldn't be surfaced.

If you want to try and convince us otherwise, feel free to open a discussion on the django-dev mailing list - but you're going to need a more convincing argument than "I want Django's ORM to compensate for a bad naming decision that I made on an old model".

[mdengler]

I think everyone's managed to completely miss the point of this patch. This feature / patch is very useful to keep django-using code django-ish in an environment where one is ORM'ing a schema not under ones' control.

Differently put, the point is to add a small feature to cope with normal (and healthy) schema evolution in a world where a) schemas might not be under the control of the django-using coders; and/or b) the schema-designers' naming preferences do not match django's default naming conventions.

as to your comments:

1) "zen of python": not applicable anyway because the point is to handle schema changes more gracefully. Alex missed the point saying "the field name has changed" - no, it hasn't, and that's the whole point: the database column name has remained the same but django's going to change the ORM because of the new foreign-key-ness, in a cumbersome way.
Besides, the PEP-20 aphorism actually has the word "obvious" in a critical place: "There should be one-- and preferably only one --obvious way to do it." (terrible formatting from the original ​http://www.python.org/dev/peps/pep-0020/ ). This makes all the difference in applying this to knock-on effects of patches; no obvious redundancy has been introduced while making the django-generated code more usable.

2) It seems a bit contradictory to say both "django attempts to hide the [underlying db details like column names]" and "django shouldn't compensate for [underlying details like column names]". This patch isn't about bad column names :). It's about hiding underlying details that change when a column gets "expanded" into a table (c.f. "Later, I decide to actually create a ... table [to represent the old column's data and more]").