Opened 3 years ago

Last modified 3 years ago

#18597 new Bug

`BaseInlineFormSet` should attempt to get it's queryset from it's instance related manager before falling back to it's model's default manager

Reported by: charettes Owned by: nobody
Component: Forms Version: 1.4
Severity: Normal Keywords: model formset inline
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by aaugustin)

The newly introduced prefetch_related method can be quite handy to avoid unnecessary queries (thanks Luke :), however it's quite useless when used with BaseInlineFormSet since it won't even try to get its needed `queryset` from the related manager.

i.e.

from django.db import models

class Author(models.Model):
    name = models.CharField(max_length=255)

class Book(models.Model):
    name = models.CharField(max_length=255)
    author = models.ForeignKey(Author, related_name='books')
    
    class Meta:
        # Needed because `BaseModelFormSet.get_query_set` wants an ordered set or issue another query
        ordering = ('pk',)
In [1]: from django.conf import settings

In [2]: from django.db import connection

In [3]: from library.models import *

In [4]: from django.forms.models import inlineformset_factory

In [5]: a = Author.objects.create(name='David Abram')

In [6]: b1 = Book.objects.create(name='Becoming Animal', author=a)

In [7]: b2 = Book.objects.create(name='The Spell of the Sensuous', author=a)

In [8]: BookInlineFormSet = inlineformset_factory(Author, Book)

In [9]: settings.DEBUG = True

In [10]: instance = Author.objects.prefetch_related('books').get()

In [11]: BookInlineFormSet(instance=instance)
Out[11]: <django.forms.formsets.BookFormFormSet at 0x3c68d50>

In [12]: print connection.queries
[{u'time': u'0.000', u'sql': u'SELECT "library_author"."id", "library_author"."name" FROM "library_author"'},
 {u'time': u'0.000', u'sql': u'SELECT "library_book"."id", "library_book"."name", "library_book"."author_id" FROM "library_book" WHERE "library_book"."author_id" IN (1) ORDER BY "library_book"."id" ASC'},
 {u'time': u'0.000', u'sql': u'SELECT "library_book"."id", "library_book"."name", "library_book"."author_id" FROM "library_book" WHERE "library_book"."author_id" = 1  ORDER BY "library_book"."id" ASC'}]

I guess it's only a matter of time before people start trying to optimize their formsets (or their ModelAdmin by overriding get_queryset) and stumble on this limitation.

I've written a patch (I'll create a pull request) for which I'll write tests if this optimization is Accepted.

Attachments (1)

formset-optimisation.diff (1.8 KB) - added by charettes 3 years ago.
ticket-18597.0.diff

Download all attachments as: .zip

Change History (4)

Changed 3 years ago by charettes

ticket-18597.0.diff

comment:1 Changed 3 years ago by charettes

  • Has patch set
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset

All tests pass on Python 2.7.3rc1 Postgres and Sqlite.

Last edited 3 years ago by charettes (previous) (diff)

comment:2 Changed 3 years ago by aaugustin

  • Description modified (diff)

comment:3 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Cleanup/optimization to Bug

I'm not sold on the patch -- it looks suspiciously complex -- but the problem is real.

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