Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  lukeplant
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Release blocker  |   Resolution:  fixed
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by lukeplant):

 * status:  new => closed
 * resolution:   => fixed


Comment:

 In [17600]:
 {{{
 #!CommitTicketReference repository="" revision="17600"
 Fixed #17668 - prefetch_related does not work in in_bulk

 Thanks to gurets for the report, and akaariai for the initial patch.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  lukeplant
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Release blocker  |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by lukeplant):

 * type:  Uncategorized => Bug


Comment:

 @akaariai: You haven't wasted anyone's time - your input is really
 appreciated!

 I think we should probably wait until someone asks for a way to check for
 prefetches. I guess the whole `QuerySet` API is a bit lacking in this
 regard - I don't think we have a public API for checking other things like
 select_related and order_by etc. Perhaps there would be an API that covers
 all of those e.g. `get_query_options()` that returns a dictionary of
 things that have been set.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  lukeplant
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Release blocker  |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 I am OK with the above.

 Should there be a way to see if there are prefetches? It would be cheap to
 add a way to see the current prefetches, this would give a chance to skip
 the .iterator() call if there are prefetches defined.

 Or maybe just document the current behavior, fix in_bulk and be done with
 it. If somebody complains, add a way to check the prefetches then. I have
 a feeling I have wasted enough everybody's time on this issue already...

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  lukeplant
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Release blocker  |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by lukeplant):

 * owner:  nobody => lukeplant


Comment:

 I thought I had replied here, but obviously not. Thanks for setting this
 down clearly.

 I favour option 3 for the following reasons:

 `iterator()` by definition always comes last, and therefore is always in
 the code that actually generates the query. It is confusing that your
 iterator() call gets effectively ignored because of something that got
 passed in to the code in front of you, over which you might have little
 control, and I think more confusing than the your `prefetch_related()`
 getting ignored because of something that comes later. There are many ways
 in which `QuerySet` calls can get ignored by something that comes later
 e.g. `order_by()` always works this way, `prefetch_related()` itself works
 this way if you do `prefetch_related(None)`.

 I also don't think that we can correctly judge the potential performance
 loss - there will almost certainly be cases which contradict our
 intuition. We should therefore just go on what is the most obvious API,
 rather than trying to second-guess the developer or the situation.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  nobody
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Release blocker  |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 There are three possible choices for the iterator() + prefetch_related():
   1. Raise error.
   2. Favor prefetch: when iterator is used, fetch all the objects and
 related objects in one go, then yield them one at a time.
   3. Favor iterator: throw out the defined prefetch, and do the iterator
 properly. This is what is currently done.

 I suggested option 1 already, but that wasn't favored. From the rest
 option 2 is in my opinion better than the currently used option 3. Just
 because the potential performance loss is less in that case. You would get
 the reduced memory usage given by .iterator() when it is possible to do
 that. If there is prefetches, they are done correctly, but you lose the
 memory efficiency of iterator(). For example, in_bulk would have worked
 perfectly without any changes to it with option 2.

 Option 2 should be easy to implement, where the error is raised currently,
 do:
 {{{
 for obj in list(self):
 yield obj
 }}}
 I haven't actually tested this though...

 Anyways, which option is chosen isn't __that__ important. I could live
 with any of them.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  nobody
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Release blocker  |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by lukeplant):

 I'm not sure about this solution.

 I agree that in_bulk does not need to use iterator() - it doesn't gain
 much by using it, since it immediately consumes the whole generator, it
 only avoids the creation of a lists. This part of the patch has a serious
 bug though - it should be `list(qs)` not `list(self)`. In fact it can be
 simplified to just: `return dict([(obj._get_pk_val(), obj) for obj in
 qs])` - no need to create a list we will throw away.

 However, making iterator() blow up if prefetch_related() is used is more
 dubious.  If you have a 3rd party library, or just some generic code, that
 uses your model (but is agnostic to your model), and uses `iterator()`,
 this patch will cause it to blow up if you used prefetch_related() on a
 default manager, for example. This patch effectively means that many
 instances of iterator() would need patching to be safe in the general
 case. I don't think people who use iterator() shouldn't have to worry
 about clearing prefetch_related() first - that's a nasty little gotcha
 that will clutter people's code.

 So at the moment I think this should be a documentation fix for
 iterator(). The note should probably go on both iterator() and
 prefetch_related() docs.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  nobody
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Release blocker  |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by gurets):

 Thank you!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  nobody
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Release blocker  |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by julien):

 * severity:  Normal => Release blocker


Comment:

 Thank you both for the report and patch. Marking this ticket as release
 blocker since this is a limitation/bug in a new feature.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  nobody
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Normal   |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by akaariai):

 Ok, I have a patch for this. It is now an error to use .iterator()
 together with .prefetch_related(). in_bulk does not use the iterator any
 more. It first forces the query into a list.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |Owner:  nobody
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:
  (models, ORM)  |  1.4-alpha-1
 Severity:  Normal   |   Resolution:
 Keywords:  prefetch_related,| Triage Stage:  Accepted
  in_bulk|  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by akaariai):

 * cc: anssi.kaariainen@… (added)
 * needs_better_patch:   => 0
 * needs_docs:   => 0
 * needs_tests:   => 0
 * stage:  Unreviewed => Accepted


Comment:

 The problem is anything using the .iterator() of qs will not do the
 prefetching. This should be documented, as there is no possibility to both
 yield object at a time, and do prefetching. Maybe also an assert in the
 iterator about this would be warranted. Another option is to force the
 whole query to be iterated over and then doing the prefetch when calling
 .iterator() and using .prefetch_related() in the same query.

 Marking as accepted. Maybe this should be a release blocker?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

[Django] #17668: prefetch_related does not work in in_bulk

[Django]

#17668: prefetch_related does not work in in_bulk
-+-
 Reporter:  gurets@… |  Owner:  nobody
 Type:  Uncategorized| Status:  new
Component:  Database layer   |Version:  1.4-alpha-1
  (models, ORM)  |   Keywords:  prefetch_related,
 Severity:  Normal   |  in_bulk
 Triage Stage:  Unreviewed   |  Has patch:  0
Easy pickings:  0|  UI/UX:  0
-+-
 Hi,
 When using the in_bulk - prefetch_related does not work.

 For example: in_bulk uses django-haystack. load data from its storage
 (solr, xapian..) and search objects in django db backend.

 qs = Book.objects.all().prefetch_related('authors')

 books = qs.in_bulk([1,2,3])

 books at this time without prefetched authors.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.