#21738 closed Uncategorized (wontfix)
models.F does not accept fields generated via QuerySet.extra
| Reported by: | Artem Skoretskiy | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| 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 by , 12 years ago
comment:2 by , 12 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → 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.
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.