Opened 5 years ago

Closed 4 months ago

Last modified 4 months ago

#28897 closed Bug (fixed)

QuerySet.update() raises FieldError on queryset ordered by an annotated field.

Reported by: Colton Hicks Owned by: David Wobrock
Component: Database layer (models, ORM) Version: 4.0
Severity: Normal Keywords: Admin Interface, Custom Action, Annotated Field,
Cc: Saeed Blanchette, David Wobrock Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A FieldError results if I try to perform a custom a custom action on data in the admin interface IF that data is sorted by an annotated field. The action is obviously not acting on an annotated field as those fields do not exist in the database, but raises a FieldError saying it 'Cannot resolve keyword 'field_that_data_is_currently_sorted_by' into field.' and then lists the choices for fields (all of which are the raw model fields, not the annotated fields in the custom queryset).

My admin model:

@admin.register(Position)
class PositionAdmin(admin.ModelAdmin):

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)

    def get_queryset(self, request):

        qs = super(PositionAdmin, self).get_queryset(request).prefetch_related(
            'orders').select_related(
                'portfolio', 'current_position').select_related(
                    'current_position').annotate(
                Count('orders', distinct=True),
                Count('short_requests'),
                total_position_value=ExpressionWrapper(
                    F('price') * F('quantity'), output_field=IntegerField()),
                diff_shares=Coalesce(
                    F('quantity')
                    - F('current_position__quantity'),
                    F('quantity')),
                diff_value=ExpressionWrapper(
                    F('diff_shares') * F('price'),
                    output_field=IntegerField()),
                orders_with_errors=Count(
                    Case(When(~Q(orders__error=''), then=1))),
                non_accepted_orders=Count(
                    Case(When(Q(orders__status=''), then=1))),
                order_today=Count(
                    Case(When(
                        orders__created_at__gte=_brokerage_today_start(),
                        then=1))))
        return qs

    # Custom Action
    def approve_position_for_trading(self, request, queryset):
        try:
            num_pos_approved = queryset.update(approved=True)
        except FieldError:
            self.message_user(
                request, "You cannot perform actions when you have sorted by "
                "this column. Please remove your column sortings and then try "
                "your action again.", level=messages.ERROR)
        else:
            if num_pos_approved == 1:
                message_bit = "1 position was was"
            else:
                message_bit = "%s positions were" % num_pos_approved
            self.message_user(
                request, "%s successfully approved for trading." % message_bit)

I had to write code to handle the error so that the user will know how to proceed. However, this seems like bad behavior. Django can successfully perform that action (changing the 'Approved' field to 'True') on the data if it is sorted by any core model field, or if sorted by no field at all. However, if the data is sorted by a field resulting from the annotation of my queryset, then I get the FieldError when I try to perform the action. This seems like a bug.

Change History (20)

comment:1 Changed 5 years ago by Sergey Fedoseev

Could you provide traceback and try minimize your example?

comment:2 Changed 5 years ago by Sergey Fedoseev

Cc: Sergey Fedoseev added

comment:3 Changed 5 years ago by Tim Graham

Resolution: needsinfo
Status: newclosed

Yes, a more minimal (but complete, with models) example seems needed.

comment:4 Changed 10 months ago by Márton Salomváry

Resolution: needsinfo
Status: closednew
Version: 2.04.0

Hey folks, I've run into this bug both on django 3.x and 4.x. Here is a small showcase project: https://github.com/salomvary/django_admin_action_bug

Steps to reproduce:

  • Run the project
  • Sign in to admin
  • Create a few "Things"
  • Go to Home › My_App › Things and select a Thing using the checkboxes
  • Go to Actions > "Update Something"
  • Click "Go"

You will see the following error page:

FieldError at /admin/my_app/thing/

Cannot resolve keyword 'other_things_count' into field. Choices are: id, is_something, other_things

Few things to note:

  • Removing queryset.order_by eliminates the problem
  • Removing annotations that involve joins also eliminates the problem

Stack trace:

