trim_joins bug with nested related models using ForeignObject

[darius]

the order in which we declare the from_fields and to_fields does matter if we make a nested lookup.

it is clear that the fields in from_fields and to_fields must be on the same orders, but if 3 models use ForeignObject to links themselves, and the fields are the sames on all 3 models, the order in which they are declared in both fields must be the same.

a example app that can trigger the bug follow : 

class Address(models.Model):
    company = models.CharField(max_length=1)
    tiers_id = models.IntegerField()
    city = models.CharField(max_length=255)
    postcode = models.CharField(max_length=32)

    class Meta(object):
        unique_together = [
            ("company", "tiers_id"),
        ]


class Customer(models.Model):
    company = models.CharField(max_length=1)
    customer_id = models.IntegerField()
    name = models.CharField(max_length=255)
    address = ForeignObject(
        Address, on_delete=CASCADE, null=True,
        from_fields=["customer_id", "company"],
        to_fields=["tiers_id", "company"]
    )

    class Meta(object):
        unique_together = [
            ("company", "customer_id"),
        ]

class Contact(models.Model):
    company_code = models.CharField(max_length=1)
    customer_code = models.IntegerField()
    surname = models.CharField(max_length=255)
    # virtual field
    customer = ForeignObject(
        Customer, on_delete=CASCADE, related_name='contacts',
        # not the same order as for Customer -> address which is (customer, company)
        to_fields=["company", "customer_id"],
        from_fields=["company_code", "customer_code"]
        # with same order as for Customer, the bug does not trigger
        # to_fields = ["customer_id", "company"],
        # from_fields = ["customer_code", "company_code"]
    )

class PhoneNumber(models.Model):
    num = models.CharField(max_length=32)
    type_number = models.IntegerField()
    contact = models.ForeignKey(Contact, on_delete=CASCADE, related_name='phonenumbers')

with this models.py, the different orders of the fields Contact.customer can break all query like PhoneNumber.objects.filter(contact__customer__address=a).(see comment in Customer)

I found the problem is in django.db.models.query.Query.trim_joins line 1444 on release 1.9.5

targets = tuple(r[0] for r in info.join_field.related_fields if r[1].column in cur_targets)

we see that the new targets is created using the previous used targets, but the order of the previous target is not kept, and the order of to_fields is used to define the new targets.
this lead to a query like :

SELECT "buggyapp_phonenumber"."id", "buggyapp_phonenumber"."num", "buggyapp_phonenumber"."type_number", "buggyapp_phonenumber"."contact_id" FROM "buggyapp_phonenumber" INNER JOIN "buggyapp_contact" ON ("buggyapp_phonenumber"."contact_id" = "buggyapp_contact"."id") WHERE ("buggyapp_contact"."company_code" = 10 AND "buggyapp_contact"."customer_code" = a)

instead of

SELECT "buggyapp_phonenumber"."id", "buggyapp_phonenumber"."num", "buggyapp_phonenumber"."type_number", "buggyapp_phonenumber"."contact_id" FROM "buggyapp_phonenumber" INNER JOIN "buggyapp_contact" ON ("buggyapp_phonenumber"."contact_id" = "buggyapp_contact"."id") WHERE ("buggyapp_contact"."customer_code" = 10 AND "buggyapp_contact"."company_code" = a)

the where part have the values order kept, but the fields order is inconsistent and it end with customer_code = 'a' instead of 10.

the attached files contains a basic app that trigger the bug, and a patch in trim_query that will build the new targets with same order as the previous.
the patch is made for django 1.9.5

[darius]

simple app to trigger the bug

[darius]

patch that keep order of targets in trim_joins

[Anssi Kääriäinen]

Can you create a test for this in Django's test suite? See tests/foreign_object/ directory for some similar tests.

[darius]

Replying to akaariai:

Can you create a test for this in Django's test suite? See tests/foreign_object/ directory for some similar tests.

I just forked the django repo (hope it was what you expected) and wrote down my tests in tests/foreign_object/.

the test test_very_deep_optimized_forward fail without the patch, and after patch, all tests pass on my computer.

all is on my branch ​https://github.com/ornoone/django/tree/%2326515_trimjoin_bug

[Tim Graham]

Could you please send a pull request against master with those changes? See our PatchReviewChecklist.

[darius]

done right now.

​https://github.com/django/django/pull/6473

thanks

[Tim Graham]

Left comments for cosmetic improvements on the PR.

[Tim Graham <timograham@…>]

In a7ad473a:

Fixed #26515 -- Fixed Query.trim_joins() for nested ForeignObjects.