schinckel / django-computed-field

ComputedField() for django
MIT License
26 stars 2 forks source link

What about ForeignKey and ManyToMany fields? #2

Open Bobsans opened 5 years ago

schinckel commented 5 years ago

Pretty sure that they work: you just need to wrap the reference in an expression of some sort: you can't just use ComputedField(F('foo__bar'))

schinckel commented 5 years ago

Lookups over M2M fields are a bit harder: you have possibly multiple records that you are referring to, so I'm not sure how that would work.

I guess you could return and array of values.

schinckel commented 5 years ago

...but that might limit it to postgres.

schinckel commented 5 years ago

I think maybe ManyToMany may be non-trivial. Requiring the user to use an Array constructer on a subquery might work though.

Bobsans commented 5 years ago

I want to do this thing:

class Phone(models.Model):
    number = models.CharField()
    use_count = ComputedField(Count('records'))

class Record(models.Model):
    phones = models.ManyToManyField(Phone, related_name='records')

but it does not work.

Although if you do (without use_count field in model)

Phone.objects.annotate(use_count=Count('records')).all()

everything works fine.

Bobsans commented 5 years ago

And here is DB queries:

When using annotate:

Phone.objects.annotate(count=Count('records'))[:5]
SELECT
    "phones_phone"."id",
    COUNT("records_record_phones"."record_id") AS "count"
FROM "phones_phone"
    LEFT OUTER JOIN "records_record_phones" ON ("phones_phone"."id" = "records_record_phones"."phone_id")
GROUP BY "phones_phone"."id"
LIMIT 5;

and when using ComputedField:

Phone.objects.all()[:5]
SELECT
    "phones_phone"."id",
    COUNT("records_record_phones"."id")
FROM "phones_phone"
    LEFT OUTER JOIN "records_record_phones" ON ("phones_phone"."id" = "records_record_phones"."phone_id")
LIMIT 5;

It no have GROUP BY and wrong field in count.

schinckel commented 5 years ago

Ah, that's a ComputedField of an aggregate.

I'll try to come up with a mechanism: it's possible a Subquery could work.

Bobsans commented 5 years ago

But idea of this module very cool) Thank you.

schinckel commented 5 years ago

Ah, it looks to be doing an .aggregate() like call, when it should be an .annotate().

schinckel commented 5 years ago

When you do a Phone.objects.get(pk=pk), it will give you the correct answer. But that's because there's only one.

Bobsans commented 5 years ago

It's work, but query.group_by not always tuple. And next code work correctly:

class Phone(models.Model):
    number = models.CharField()
    verified = models.BooleanField()
    use_count = ComputedField(Count('records'))

class Record(models.Model):
    phones = models.ManyToManyField(Phone, related_name='records')
    has_verified_phones = ComputedField(Count(Case(When(phones__verified=True, then=1), output_field=BooleanField())))

records = list(Record.objects.all()[:10])

But if add custom annotate (it's just an example:)):

records = list(Record.objects.annotate(has_not_verified=Count(Case(When(phones__verified=False, then=1), output_field=BooleanField()))).all()[:10])

next code raise exception https://github.com/schinckel/django-computed-field/blob/7bcaaa9d42139f3abe78c551c12b7f3019b6a069/src/computed_field/fields.py#L87-L92

    query.group_by += (self.model._meta.pk.name,)
TypeError: unsupported operand type(s) for +=: 'bool' and 'tuple'