Internal Server Error: /admin/my_app/thing/
Traceback (most recent call last):
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/contrib/admin/options.py", line 622, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/views/decorators/cache.py", line 56, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/contrib/admin/sites.py", line 236, in inner
    return view(request, *args, **kwargs)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/utils/decorators.py", line 43, in _wrapper
    return bound_method(*args, **kwargs)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/contrib/admin/options.py", line 1736, in changelist_view
    response = self.response_action(request, queryset=cl.get_queryset(request))
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/contrib/admin/options.py", line 1417, in response_action
    response = func(self, request, queryset)
  File "/Users/mrc/Projects/django_admin_action_bug/my_app/admin.py", line 9, in update_something
    queryset.update(is_something=True)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/query.py", line 790, in update
    rows = query.get_compiler(self.db).execute_sql(CURSOR)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1591, in execute_sql
    cursor = super().execute_sql(result_type)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1189, in execute_sql
    sql, params = self.as_sql()
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1526, in as_sql
    self.pre_sql_setup()
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1626, in pre_sql_setup
    super().pre_sql_setup()
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 60, in pre_sql_setup
    order_by = self.get_order_by()
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 389, in get_order_by
    for expr, is_ref in self._order_by_pairs():
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 373, in _order_by_pairs
    yield from self.find_ordering_name(
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 767, in find_ordering_name
    field, targets, alias, joins, path, opts, transform_function = self._setup_joins(pieces, opts, alias)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 807, in _setup_joins
    field, targets, opts, joins, path, transform_function = self.query.setup_joins(pieces, opts, alias)
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/query.py", line 1604, in setup_joins
    path, final_field, targets, rest = self.names_to_path(
  File "/Users/mrc/Projects/django_admin_action_bug/.venv/lib/python3.9/site-packages/django/db/models/sql/query.py", line 1522, in names_to_path
    raise FieldError("Cannot resolve keyword '%s' into field. "
django.core.exceptions.FieldError: Cannot resolve keyword 'other_things_count' into field. Choices are: id, is_something, other_things
[09/Dec/2021 17:00:45] "POST /admin/my_app/thing/ HTTP/1.1" 500 154267

comment:5 Changed 10 months ago by Sergey Fedoseev

Cc: Sergey Fedoseev removed

comment:6 Changed 10 months ago by Mariusz Felisiak

Component: contrib.adminDatabase layer (models, ORM)
Summary: FieldError when performing custom action in Admin Interface IF data is sorted by an annotated fieldQuerySet.update() raises FieldError on queryset ordered by an annotated field.
Triage Stage: UnreviewedAccepted

Thanks for details! QuerySet.update() removes all annotations (see a84344bc539c66589c8d4fe30c6ceaecf8ba1af3) that's why order_by() raises a FieldError. We should probably clear ordering in QuerySet.update() or at least remove all annotated fields from it, e.g.

diff --git a/django/db/models/query.py b/django/db/models/query.py
index fb6639793a..1db9de0b5c 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -790,6 +790,7 @@ class QuerySet:
         query = self.query.chain(sql.UpdateQuery)
         query.add_update_values(kwargs)
         # Clear any annotations so that they won't be present in subqueries.
+        query.clear_ordering()
         query.annotations = {}
         with transaction.mark_for_rollback_on_error(using=self.db):
             rows = query.get_compiler(self.db).execute_sql(CURSOR)

or

diff --git a/django/db/models/query.py b/django/db/models/query.py
index fb6639793a..90a0041d66 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -790,6 +790,9 @@ class QuerySet:
         query = self.query.chain(sql.UpdateQuery)
         query.add_update_values(kwargs)
         # Clear any annotations so that they won't be present in subqueries.
+        query.order_by = tuple(
+            col for col in query.order_by if col not in query.annotations
+        )
         query.annotations = {}
         with transaction.mark_for_rollback_on_error(using=self.db):
             rows = query.get_compiler(self.db).execute_sql(CURSOR)

As a workaround you can clear ordering in your actions, e.g.

def update_something(admin, request, queryset):
    queryset.order_by().update(is_something=True)
Version 0, edited 10 months ago by Mariusz Felisiak (next)

comment:7 Changed 10 months ago by Márton Salomváry

Thanks for the workaround, I can confirm it makes the exception go away.

comment:8 Changed 8 months ago by Saeed Blanchette

Hi, guys I'd love to work in this, any suggestions ! I need help to understand this more and what can you do next

comment:9 Changed 8 months ago by Saeed Blanchette

Cc: Saeed Blanchette added

comment:10 Changed 7 months ago by Mariusz Felisiak

Saeed, Have you seen my comment?

comment:11 Changed 4 months ago by David Wobrock

Cc: David Wobrock added
Has patch: set
Owner: changed from nobody to David Wobrock
Status: newassigned

The suggested code seems to fix it indeed :)

I could only reproduce when annotating/ordering on a ManyToManyField.
I used the snippet that keeps some order_by and doesn't clear it entirely ¯\_(ツ)_/¯
PR

comment:12 Changed 4 months ago by David Wobrock

Patch needs improvement: set

comment:13 Changed 4 months ago by David Wobrock

Patch needs improvement: unset

comment:14 Changed 4 months ago by Mariusz Felisiak

Patch needs improvement: set

comment:15 Changed 4 months ago by David Wobrock

Patch needs improvement: unset

comment:16 Changed 4 months ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin

comment:17 Changed 4 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In f4680a11:

Refs #28897 -- Added test for QuerySet.update() on querysets ordered by inline m2m annotation.

comment:18 Changed 4 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 3ef37a5:

Fixed #28897 -- Fixed QuerySet.update() on querysets ordered by annotations.

comment:19 Changed 4 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In ccb24384:

[4.1.x] Refs #28897 -- Added test for QuerySet.update() on querysets ordered by inline m2m annotation.

Backport of f4680a112d01d85540411673eade31f37712d0a6 from main

comment:20 Changed 4 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In d44dc31f:

[4.1.x] Fixed #28897 -- Fixed QuerySet.update() on querysets ordered by annotations.

Backport of 3ef37a5245015f69a9b9f884ebc289a35d02c5f6 from main

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