Обсуждение: Change in datetime type casting
From the docs:
>>> dt = datetime.datetime.now()
>>> dt
datetime.datetime(2010, 2, 8, 1, 40, 27, 425337)
>>> cur.mogrify("SELECT %s, %s, %s;", (dt, dt.date(), dt.time()))
"SELECT '2010-02-08T01:40:27.425337', '2010-02-08', '01:40:27.425337';"
Current (2.4.3) behavior:
dt
Out[28]: datetime.datetime(2012, 6, 27, 16, 11, 33, 125585)
cur1.mogrify("SELECT %s, %s, %s;", (dt, dt.date(), dt.time()))
Out[30]: "SELECT '2012-06-27T16:11:33.125585'::timestamp,
'2012-06-27'::date, '16:11:33.125585'::time;"
Note the addition of the casts. This is causing problems when using the
hstore adapter as hstore expects a plain string. Is there a way to get
around this?
--
Adrian Klaver
adrian.klaver@gmail.com
On 28/06/12 01:19, Adrian Klaver wrote:
> From the docs:
>>>> dt = datetime.datetime.now()
>>>> dt
> datetime.datetime(2010, 2, 8, 1, 40, 27, 425337)
>
>>>> cur.mogrify("SELECT %s, %s, %s;", (dt, dt.date(), dt.time()))
> "SELECT '2010-02-08T01:40:27.425337', '2010-02-08', '01:40:27.425337';"
>
> Current (2.4.3) behavior:
> dt
> Out[28]: datetime.datetime(2012, 6, 27, 16, 11, 33, 125585)
>
> cur1.mogrify("SELECT %s, %s, %s;", (dt, dt.date(), dt.time()))
> Out[30]: "SELECT '2012-06-27T16:11:33.125585'::timestamp,
> '2012-06-27'::date, '16:11:33.125585'::time;"
>
> Note the addition of the casts. This is causing problems when using the
> hstore adapter as hstore expects a plain string. Is there a way to get
> around this?
The cast were introduced because PostgreSQL 9.x is much strictier about
function signatures than the 8.x series. Sometimes functions taking
parameters were not found because of the implicit text->other type cast.
But hstore does exactly the opposite. *sigh*
I don't have a nice solution right now, let's think aout it.
federico
--
Federico Di Gregorio federico.digregorio@dndg.it
Studio Associato Di Nunzio e Di Gregorio http://dndg.it
Gli esseri umani, a volte, sono destinati, per il solo fatto di
esistere, a fare del male a qualcuno. -- Haruki Murakami
On Thu, Jun 28, 2012 at 7:30 AM, Federico Di Gregorio <fog@dndg.it> wrote: > On 28/06/12 01:19, Adrian Klaver wrote: >> Note the addition of the casts. This is causing problems when using the >> hstore adapter as hstore expects a plain string. Is there a way to get >> around this? > > The cast were introduced because PostgreSQL 9.x is much strictier about > function signatures than the 8.x series. Sometimes functions taking > parameters were not found because of the implicit text->other type cast. Hstore is a mapping from strings to strings, not a generic dictionary mapping. There's never been a promise to adapt generic python dictionary (mapping hashable -> anything) into it. Even if you ram a python date into a postgres hstore, reading it back you will get a string. You can write a custom adapter to map the datetimes to strings without the cast and use them into hstore. -- Daniele
On 28/06/12 11:21, Daniele Varrazzo wrote: > On Thu, Jun 28, 2012 at 7:30 AM, Federico Di Gregorio <fog@dndg.it> wrote: >> > On 28/06/12 01:19, Adrian Klaver wrote: >>> >> Note the addition of the casts. This is causing problems when using the >>> >> hstore adapter as hstore expects a plain string. Is there a way to get >>> >> around this? >> > >> > The cast were introduced because PostgreSQL 9.x is much strictier about >> > function signatures than the 8.x series. Sometimes functions taking >> > parameters were not found because of the implicit text->other type cast. > Hstore is a mapping from strings to strings, not a generic dictionary > mapping. There's never been a promise to adapt generic python > dictionary (mapping hashable -> anything) into it. Even if you ram a > python date into a postgres hstore, reading it back you will get a > string. Doh! I wasn't aware that hstore used text only (never needed it myself). federico -- Federico Di Gregorio federico.digregorio@dndg.it Studio Associato Di Nunzio e Di Gregorio http://dndg.it Everything will be OK at the end. If it's not OK, it's not the end. -- Unknown
On 06/28/2012 02:21 AM, Daniele Varrazzo wrote: > On Thu, Jun 28, 2012 at 7:30 AM, Federico Di Gregorio <fog@dndg.it> wrote: >> On 28/06/12 01:19, Adrian Klaver wrote: > >>> Note the addition of the casts. This is causing problems when using the >>> hstore adapter as hstore expects a plain string. Is there a way to get >>> around this? >> >> The cast were introduced because PostgreSQL 9.x is much strictier about >> function signatures than the 8.x series. Sometimes functions taking >> parameters were not found because of the implicit text->other type cast. > > Hstore is a mapping from strings to strings, not a generic dictionary > mapping. There's never been a promise to adapt generic python > dictionary (mapping hashable -> anything) into it. Even if you ram a > python date into a postgres hstore, reading it back you will get a > string. Which begs the question why does the adapter not do str(value) when creating the ARRAY? > > You can write a custom adapter to map the datetimes to strings without > the cast and use them into hstore. Bigger question:) Is it possible to turn adaption off for a connection/cursor? > > -- Daniele > -- Adrian Klaver adrian.klaver@gmail.com
On Thu, Jun 28, 2012 at 3:11 PM, Adrian Klaver <adrian.klaver@gmail.com> wrote: > Which begs the question why does the adapter not do str(value) when creating > the ARRAY? Because it would give a false promise of working. If I put a date in my db, I'm expecting a date to come out: any other result is an error. The hstore adapter is documented to take strings mapping: results with any other mapping is undefined. A better behaviour could be to explicitly raise an explicit exception, but it would be less efficient than trusting the user he is using the adapter as documented (see HstoreAdapter._getquoted_9(): we adapt the dict.values() list, not iterate on each values). 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. > Bigger question:) Is it possible to turn adaption off for a > connection/cursor? No, I think adaptation is currently only global. No, I don't like this asymmetry with typecasting either. -- Daniele
On 06/28/2012 07:43 AM, Daniele Varrazzo wrote: > On Thu, Jun 28, 2012 at 3:11 PM, Adrian Klaver <adrian.klaver@gmail.com> wrote: > >> Which begs the question why does the adapter not do str(value) when creating >> the ARRAY? > > Because it would give a false promise of working. If I put a date in > my db, I'm expecting a date to come out: any other result is an error. > The hstore adapter is documented to take strings mapping: results with > any other mapping is undefined. A better behaviour could be to > explicitly raise an explicit exception, but it would be less efficient > than trusting the user he is using the adapter as documented (see > HstoreAdapter._getquoted_9(): we adapt the dict.values() list, not > iterate on each values). I am not seeing the false promise. hstore is documented to only work with string keys and values. I am not entering into a date field but a hstore field. I would expect the date to be entered or returned as a string. I see nothing wrong with the adapter doing that on my behalf. The precedent as it where would be the Python csv module. In fact I may just run data through csv to get the effect, though it would be nice not to. > > -- Daniele > -- Adrian Klaver adrian.klaver@gmail.com
On 28/06/12 16:43, Daniele Varrazzo wrote: > >> > Bigger question:) Is it possible to turn adaption off for a >> > connection/cursor? > No, I think adaptation is currently only global. No, I don't like this > asymmetry with typecasting either. This asymmetry exists because adaptation was modeled on PEP 246 and at the time a global type registry was enough (you could always write your own wrapper that would adapt the wrapped object, see AsIs for an example.) Having per-connection and per-cursor adapter registries would be nice. federico -- Federico Di Gregorio federico.digregorio@dndg.it Studio Associato Di Nunzio e Di Gregorio http://dndg.it Qu'est ce que la folie? Juste un sentiment de liberté si fort qu'on en oublie ce qui nous rattache au monde... -- J. de Loctra
On 06/28/2012 07:43 AM, Daniele Varrazzo wrote:
> On Thu, Jun 28, 2012 at 3:11 PM, Adrian Klaver <adrian.klaver@gmail.com> wrote:
>
>> Which begs the question why does the adapter not do str(value) when creating
>> the ARRAY?
>
> Because it would give a false promise of working. If I put a date in
> my db, I'm expecting a date to come out: any other result is an error.
> The hstore adapter is documented to take strings mapping: results with
> any other mapping is undefined. A better behaviour could be to
> explicitly raise an explicit exception, but it would be less efficient
> than trusting the user he is using the adapter as documented (see
> HstoreAdapter._getquoted_9(): we adapt the dict.values() list, not
> iterate on each values).
>
> 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.
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?
> -- Daniele
>
--
Adrian Klaver
adrian.klaver@gmail.com
On Fri, Jun 29, 2012 at 07:51:44AM -0700, Adrian Klaver wrote: > >>Which begs the question why does the adapter not do str(value) when creating > >>the ARRAY? > > > >Because it would give a false promise of working. If I put a date in > >my db, I'm expecting a date to come out: any other result is an error. > >The hstore adapter is documented to take strings mapping: results with > >any other mapping is undefined. A better behaviour could be to > >explicitly raise an explicit exception, but it would be less efficient > >than trusting the user he is using the adapter as documented (see > >HstoreAdapter._getquoted_9(): we adapt the dict.values() list, not > >iterate on each values). > > > >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. > > 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? It would not solve this concern voiced above: > >Because it would give a false promise of working. Karsten -- GPG key ID E4071346 @ gpg-keyserver.de E167 67FD A291 2BEA 73BD 4537 78B9 A9F9 E407 1346
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? Using str() is wrong: at least you should use adapt() and .getquoted() to avoid SQL-injection attacks. But I think that adding something like that would be fine. Daniele? federico -- Federico Di Gregorio federico.digregorio@dndg.it Studio Associato Di Nunzio e Di Gregorio http://dndg.it I filosofi son come i sociologi: il mondo non lo capiscono. -- A.R.M.
On 06/29/2012 07:59 AM, Karsten Hilbert wrote: > On Fri, Jun 29, 2012 at 07:51:44AM -0700, Adrian Klaver wrote: > >>>> Which begs the question why does the adapter not do str(value) when creating >>>> the ARRAY? >>> >>> Because it would give a false promise of working. If I put a date in >>> my db, I'm expecting a date to come out: any other result is an error. >>> The hstore adapter is documented to take strings mapping: results with >>> any other mapping is undefined. A better behaviour could be to >>> explicitly raise an explicit exception, but it would be less efficient >>> than trusting the user he is using the adapter as documented (see >>> HstoreAdapter._getquoted_9(): we adapt the dict.values() list, not >>> iterate on each values). >>> >>> 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. >> >> 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? > > It would not solve this concern voiced above: > >>> Because it would give a false promise of working. As I said before there is no false promise. hstore is just a fancy string type. There is no expectation that types will be preserved, so why maintain the illusion? > > Karsten > -- Adrian Klaver adrian.klaver@gmail.com
On 06/29/2012 07:59 AM, Federico Di Gregorio 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?
>
> Using str() is wrong: at least you should use adapt() and .getquoted()
> to avoid SQL-injection attacks.
The above was a quick and dirty hack. I am still working my way through
the adaptation mechanism. In fact after I sent the previous, I had
another thought:
class HstoreAdapter(object):
"""Adapt a Python dict to the hstore syntax."""
def __init__(self, wrapped, stringify=False):
self.wrapped = wrapped
if stringify:
for k in self.wrapped:
self.wrapped[k] = str(self.wrapped[k])
This would preserve present behavior in the default case. I just am not
sure how to pass the stringify flag down through the register_hstore()
process.
>
> But I think that adding something like that would be fine. Daniele?
>
> federico
>
--
Adrian Klaver
adrian.klaver@gmail.com
On 29/06/12 17:10, Adrian Klaver wrote: >>> >>> Is there a possibility it could find its way into psycopg2 proper? >> >> Using str() is wrong: at least you should use adapt() and .getquoted() >> to avoid SQL-injection attacks. > > The above was a quick and dirty hack. I am still working my way through > the adaptation mechanism. In fact after I sent the previous, I had > another thought: > > class HstoreAdapter(object): > """Adapt a Python dict to the hstore syntax.""" > def __init__(self, wrapped, stringify=False): > self.wrapped = wrapped > if stringify: > for k in self.wrapped: > self.wrapped[k] = str(self.wrapped[k]) > > This would preserve present behavior in the default case. I just am not > sure how to pass the stringify flag down through the register_hstore() > process. Nah, adapter can't take extra arguments. Given that what you actually *get* from the database are just strings I agree with you that transforming everything to string on input too is good. We can raise an exception unless all key/values are already strings but that will put burden on the user for no good reason. Unless Daniele has string objections to it or beats me to the implementation (as always) I'll push something working this week end. federico -- Federico Di Gregorio federico.digregorio@dndg.it Studio Associato Di Nunzio e Di Gregorio http://dndg.it Il panda ha l'apparato digerente di un carnivoro (e.g., di un orso). Il panda ha scelto di cibarsi esclusivamente di germogli di bambù. Quindi, il panda è l'unico animale vegano del pianeta. Il panda merita di estinguersi. -- Maria, Alice, Federico
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
On 06/29/2012 08:53 AM, Daniele Varrazzo wrote:
> 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".
test=> \d bool_test
Table "public.bool_test"
Column | Type | Modifiers
----------+---------+-----------
ifd | integer |
bool_fld | boolean |
test=> INSERT INTO bool_test VALUES (34,'True');
INSERT 0 1
^
test=> SELECT * from bool_test where ifd=34;
ifd | bool_fld
-----+----------
34 | t
(1 row)
No problem.
>
> 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.
Once again there is no putting of non string stuff into a hstore field.
By definition it is string only. If someone wants to maintain type
consistency, do not use hstore, use a regular field. When you make the
choice to use an hstore field you have made the choice to ignore types.
The adapter should respect that. It is up to the user to deal with that
when the data is returned.
>
>
> -- Daniele
>
--
Adrian Klaver
adrian.klaver@gmail.com
On Fri, Jun 29, 2012 at 6:27 PM, Adrian Klaver <adrian.klaver@gmail.com> wrote:
> On 06/29/2012 08:53 AM, Daniele Varrazzo wrote:
>> 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".
> test=> INSERT INTO bool_test VALUES (34,'True');
> INSERT 0 1
You are only thinking about half of the story: writing stuff in. I am
thinking about the people who will have to read things out. Writing
"True" as a boolean, not only you are giving people the problem of
knowing the type, you are also adding an entirely different
representation of a boolean into the database that any wannabe user of
that hstore value will have to know. Which is good as any other (but
less good than the only *output* postgres provides), and binds us,
hands and feet, to maintain that one.
It may eventually happen in the future that we will allow any type
into an hstore, but that their conversion will be str() will just not
happen.
But then, what about the keys? Shall we convert them too or not? If
so, what about the dict {1: 'hello', '1': 'world'}: how do you convert
it into an hstore?
-- Daniele
On 06/29/2012 02:36 PM, Daniele Varrazzo wrote:
> On Fri, Jun 29, 2012 at 6:27 PM, Adrian Klaver <adrian.klaver@gmail.com> wrote:
>
>> test=> INSERT INTO bool_test VALUES (34,'True');
>> INSERT 0 1
>
> You are only thinking about half of the story: writing stuff in. I am
> thinking about the people who will have to read things out. Writing
> "True" as a boolean, not only you are giving people the problem of
> knowing the type, you are also adding an entirely different
> representation of a boolean into the database that any wannabe user of
> that hstore value will have to know. Which is good as any other (but
> less good than the only *output* postgres provides), and binds us,
> hands and feet, to maintain that one.
The basic problem is that you are clinging to the notion that types have any
place in hstore. To illustrate:
test=> \d hstore_type
Table "utility.hstore_type"
Column | Type | Modifiers
--------+-------------------+-----------
fld_1 | integer |
fld_2 | character varying |
fld_3 | boolean |
fld_4 | character varying |
test=> SELECT * from hstore_type ;
fld_1 | fld_2 | fld_3 | fld_4
-------+-------+-------+-------
1 | 1 | t | t
2 | 2 | f | f
test=> insert INTO hstore_test(hstore_fld) SELECT hstore(ht) from hstore_type as ht;
INSERT 0 2
test=> SELECT * from hstore_test;
id | hstore_fld
----+--------------------------------------------------------
4 | "fld_1"=>"1", "fld_2"=>"1", "fld_3"=>"t", "fld_4"=>"t"
5 | "fld_1"=>"2", "fld_2"=>"2", "fld_3"=>"f", "fld_4"=>"f"
While I like the idea of a hstore type that maintains awareness of the type of its element
values, that does not exist at this time. Trying to get psycopg2 to impose that over
top of what does exist is fruitless. If you forget about maintaining type information the
problem gets a whole lot easier.
>
> It may eventually happen in the future that we will allow any type
> into an hstore, but that their conversion will be str() will just not
> happen.
That was a quick and dirty hack for proof of concept. I understand now the dangers in that
approach and can see a more sophisticated method is in order.
>
> But then, what about the keys? Shall we convert them too or not? If
> so, what about the dict {1: 'hello', '1': 'world'}: how do you convert
> it into an hstore?
Yes convert keys. Throw an exception when keys convert to same string value.
>
>
> -- Daniele
>
--
Adrian Klaver
adrian.klaver@gmail.com
On 29/06/12 23:36, Daniele Varrazzo wrote:
>> test=> INSERT INTO bool_test VALUES (34,'True');
>> > INSERT 0 1
> You are only thinking about half of the story: writing stuff in. I am
> thinking about the people who will have to read things out. Writing
> "True" as a boolean, not only you are giving people the problem of
> knowing the type, you are also adding an entirely different
> representation of a boolean into the database that any wannabe user of
> that hstore value will have to know. Which is good as any other (but
> less good than the only *output* postgres provides), and binds us,
> hands and feet, to maintain that one.
>
> It may eventually happen in the future that we will allow any type
> into an hstore, but that their conversion will be str() will just not
> happen.
>
> But then, what about the keys? Shall we convert them too or not? If
> so, what about the dict {1: 'hello', '1': 'world'}: how do you convert
> it into an hstore?
Hi *,
I investigated the problem a little bit during the weekend and tried
with some "dumb" conversions, like using adapt() and then removing the
cast from the resulting string (and adding an E' before the whole string
if any of the strings had it). It works, barely, and it is a #?^!£$% hack.
After writing the code I guessed at least a couple of ways of mixing
different data types to produce a string with the wrong quoting. That's
still not valid SQL-injection but not far from it.
So, from a security standpoint, I see 2 answers to this problem:
1) Convert everything to string (using str()) and then quote that to
build the hstore data. I don't like this but at least it is safe.
2) Throw an exception if keys/values aren't strings and force the user
to convert the data before trying to send it to the hstore.
I am starting to think that (2) is the only sane solution.
federico
--
Federico Di Gregorio federico.digregorio@dndg.it
Studio Associato Di Nunzio e Di Gregorio http://dndg.it
Il panda ha l'apparato digerente di un carnivoro (e.g., di un orso).
Il panda ha scelto di cibarsi esclusivamente di germogli di bambù.
Quindi, il panda è l'unico animale vegano del pianeta. Il panda
merita di estinguersi. -- Maria, Alice, Federico
On Mon, Jul 2, 2012 at 9:24 AM, Federico Di Gregorio <fog@dndg.it> wrote: > 2) Throw an exception if keys/values aren't strings and force the user > to convert the data before trying to send it to the hstore. We can definitely add a check to verify every key is a basestring and every value is a basestring or null (this is exactly the hstore domain). It is an extra iteration to be performed on the entire content of the dict though. Something on a slightly different note we could do: adapt takes a protocol as input (undocumented I believe, defaulting to ISQLQuote, currently the only protocol used): if we added a parameter to the protocol, defaulting to the current extensions.adapters we could have: - adaptation of hstore keys is performed on a limited set of datatypes: only str and unicode. Trying to pass another type in a dict would raise an adaptation error; - adaptation of hstore values is performed on the same types plus None; - if s.b. wanted to map other types into an hstore they can add other adapters just on the HstoreAdapter object (the object returned by register_adapter) - we can have adapters map per connection/cursor, other argument touched in this thread. So we stay strict in the default you have independently verified are the only sane default choice, allow for extension by users who know what they want to do and get a nice symmetry between adaptation and type casting. As an interface, register_adapter() could take an extra context as register_type() does: it could be a connection, a cursor or an HstoreAdapter (e.g. it could be whatever object exporting a method add_adapter() or something along this line). This is an idea of 3am after the PyBirra: if nothing makes sense here, please ignore. -- Daniele