[Django] #26565: Allow Prefetch query to returns aggregations

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #26565: Allow Prefetch query to returns aggregations

Django
#26565: Allow Prefetch query to returns aggregations
----------------------------------------------+----------------------------
     Reporter:  mlorant                       |      Owner:  nobody
         Type:  Bug                           |     Status:  new
    Component:  Database layer (models, ORM)  |    Version:  1.9
     Severity:  Normal                        |   Keywords:  prefetch,
                                              |  values
 Triage Stage:  Unreviewed                    |  Has patch:  0
Easy pickings:  0                             |      UI/UX:  0
----------------------------------------------+----------------------------
 I came across a use case that does not seems to be possible using the
 Django ORM on Django 1.9.

 Let's says I want to get all objects from a `Order` model and prefetch for
 each order all pay lines, grouped per code, to get a list similar to:
 `[{'code': 'A', 'number': 3, 'amount': 1000}, {'code': 'B', 'number': 1,
 'amount': 5000}]` if the order has four PayRollLine associated, with 3
 where code=A and 1 where code=B.

 {{{
 # Query that will be used inside the `Prefetch` object later
 # Let's assume quantity and unit_amount can't be null in the query
 # to avoid extra Coalesce...
 prefetch_qs = (
     PayrollLine.objects.values('code').annotate(
         number=ExpressionWrapper(
             Sum(F('quantity')), output_field=DecimalField()
         ),
         amount=ExpressionWrapper(
             Sum(F('unit_amount') * F('quantity')),
 output_field=DecimalField()
         ),
     )
 )
 print(
     Order.objects.all()
     .prefetch_related(
         Prefetch('pay_lines', queryset=prefetch_qs,
 to_attr='pay_lines_grouped')
     )
 )
 }}}

 The first query works fine alone, outside the prefetch object. But, when
 executing the second query, Django raises an exception:

 {{{
     Traceback (most recent call last):
       [...]
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/query.py", line 234, in __repr__
         data = list(self[:REPR_OUTPUT_SIZE + 1])
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/query.py", line 258, in __iter__
         self._fetch_all()
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/query.py", line 1076, in _fetch_all
         self._prefetch_related_objects()
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/query.py", line 656, in
 _prefetch_related_objects
         prefetch_related_objects(self._result_cache,
 self._prefetch_related_lookups)
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/query.py", line 1457, in
 prefetch_related_objects
         obj_list, additional_lookups = prefetch_one_level(obj_list,
 prefetcher, lookup, level)
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/query.py", line 1556, in prefetch_one_level
         prefetcher.get_prefetch_queryset(instances,
 lookup.get_current_queryset(level)))
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/fields/related_descriptors.py", line 544, in
 get_prefetch_queryset
         instance = instances_dict[rel_obj_attr(rel_obj)]
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/fields/related.py", line 597, in
 get_local_related_value
         return self.get_instance_value_for_fields(instance,
 self.local_related_fields)
       File "/data/.virtualenvs/ORFEO/local/lib/python2.7/site-
 packages/django/db/models/fields/related.py", line 605, in
 get_instance_value_for_fields
         opts = instance._meta
     AttributeError: 'dict' object has no attribute '_meta'
 }}}

 Looks like Django can't work with dict in prefetch_related... On IRC,
 someone point me to [this PR](https://github.com/django/django/pull/6478)
 that could resolve my problem but I think it should be verified with unit
 tests.

--
Ticket URL: <https://code.djangoproject.com/ticket/26565>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/050.275dd503dcd42166eb62ebea25af54a9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values() (was: Allow Prefetch query to returns aggregations)

Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by charettes):

 * needs_better_patch:   => 0
 * version:  1.9 => master
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 I doubt this is related to the linked PR or aggregation. I believe
 `Prefetch.queryset` cannot be used with `values()` queryset.

 Please confirm you get a similar crash with the following queryset:

 {{{#!python
 queryset = PayrollLine.objects.values('code')
 Order.objects.prefetch_related(
     Prefetch('pay_lines', queryset=queryset, to_attr='pay_lines_values')
 )
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.35a4bca35d751568baa28a9fd6d045ad%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by mlorant):

 Yes, same error & traceback using `PayrollLine.objects.values('code')`
 only.

 The error seems obvious: the prefetch mechanism tries to access to
 attributes for some reason, but it got a dict from the query, not model
 instances...

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.29114b7aae9d5593899df815ab293c91%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by charettes):

 * stage:  Unreviewed => Accepted


Comment:

 I'm not sure if this should be considered a ''New Feature'' or a ''Bug''.

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.9af577a7b904453977fd0ca42f31ba9f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by akaariai):

 I'd say there is a bug and a new feature. We should error nicely on values
 queries, and of course, ability to prefetch values queries would be a nice
 feature for aggregations specifically.

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.bcf140d81a87e0b2fa5d41ffeffad807%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by charettes):

 * has_patch:  0 => 1


