Opened 8 years ago

Closed 8 years ago

#26296 closed Cleanup/optimization (wontfix)

values() after extra() gives incorrect error message

Reported by: aimestereo Owned by: aimestereo
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by aimestereo)

In documentation there's comment in values() section about some behavior of extra

If you use a values() clause after an extra() call, any fields defined by a select argument in the extra() must be explicitly included in the values() call. Any extra() call made after a values() call will have its extra selected fields ignored.

It's points that any extra fields after values() call will be ignored. But I think that it's not correct and need more description.

For example it could be not obvious that next simple construction will cause exception and such an not obvious one!

User.objects.values('username').extra(select={'test_field': 1}).values('test_field', 'username')

FieldError: Cannot resolve keyword 'test_field' into field. Choices are: date_joined, email, first_name, groups, id, is_active, is_staff, is_superuser, last_login, last_name, logentry, password, test_field, user_permissions, username

I want to point out that test_field is listed in choices that's very confusing. It happens because of all checks is runs against extra_select but error is uses extra as part of available choices.

sorted(list(get_field_names_from_opts(opts)) + list(self.extra)
                               + list(self.annotation_select))

I can help you with this small change.

Change History (4)

comment:1 by aimestereo, 8 years ago

Description: modified (diff)
Owner: changed from nobody to aimestereo
Status: newassigned

comment:2 by Tim Graham, 8 years ago

Summary: Trick with extra fieldsvalues() after extra() gives incorrect error message

Note the documentation, "This is an old API that we aim to deprecate at some point in the future. Use it only if you cannot express your query using other queryset methods. If you do need to use it, please file a ticket using the QuerySet.extra keyword with your use case (please check the list of existing tickets first) so that we can enhance the QuerySet API to allow removing extra(). We are no longer improving or fixing bugs for this method."

In light of that, is it worth fixing this?

comment:3 by Tim Graham, 8 years ago

Component: DocumentationDatabase layer (models, ORM)
Keywords: db removed
Needs tests: set
Triage Stage: UnreviewedAccepted

I guess the patch is simple enough. It just needs a test.

comment:4 by Tim Graham, 8 years ago

Resolution: wontfix
Status: assignedclosed

The patch causes a regression in another unit test, so I'm not going to bother spending more time on this.

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