django-easy-audit icon indicating copy to clipboard operation
django-easy-audit copied to clipboard

Many duplicate queries saving formset

Open andyp05 opened this issue 5 years ago • 1 comments

I was saving a relatively small formset and the following query was run over 300 times.

user = get_user_model().objects.get(pk=user.pk)

Is there a reason that it is called in pre_save() and post_save() for every call?

The user is being set in the request that is being processed. Would it be better to cache the user per request so it is only queried once? Am I missing something important?

Thanks

andyp05 avatar Aug 24 '20 20:08 andyp05

Hello. Thank you for opening the issue.

We're open to making improvements to this flow.

I am unsure (without reviewing the code more closely), why it's needed in both the pre & post save steps. If I remember correctly, pre-save handles the delta computation. If I don't reply back within 2 weeks (I am on PTO w/o laptop for part of that time window) with some more concrete ideas/answers, please ping me again.

Regarding the number of queries, you say it's a small formset. That to me implies that there are actually many objects in the form (or an iterable of objects), and so without more information, I cannot yet provide validation of your claim that it's too many queries. It's probably too many queries, but if you can provide more of the context/scenario, that would be helpful.

I suppose there may (?) be the possibility that by the time the post-save is called, the user no longer exists. I haven't proven if that is even possible; but it seems possible that in a transaction (or even if not), the pre-save has a user, but some signal logic before our post-save runs, has deleted the user. I'm not necessarily suggesting that we actually consider that edge case, but I'd be remiss if I didn't consider if it was possible, considering that this library is auditing database activity, and being correct 100% of the time (even if inefficient), is probably better than 90% and efficient. I'm not going to make this a politic or scare tactic -- just where my thoughts lead.

So, let's discuss it more, and see if we can improve this!

jheld avatar Aug 25 '20 13:08 jheld