Re: Sanitize schema name

Hi Daniele,

some comments below:

On 21/05/2015 01:51, Daniele Varrazzo wrote:
> I'm sorry but I don't like this idea:
>
> On Wed, May 20, 2015 at 10:14 AM, Federico Di Gregorio <fog(at)dndg(dot)it> wrote:
>> On 13/05/2015 16:13, Elliot S wrote:
>>>
>>> I like this idea and drafted it up.
>>>
>>> Looking for comments on this patch:
>>>
>>>
>>> https://github.com/yieldsfalsehood/psycopg2/commit/f86f773de6ee99e2d7a2807136dcb458d97ba852
>>>
>>> In short:
>>> 1. identifier quoting may use PQescapeIdentifier if it's available,
>>> otherwise the pure-psyco escaping is done
>
> No: only the libpq version should be used. The psycopg one is not
> generic enough and only for internal use. The function is exposed in
> all the currently supported libpq versions so we don't really care
> about the older ones.

Agreed.

>>> 2. the %t format is now accepted, and its value must be either a
>>> string or bytes (no error handling is done yet if this isn't the case) -
>>> replacement for this calls out to the identifier quoting
>
> I don't like this idea: %t is nowhere standard. Psycopg has already
> powerful enough types-based adaptation: using it with identifiers has
> always been rejected because moving to libpq parameters would break.
> But the %t would break too so I don't see why to jump on this idea.
> Furthermore using a different parameter goes sideways to the entire
> type system: what if one passes a number to a %t? Looking at the patch
> it seems like it silently discards the value. That's a no for me.
>
> IMO we should just do:
>
> 1) expose the PQescape* libpq functions as pure bytes-bytes functions
> for people to use them as they wish. Under a good moon we should
> roundtrip unicode around. You know, python 3...
> 2) have a wrapper identifier object, whose adaptation result in
> identifier escaping. With current psycopg it could be used with:
>
> cur.execute("update table set %s = %s", (ident(field), value))

No no no. Bound variables should be easily identifiable and should not
be mixed with identifiers. If you don't like %t, that's OK but having an
ident behave like a bound variable isn't good either. I'd say lets just
stick to expose quoting functions in psycopg.extensions or as extra
methods on the connection object.

> if we want this to be used in the future:
>
> sql = "update table set %s = %%s" % ident(field) # [note]
> cur.execute(sql, (value,))
>
> [note]: this is a lie because the libpq functions take a connection as
> parameter too so we cannot just compose using the string % operator.
> We can have a cursor method doing that instead, or having "ident"
> being a cursor method.

If it requires a connection, lets stick it to the connection object.

federico

--
Federico Di Gregorio federico(dot)digregorio(at)dndg(dot)it
Di Nunzio & Di Gregorio srl http://dndg.it
When people say things are a lot more complicated than that, they
means they're getting worried that they won't like the truth.
-- Granny Weatherwax