Opened 5 years ago

Closed 5 years ago

#17594 closed Cleanup/optimization (fixed)

Tweak formset to remove extraneous query

Reported by: Travis Swicegood Owned by: nobody
Component: Forms Version: 1.4-alpha-1
Severity: Normal Keywords: modelformset,
Cc: 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

A ModelFormset.save runs a SELECT due to its reliance on the if not self.get_queryset(): line. Immediately after that call, it loops over self.initial_forms and returns values set it that. In the instance where get_queryset() is empty, self.initial_forms is also empty as far as I can tell.

This patch changes the if not self.get_queryset() to an if not self.initial_forms so the extra select is not run. I've added tests that verify the behavior and have run the entire test suite. Everything continues to pass so I believe this is a safe patch to apply, but it definitely needs a second opinion of someone more familiar with model formsets than I.

Attachments (2)

formset-tweak-109-v1.patch (2.9 KB) - added by Travis Swicegood 5 years ago.
formset-tweak-109-v2.patch (1.4 KB) - added by Travis Swicegood 5 years ago.
Tweak based on feedback

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by Travis Swicegood

Attachment: formset-tweak-109-v1.patch added

comment:1 Changed 5 years ago by Travis Swicegood

comment:2 Changed 5 years ago by Claude Paroz

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Looks fine. However I would place the test in model_formsets_regress (class FormsetTests), where an existing model can also be reused.

Changed 5 years ago by Travis Swicegood

Attachment: formset-tweak-109-v2.patch added

Tweak based on feedback

comment:3 Changed 5 years ago by Travis Swicegood

@claudep: Adjusted. All tests continue to pass and the patch is smaller.

comment:4 Changed 5 years ago by Claude Paroz

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:5 Changed 5 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [17434]:

Fixed #17594 -- Stopped ModelFormset.save from running a SELECT query by relying on the fact that the initial form is already set. Thanks, tswicegood.

Note: See TracTickets for help on using tickets.
Back to Top