Opened 7 years ago

Closed 7 years ago

Last modified 7 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: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

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@…> 7 years ago.

Download all attachments as: .zip

Change History (5)

Changed 7 years ago by Chris Chamberlin <dja@…>

comment:1 Changed 7 years ago by guettli

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 7 years ago by mtredinnick

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 follow-up: Changed 7 years ago by guettli

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

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.

comment:4 in reply to: ↑ 3 Changed 7 years ago by Chris Chamberlin

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