On Fri, Jun 29, 2012 at 3:59 PM, Federico Di Gregorio <fog@dndg.it> wrote:
> On 29/06/12 16:51, Adrian Klaver wrote:
>>> If writing a date in and reading a string out is enough for your
>>> application, you can easily write your own specialized hstore adapter
>>> based on the code in extras.py.
>> and
>> The following change in extras.py solves the problem for dates and other
>> non string types.:
>>
>> class HstoreAdapter(object):
>> """Adapt a Python dict to the hstore syntax."""
>> def __init__(self, wrapped):
>> self.wrapped = wrapped
>> for k in self.wrapped: <--
>> self.wrapped[k] = str(self.wrapped[k]) <--
>>
>> Is there a possibility it could find its way into psycopg2 proper?
This changes the dictionary inplace: this is definitely not making its way!
> Using str() is wrong: at least you should use adapt() and .getquoted()
> to avoid SQL-injection attacks.
Agreed: str() is one of the last things I'd like there. First for
encodings problem: str() or unicode()? Then, str gets you a Python
string representation of the object: str(True) is "True", in postgres
it would have been "t".
A better implementation of the idea would have *at least* the postgres
representation of the types in the database. For this, as noted by
Adrian, the current ISqlQuote is not right as it contains the quotes,
including the E where required and possibly the cast. Implementing the
long due ISQL interface (the one to complement PQexecParams) would
allow at least correct adaptation (no injection) and return postgres
format.
> But I think that adding something like that would be fine. Daniele?
I've expressed my opinion yesterday: I don't like the idea. I'm pretty
sure anybody who wants to put non-string stuff into an hstore would
have his own need and would be not a problem at all to pre-process
their dict before passing them to the database. Choosing your own
string representation seems an application choice, not one the driver
should make, and specifically passing the Python str() seems a bad
choice. The postgres string representation (such that value::type
would work, although type is not stored in the hstore and should be
known by other means) would be something I wouldn't regret.
-- Daniele