Comment:

 Submitted a [https://github.com/django/django/pull/6545 PR] for the
 ''bug'' part. This should be converted to a feature request once it's
 merged.

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.a2a47d80501691c4ccabec6c54101441%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Simon Charette <charettes@…>):

 In [changeset:"7ec330eeb96d0874949eacb8ed1bbb97e11807e1" 7ec330e]:
 {{{
 #!CommitTicketReference repository=""
 revision="7ec330eeb96d0874949eacb8ed1bbb97e11807e1"
 Refs #26565 -- Errored nicely when using Prefetch with a values()
 queryset.

 Thanks Maxime Lorant for the report and Anssi for the suggestion.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.f402d796491901f99a2c4ed4606fd291%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by charettes):

 * type:  Bug => New feature


--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.f12522e9194a1d21af3ebbe820ffbe3d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by charettes):

 * has_patch:  1 => 0


--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:8>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.304ee55ea59d9579a168178cede77301%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  mlorant              |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Gwildor):

 Just to add, it would be great if not only `.values()` would be supported,
 but also `.values_list(flat=True)`.

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.492be72031da38a4b362d86310bd5a85%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  Maxime Lorant        |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Alexander Schepanovski):

 Anyone on this? Would be great to have.

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.732ad810950e3008e1dd6825b8e9874a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  Maxime Lorant        |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

 No one has claimed it yet Alexander, feel free to propose a patch!

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.9c66bc21145738f7f39925b74ac72bb6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  Maxime Lorant        |                    Owner:
                                     |  skrilladeville
         Type:  New feature          |                   Status:  assigned
    Component:  Migrations           |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values,    |             Triage Stage:  Accepted
  payroll, pay                       |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by skrilladeville):

 * keywords:  prefetch, values => prefetch, values, payroll, pay
 * owner:  nobody => skrilladeville
 * status:  new => assigned
 * component:  Database layer (models, ORM) => Migrations


--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:12>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.02116c74ff37a1e3479edbb481524143%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  Maxime Lorant        |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Dan LaManna):

 * cc: Dan LaManna (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:12>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.bf0774a6e163e40338bc078982977e2d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  Maxime Lorant        |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Micah Lyle):

 * cc: Micah Lyle (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:13>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.d0c387aac55787ee0623d2eaed16a7cf%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #26565: Allow Prefetch query to use .values()

Django
In reply to this post by Django
#26565: Allow Prefetch query to use .values()
-------------------------------------+-------------------------------------
     Reporter:  Maxime Lorant        |                    Owner:  nobody
         Type:  New feature          |                   Status:  new
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:  prefetch, values     |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Micah Lyle):

 I took a stab at this during the DjangoCon 2019 sprints.

 I had very limited experience with the prefetching/ORM internals before
 this, so there could be workarounds/solutions that I'm unaware of, but
 here's what I found so far.

 One thing I tried to tackle is handling the many to many case with just
 `values()`. My goal was to take the `'_prefetch_related_val_%s'` in the
 `queryset.extra(...)` call in the `get_prefetch_queryset()`
 `ManyRelatedManager` and add/apply it to whatever `values()` call was made
 and then join that by doing `pop()` from the dictionary instead of a
 `getattr` call (which would effectively give the developer the same query
 that they originally made by modifying it under the hood). I ran into some
 issues doing this though:

     "Any `extra()` call made after a `values()` call will have its extra
 selected fields ignored" (from the Django 2.2 docs)."

 This means that the `queryset.extra(...)` call in the `ManyRelatedManager`
 (which from my quick search appears to be the only remaining usage of
 `extra(...)` across the entire Django codebase (except the tests)) is
 effectively ignored, and even if I try to add a `.values(*existing_values,
 '_prefetch_related_val_%s')` call, it returns this error (from my example
 test/use case using the models from the prefetch tests):

 {{{
 queryset.values('id', 'title', '_prefetch_related_val_author_id')

 django.core.exceptions.FieldError: Cannot resolve keyword
 '_prefetch_related_val_author_id' into field. Choices are:
 _prefetch_related_val_author_id, authors, bio, bookwithyear,
 first_time_authors, id, read_by, title
 }}}

 Is this a bug? It says it can't resolve a field and then says that field
 is a choice.

 I thought about trying to workaround it by essentially "undoing" the
 `values()` part of the `queryset` and then reapplying it at the end with
 the value from the join table (which then gets `pop`ed, but I'm not sure
 if that's feasible/doable and what that would actually look like. Besides,
 this entire solution so far feels somewhat hacky in the first place.

 I think that whatever change makes this happen may require some sort of
 specification on the developer's end for how to apply the join from the
 prefetched dictionaries to the existing list of instances. Maybe even
 writing parts of a custom prefetcher and specifying that custom prefetcher
 as a fourth keyword argument (something I explored, but didn't get very
 far) to the `Prefetch` object, and then tweaking the logic in
 `prefetch_related_objects` and other functions to support using that
 fourth keyword argument.

 I'm also wondering if there's a way with Django 3.1+ technology/API to
 rewrite the `queryset.extra(...)` call in `ManyRelatedManager`. An out
 there idea I had was replacing the `queryset.extra(...)` call with a query
 on the through table and then doing a select related to the foreign key
 that is on the side we want to bring in to join to the existing instances
 (in the case above, we'd grab
 `Book.authors.through.objects.filter_based_on_what_we_have_for_books_and_authors().select_related('book')`
 and then transform to a values result from there but that would (I think)
 prevent aggregations in the first place and run into a number of other
 problems.

--
Ticket URL: <https://code.djangoproject.com/ticket/26565#comment:14>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.bc27cd3ad77e6c908b55310b13b74907%40djangoproject.com.