N+1 query in BlogIndexView

pjmcdermott commented 6 years ago

I am using jmcarp/nplusone to detect issues with N+1 queries in my current project. It is flagging an issue with the BlogIndexView.

WARNING notifiers Potential n+1 query detected on `Post.blog`
WARNING notifiers Potential n+1 query detected on `Post.section`
WARNING notifiers Potential n+1 query detected on `Post.blog`
WARNING notifiers Potential n+1 query detected on `Post.section`

This is suggesting that properties of Post.blog and Post.section are being used in constructing the view.

I can indeed see references to post.section.name & post.section.slug in dateline.html which is called for every blog post. (I can't find any references to properties of Post.blog, but I suspect they may be in code rather than templates, and tied up with this "scoper" mechanism).

Looking in pinx/blog/views.py I can see that BlogIndexView has a get_queryset() method:

    def get_queryset(self):
        blog = hookset.get_blog(**self.kwargs)
        qs = Post.objects.current().filter(blog=blog)
        return self.search(qs)

I believe a select_related('section', 'blog') should be appended to the queryset in order to prevent N+1 issues with calls to this index page.

It would be useful if someone could confirm explicitly that properties of Post.blog are being accessed in this view somewhere.

pjmcdermott commented 6 years ago

I have found the following patch on pinax/blog/views.py resolves any complaints about N+1 issues:

*** views.py    2017-12-15 21:45:44.593779826 +0000
--- views.py.orig   2017-12-15 21:49:14.571454478 +0000
***************
*** 58,64 ****

      def get_queryset(self):
          blog = hookset.get_blog(**self.kwargs)
!         qs = Post.objects.current().filter(blog=blog).select_related('section', 'blog')
          return self.search(qs)

--- 58,64 ----

      def get_queryset(self):
          blog = hookset.get_blog(**self.kwargs)
!         qs = Post.objects.current().filter(blog=blog)
          return self.search(qs)

***************
*** 165,171 ****
      scoper_lookup = kwargs.get(settings.PINAX_BLOG_SCOPING_URL_VAR, None)

      blog = hookset.get_blog(**kwargs)
!     posts = Post.objects.published().filter(blog=blog).order_by("-updated").select_related('author', 'blog')

      blog_url_kwargs = {}
      if scoper_lookup is not None:
--- 165,171 ----
      scoper_lookup = kwargs.get(settings.PINAX_BLOG_SCOPING_URL_VAR, None)

      blog = hookset.get_blog(**kwargs)
!     posts = Post.objects.published().filter(blog=blog).order_by("-updated")

      blog_url_kwargs = {}
      if scoper_lookup is not None:

I would do a pull request by I have never found this push/pull/fork/clone business straightforward!

paltman commented 6 years ago

@pjmcdermott great find! Thanks for that!

grahamu commented 6 years ago

I added this patch in https://github.com/pinax/pinax-blog/pull/108. Thanks @pjmcdermott .

pjmcdermott commented 6 years ago

I found I had to change 'section' to 'author' in the above once I switched to pinax==7.0.4. Don't understand why. Has there been a change in the model?

pinax / pinax-blog

N+1 query in BlogIndexView #104