Opened 17 months ago

Closed 16 months ago

Last modified 16 months ago

#21738 closed Uncategorized (wontfix)

models.F does not accept fields generated via QuerySet.extra

Reported by: tonnzor Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: tonn81@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I found that models.F has a limited usage -- it cannot be used for fields added manually via QuerySet.extra(select={...})

Sample use case:

User.objects.extra(select={'new_is_active': '(CASE WHEN id > 100 THEN 1 ELSE 0 END)'}).update(is_active=models.F('new_is_active'))

Probably it was intended but if F supports extra fields -- it would be really powerful tool. Otherwise you have to fall to raw SQL.

Also I see at least one way to convert it to SQL -- so there should be a way to implement it:

UPDATE auth_user
set is_active = (CASE WHEN id > 100 THEN 1 ELSE 0 END)

Here is the example:

git clone https://github.com/django/django.git
cd django
cp django/bin/django-admin.py .
./django-admin.py startproject ttest
cd ttest
ln -s ../django
./manage.py syncdb --noinput

echo "from django.contrib.auth.models import *; User.objects.extra(select={'new_is_active': '(CASE WHEN id > 100 THEN 1 ELSE 0 END)'}).update(is_active=models.F('new_is_active'))" | ./manage.py shell

Python 2.7.5 (default, Aug 25 2013, 00:04:04) 
[GCC 4.2.1 Compatible Apple LLVM 5.0 (clang-500.0.68)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Users/tonnzor/build/django/ttest/django/db/models/query.py", line 577, in update
    rows = query.get_compiler(self.db).execute_sql(None)
  File "/Users/tonnzor/build/django/ttest/django/db/models/sql/compiler.py", line 958, in execute_sql
    cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
  File "/Users/tonnzor/build/django/ttest/django/db/models/sql/compiler.py", line 752, in execute_sql
    sql, params = self.as_sql()
  File "/Users/tonnzor/build/django/ttest/django/db/models/sql/compiler.py", line 932, in as_sql
    val = SQLEvaluator(val, self.query, allow_joins=False)
  File "/Users/tonnzor/build/django/ttest/django/db/models/sql/expressions.py", line 14, in __init__
    self.expression.prepare(self, query, allow_joins)
  File "/Users/tonnzor/build/django/ttest/django/db/models/expressions.py", line 149, in prepare
    return evaluator.prepare_leaf(self, query, allow_joins)
  File "/Users/tonnzor/build/django/ttest/django/db/models/sql/expressions.py", line 62, in prepare_leaf
    query.get_initial_alias(), self.reuse)
  File "/Users/tonnzor/build/django/ttest/django/db/models/sql/query.py", line 1375, in setup_joins
    names, opts, allow_many)
  File "/Users/tonnzor/build/django/ttest/django/db/models/sql/query.py", line 1302, in names_to_path
    "Choices are: %s" % (name, ", ".join(available)))
FieldError: Cannot resolve keyword 'new_is_active' into field. Choices are: date_joined, email, first_name, groups, id, is_active, is_staff, is_superuser, last_login, last_name, logentry, password, user_permissions, username

Change History (3)

comment:1 Changed 17 months ago by smeatonj

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I think this ticket is somewhat related to https://code.djangoproject.com/ticket/14030

Extra doesn't do anything smart. It stashes the raw value into a dictionary, and is simply added to the query on execution. Ideally, extra wouldn't be used at all. Instead, you should be able to:

User.objects.annotate(new_is_active=SQL('CASE WHEN id > 100 THEN 1 ELSE 0 END')).update(is_active=models.F('new_is_active'))

I don't think there's any good reason to improve .extra() behaviour, as it is mostly a workaround for issues in other parts of the ORM which should be fixed instead.

comment:2 Changed 16 months ago by akaariai

  • Resolution set to wontfix
  • Status changed from new to closed

I agree that adding more hacks for .extra() isn't a good idea. So, I'll wontfix this.

We could possibly make .extra() to automatically do .annotate(new_is_active=SQL('CASE WHEN id > 100 THEN 1 ELSE 0 END')), but this is pending on getting .annotate() support for that.

comment:3 Changed 16 months ago by tonnzor

OK, I will track #14030 then -- that would solve my problem as well.

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