Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#6184 closed (invalid)

encoding conversion breaks floating-point placeholders in raw PostgreSQL queries

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

Description

SVN revision 6903 rejects queries using floating-point values and placeholders:

>>> from django.db import connection
>>> c=connection.cursor()
>>> c.execute('SELECT * from my_table where floatfield > %f',[1])     # ints work fine
>>> c.execute('SELECT * from my_table where floatfield > %f',[0.1])
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "django/db/backends/util.py", line 18, in execute
  File "django/db/backends/postgresql/base.py", line 47, in execute
TypeError: float argument required

This is a regression from sometime between revision 4926 and 6903; sorry I can't be more specific.

The attached patch fixes this by modifying django.utils.encoding.smart_str() to really convert only instances of basestring when strings_only is true.

Attachments (1)

6184.diff (515 bytes ) - added by Chris Chamberlin <dja@…> 16 years ago.

Download all attachments as: .zip

Change History (5)

by Chris Chamberlin <dja@…>, 16 years ago

Attachment: 6184.diff added

comment:1 by Thomas Güttler, 16 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

I can't reproduce the problem. What postgres adapter are you using (postgres or psycopg2)?.

For me (psycopg2) it fails for ints, too:

>>> from django.db import connection
>>> c=connection.cursor()
>>> c.execute('SELECT * from modwork_beleg where id > %d',[1])
TypeError: int argument required
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/localhome/modw/modwork_esg/django/db/backends/util.py", line 18, in execute
    return self.cursor.execute(sql, params)

If I use %s instead of %f it works:

c.execute('SELECT * from modwork_beleg where id > %s',[1])
>>> import psycopg2
>>> psycopg2.paramstyle
'pyformat'

At least for psycopg2 smart_str() (which changed in the patch) is not called.
I modified it to throw an AssertionError and the result was the same.

I looked at the code of the old postgres adapter. Here the paramters get converted:

    def format_params(self, params):
        if isinstance(params, dict):
            result = {}
            charset = self.charset
            for key, value in params.items():
                result[smart_str(key, charset)] = smart_str(value, charset)
            return result
        else:
            return tuple([smart_str(p, self.charset, True) for p in params])

Maybe it is enough to add float to the isinstance test at the beginning of smart_str.

comment:2 by Malcolm Tredinnick, 16 years ago

If this does turn out to be a genuine problem, the patch is not correct at all. It is very important that most objects are passed through the unicode() path, since they know how to turn themselves into strings correctly. The "strings_only" whitelist is a list of exceptions for certain native Python types only that for technical reasons should be kept as their native types (each one has a specific reason). Changing that would break the object-oriented behaviour of Python.

comment:3 by Thomas Güttler, 16 years ago

Resolution: invalid
Status: newclosed

from: http://www.initd.org/tracker/psycopg/wiki/psycopg2_documentation#adaptation-of-python-values-to-sql-types

Adaptation of Python values to SQL types:
...
The variables placeholder must always be a %s, 
even if a different placeholder (such as a %d for an integer) 
may look more appropriate.

in reply to:  3 comment:4 by Chris Chamberlin, 16 years ago

Aha. Thanks.

Just for completeness in case someone else comes across this: I'm still using psycopg 1.x, and does apparently support the %d and %f placeholders; this change to support %s only is noted in psycopg's Migration from psycopg 1 to 2 documentation. However, using %s fixes the problem in psycopg1 as well, and is a perfectly reasonable workaround given that it's an old version of the adapter.

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