netzkolchose / django-computedfields

Provides autogenerated autoupdated database fields for model methods.
MIT License
94 stars 14 forks source link

Expected behavior? #134

Closed Alex-Sichkar closed 1 year ago

Alex-Sichkar commented 1 year ago

Hey, thank you for the great project. I have some questions about the behavior. I'm using Python 3.11 + Django 4.2.2 + django-computedfields 0.2.3.

class Parent(ComputedFieldsModel):
    @computed(models.IntegerField(default=0), depends=[
        ('child_set', ["qty"])
    ])
    def total(self):
        res = sum([c.qty for c in self.child_set.all()], 0)
        print(f'computed: {res=}')
        return res

class Child(models.Model):
    parent = models.ForeignKey(Parent, on_delete=models.CASCADE)
    qty = models.IntegerField()
In [1]: p = Parent.objects.all()[0]

In [2]: c = p.child_set.all()[0]

In [3]: c.save()  # Computed twice
computed: res=0
computed: res=0

In [4]: c.qty += 1
In [5]: c.save()  # Computed once (just as I expected)
computed: res=1

In [6]: c.save(update_fields=['qty'])  # Computed once
computed: res=1

In [7]: c.save()  # Computed once, but should have to? If so, is there a way to avoid that?
computed: res=1
jerch commented 1 year ago

Regarding your o.save question - this cannot be avoided for empty update_fields, as the resolver does not track field changes of memory loaded instances. Means if you avoid update_fields in arguments, it will recalc all cfs depending on fields of that model instance. With given update_fields the resolver would narrow to fields of interest though.

What I wonder currently - why is on c.save() in your shell test the field computed twice? Should normally not be the case, unless there are other cf connections not shown in your snippet above.

Alex-Sichkar commented 1 year ago

@jerch,

Thanks for the response.

What I wonder currently - why is on c.save() in your shell test the field computed twice?

Maybe it's because there was a second shell opened, and the signals were registered twice. I will check again. But why, when update_fields is passed, is it computed only once then...

jerch commented 1 year ago

Oh nevermind, that double calc is to be expected. Thats because it is bound via an fk relation, here a plain save causes the pre_save signal to collect the old parent instance, and compares it with the maybe new parent instance in post_save. This is needed to spot a child moving to a different parent, as on a move both old and new parent need to be updated. Again thats caused from the plain save call, as parent might have changed.

Overall I did not put much effort into avoiding recalcs on python side, the resolver mostly tries to avoid issuing update calls to the database, as thats the most time consuming part. Also for that reason recalcs should be side effect free / re-entrant. (Maybe I should not that in the docs :blush:)

jerch commented 1 year ago

Closing the issue - feel free to reopen if anything is left unanswered.