[Django] #28838: annotations + base_manager_name + instance.save() raises exception

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
               Reporter:  James      |          Owner:  nobody
  Addison                            |
                   Type:  Bug        |         Status:  new
              Component:  Database   |        Version:  1.11
  layer (models, ORM)                |
               Severity:  Normal     |       Keywords:
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 With a models.py like:
 {{{#!python
 class ItemManager(models.Manager):
     def get_queryset(self):
         return super().get_queryset().annotate(
             rating_avg=models.Avg('review__rating')
         )


 class Item(models.Model):
     name = models.CharField(max_length=10, default='')

     objects = ItemManager()

     class Meta:
         base_manager_name = 'objects'


 class Review(models.Model):
     item = models.ForeignKey(Item)
     rating = models.PositiveSmallIntegerField(default=0)
 }}}

 If an `Item` instance is modified and `item.save()` is called like this:
 {{{
     item = Item.objects.first()
     item.name = 'abc'
     item.save()
 }}}
 It will raise this exception:
 {{{
 Traceback (most recent call last):
   File "/home/myuser/django_annotations_bug/the_bug/tests.py", line 9, in
 test_bug_will_be_triggered
     item.save()
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/models/base.py", line 808, in save
     force_update=force_update, update_fields=update_fields)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/models/base.py", line 838, in save_base
     updated = self._save_table(raw, cls, force_insert, force_update,
 using, update_fields)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/models/base.py", line 905, in _save_table
     forced_update)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/models/base.py", line 955, in _do_update
     return filtered._update(values) > 0
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/models/query.py", line 664, in _update
     return query.get_compiler(self.db).execute_sql(CURSOR)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/models/sql/compiler.py", line 1199, in
 execute_sql
     cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/models/sql/compiler.py", line 894, in execute_sql
     raise original_exception
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/models/sql/compiler.py", line 884, in execute_sql
     cursor.execute(sql, params)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/backends/utils.py", line 65, in execute
     return self.cursor.execute(sql, params)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/utils.py", line 94, in __exit__
     six.reraise(dj_exc_type, dj_exc_value, traceback)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/utils/six.py", line 685, in reraise
     raise value.with_traceback(tb)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/backends/utils.py", line 65, in execute
     return self.cursor.execute(sql, params)
   File "/home/myuser/.virtualenvs/django_annotations_bug/lib/python3.6
 /site-packages/django/db/backends/sqlite3/base.py", line 328, in execute
     return Database.Cursor.execute(self, query, params)
 django.db.utils.OperationalError: sub-select returns 2 columns - expected
 1
 }}}

 I have attached a simple sample project that reproduces this error. To set
 it up:
 {{{
 tar xzvf bug.tgz
 cd django_annotations_bug
 mkvirtualenv -ppython3 buggy_mcbugface
 pip install -r requirements.txt
 ./manage.py test
 }}}

 Related to #19513, #28374, #26539 - this those are related to `.update()`
 calls, mostly. This example does not use the annotation in the
 save/updating of the item, so is different I think.

--
Ticket URL: <https://code.djangoproject.com/ticket/28838>
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/051.93768f32d4e44bb0657640b2e9ca1531%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by James Addison):

 * Attachment "bug.tgz" added.

 sample project to reproduce bug

--
Ticket URL: <https://code.djangoproject.com/ticket/28838>
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/066.47ae35f682db0c7a6a23b0a9edf7bee4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
In reply to this post by Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by James Addison):

 Adding this hack to the `Item` model addresses the issue for now. Yes, I
 realize this is overriding an undocumented, internal Django `Model` method
 and is thus fragile to future changes - but it's a hack that lets me
 proceed for now and hopefully helps the Django team.
 {{{
 class Item(models.Model):
     ....
     def _do_update(self, base_qs, *args, **kwargs):
         qs = base_qs.all()
         qs.query.annotations.clear()
         return super()._do_update(qs, *args, **kwargs)
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28838#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/066.49366ddee3e498e2b90832854c43eee4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
In reply to this post by Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * stage:  Unreviewed => Accepted


Comment:

 It looks like this was also reported on the GitHub page of
 a84344bc539c66589c8d4fe30c6ceaecf8ba1af3.

--
Ticket URL: <https://code.djangoproject.com/ticket/28838#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/066.7ca213d6506d93462080f3c538914d39%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
In reply to this post by Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:
                                     |  shangdahao
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by shangdahao):

 * status:  new => assigned
 * owner:  nobody => shangdahao


--
Ticket URL: <https://code.djangoproject.com/ticket/28838#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/066.1fd8f0c7e9eef4899cb82294d499b178%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
In reply to this post by Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:
                                     |  shangdahao
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by shangdahao):

 * has_patch:  0 => 1


Comment:

 [https://github.com/django/django/pull/9587 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/28838#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/066.72dc6d59375525442a0b6dfa123f951a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
In reply to this post by Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:
                                     |  shangdahao
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * needs_better_patch:  0 => 1


Comment:

 Comments on
 [https://github.com/django/django/pull/9587#pullrequestreview-89406005 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/28838#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/066.60748262f7ad7267d1158501b0ea11a7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
In reply to this post by Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:
                                     |  shangdahao
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by shangdahao):

 * needs_better_patch:  1 => 0


--
Ticket URL: <https://code.djangoproject.com/ticket/28838#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/066.47307273beb84188b78da45ef81b7ac4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
In reply to this post by Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:
                                     |  shangdahao
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:  Ready for
                                     |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * stage:  Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/28838#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/066.7f57d239a6d468d7bf5e588bf49be6d9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #28838: annotations + base_manager_name + instance.save() raises exception

Django
In reply to this post by Django
#28838: annotations + base_manager_name + instance.save() raises exception
-------------------------------------+-------------------------------------
     Reporter:  James Addison        |                    Owner:
                                     |  shangdahao
         Type:  Bug                  |                   Status:  closed
    Component:  Database layer       |                  Version:  1.11
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:  fixed
     Keywords:                       |             Triage Stage:  Ready for
                                     |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

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


Comment:

 In [changeset:"8dc675d90f14a84ef95f16c7cc8100d9a04459b3" 8dc675d]:
 {{{
 #!CommitTicketReference repository=""
 revision="8dc675d90f14a84ef95f16c7cc8100d9a04459b3"
 Fixed #28838 -- Fixed Model.save() crash if the base manager annotates
 with a related field.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28838#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/066.bd7e1e7e613e4e9c99d0ee75d57a5079%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.