Code

Opened 2 years ago

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

Download all attachments as: .zip

Change History (7)

Changed 2 years ago by tswicegood

comment:1 Changed 2 years ago by tswicegood

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

comment:2 Changed 2 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 2 years ago by tswicegood

Tweak based on feedback

comment:3 Changed 2 years ago by tswicegood

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

comment:4 Changed 2 years ago by claudep

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

comment:5 Changed 2 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.