Opened 8 years ago

Last modified 8 years ago

#28101 closed Bug

Wrong field used for sub query lookup on nested query using to_field ForeignKey — at Version 1

Reported by: Kristian Klette Owned by: nobody
Component: Database layer (models, ORM) Version: 1.11
Severity: Release blocker Keywords:
Cc: stein.magnus@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Kristian Klette)

Given the following models:

class CustomerUser(models.Model):
    name = models.CharField(max_length=100)


class Customer(models.Model):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    customer_number = models.CharField(unique=True, max_length=100)
    users = models.ManyToManyField(CustomerUser)


class CustomerProduct(models.Model):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    customer = models.ForeignKey(
        Customer, to_field='customer_number', on_delete=models.PROTECT)
    name = models.CharField(max_length=100)


class CustomerPayment(models.Model):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
    product = models.ForeignKey(CustomerProduct, on_delete=models.PROTECT)

And the tests:

class SubQueryM2MWithToFieldFkTests(TestCase):
    def test_regression(self):
        user = CustomerUser.objects.create(name='my user')
        customer = Customer.objects.create(customer_number='A123')
        customer.users.add(user)
        product = CustomerProduct.objects.create(customer=customer, name='Foo')
        payment = CustomerPayment.objects.create(product=product)

        products = CustomerProduct.objects.filter(
            customer__in=user.customer_set.all())

        result = CustomerPayment.objects.filter(product__in=products)
        self.assertEquals(result.count(), 1)
        self.assertEquals(list(result), [payment])

One should get the query:

SELECT "queries_customerpayment"."id", "queries_customerpayment"."product_id" 
FROM "queries_customerpayment" 
WHERE "queries_customerpayment"."product_id" IN (
  SELECT V0."id" AS Col1 FROM "queries_customerproduct" V0 
  WHERE V0."customer_id" IN (
    SELECT U0."customer_number" AS Col1 
    FROM "queries_customer" U0 INNER JOIN "queries_customer_users" U1 ON (U0."id" = U1."customer_id") 
    WHERE U1."customeruser_id" = 1))

But at least 1.11 and master generates:

SELECT "queries_customerpayment"."id", "queries_customerpayment"."product_id" 
FROM "queries_customerpayment" 
WHERE "queries_customerpayment"."product_id" IN (
   SELECT V0."id" AS Col1 FROM "queries_customerproduct" V0 
   WHERE V0."customer_id" IN (
      SELECT U0."id" AS Col1 FROM "queries_customer" U0 INNER JOIN "queries_customer_users" U1 ON (U0."id" = U1."customer_id")
      WHERE U1."customeruser_id" = 1))

which attempts to match the wrong Customer-field to CustomerProduct.customer_id.

I tried to figure out what was going on and the test passes if I add print(self) to Queryset._prepare_as_filter_value in the forced pk block. Quite arbitrary placement of the print, and probably not related to _prepare_as_filter_value. My guess would be that the evaluation of the inner queryset fixes some field matching some other place - but I'm way out of my depth here :)

The tests were added directly to tests/queries/tests.py and the models to tests/queries/models.py and run using Django's test suite.

Might be related: https://code.djangoproject.com/ticket/26196

Running git bisect found the following change as the first bad one, so seems like a regression.

7a2c27112d1f804f75191e9bf45a96a89318a684 is the first bad commit
commit 7a2c27112d1f804f75191e9bf45a96a89318a684
Author: Jani Tiainen <jani.tiainen@tintti.net>
Date:   Wed Aug 31 21:16:39 2016 +0300

    Fixed #27159 -- Prevented pickling a query with an __in=inner_qs lookup from evaluating inner_qs.

Change History (1)

comment:1 by Kristian Klette, 8 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top