Re: [Django] #9619: to_python not called when fetching data with .values(...)
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: SVN
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Design
Keywords: values,custom | decision needed
field, datetime | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by sam@…):
* ui_ux: => 0
* easy: => 0
Comment:
IMHO, if this isn't a bug it's definitely a gotcha (and the documentation
should be updated). I spent the last several hours trying to debug why
values_list was not returning the python objects defined by my custom
model field's to_python method.
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:14>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: SVN
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Design
Keywords: values,custom | decision needed
field, datetime | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by aaugustin):
I probably haven't been here long enough, but I'm finding this behavior is
extremely unexpected.
I'll leave this ticket in DDN because I'm not sure I grasp all the
consequences.
But my gut feeling is that we should swallow the backwards incompatibility
and fix this at some point — or at the very least make it clear in the
documentation that `QuerySet.values()` returns raw database values, as
spit by the database adapter.
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: values,custom | Needs documentation: 0
field, datetime | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* stage: Design decision needed => Accepted
Comment:
I vote for raw kwarg, if True no conversion happens. One day the default
should be False, but likely with backwards compatibility period where it
default still to True.
It might be worth it to inspect if there is a way to skip those method
calls that do not actually do anything, maybe just if 'col_field.to_python
== Field.to_python'. The standard Field.to_python just returns the value
as is. There might be need to ask the field explicitly if the given
connection's values needs conversion, most of the to_python()
implementations seem non-necessary (IntegerField does int(value) for
example, but I don't believe this is necessary on any DB's values).
The overhead of calling no-ops in trivial test case is 0.7s vs 0.07s. The
test is:
{{{
def noop(value):
return value
myvals = [
(0, 1, 2, 3, 4),
] * 100
def values():
for row in myvals:
yield row # alternate: yield map(noop, row)
from datetime import datetime
start = datetime.now()
for _ in range(0, 10000):
for val in values():
pass
print datetime.now() - start
}}}
Still, the test passes in under a second even with the map call, so it
might be that the overhead doesn't matter in practice.
Anyways, marking as accepted, it would be good to end up in a situation
where you can opt out from conversion, but the default is to use
to_python() for the values.
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: values,custom | Needs documentation: 0
field, datetime | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: marfire (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Keywords: values,custom | Triage Stage: Accepted
field, datetime |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jedie):
* cc: django@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: values,custom | Triage Stage: Accepted
field, datetime |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by jedie):
If i see it right, this has been fixed with Django v1.8 and the change
from {{{to_python()}}} to {{{from_db_value()}}} isn't it?
The docs at https://docs.djangoproject.com/en/1.8/howto/custom-model-
fields/#converting-values-to-python-objects says:
{{{
If present for the field subclass, from_db_value() will be called in all
circumstances when the data is loaded from the database, including in
aggregates and values() calls.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: values,custom | Triage Stage: Accepted
field, datetime |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"9aac99e96d100b34b6daa3a137531eff4f17076e" 9aac99e]:
{{{
#!CommitTicketReference repository=""
revision="9aac99e96d100b34b6daa3a137531eff4f17076e"
Refs #9619 -- Added a test that Field.from_db_value() is called on
QuerySet.values()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Component: Database layer | Version: master
(models, ORM) |
Keywords: values,custom | Triage Stage: Accepted
field, datetime |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: Valera Grishin | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: values,custom | Triage Stage: Accepted
field, datetime |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Markus Holtermann <info@…>):
In [changeset:"64a4211aa85795cb21a73bc93f9e43bafa1541f6" 64a4211]:
{{{
#!CommitTicketReference repository=""
revision="64a4211aa85795cb21a73bc93f9e43bafa1541f6"
Refs #9619 -- Fixed failing test case
Regression introduced in 9aac99e96d100b34b6daa3a137531eff4f17076e
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9619#comment:22>