Opened 3 years ago

Closed 3 years ago

#17594 closed Cleanup/optimization (fixed)

Tweak formset to remove extraneous query

Reported by: tswicegood 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 tswicegood 3 years ago.
formset-tweak-109-v2.patch (1.4 KB) - added by tswicegood 3 years ago.
Tweak based on feedback

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by tswicegood

comment:1 Changed 3 years ago by tswicegood

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by claudep

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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

Changed 3 years ago by tswicegood

Tweak based on feedback

comment:3 Changed 3 years ago by tswicegood

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

comment:4 Changed 3 years ago by claudep

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

comment:5 Changed 3 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

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