Re: Sanitize schema name

I'm sorry but I don't like this idea:

On Wed, May 20, 2015 at 10:14 AM, Federico Di Gregorio <fog@dndg.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.

>>    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))

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.


> Also, I'd expose the quoting function in psycopg.extensions to let the user
> build the query string separately from the .execute() call: this is useful
> if you want to stick to DBAPI in your .execute() call. I.e., to allow
> something like:
>
> from psycopg.extensions import quote_ident
>
> query = "SELECT %s FROM %s WHERE id = %%s" % (
>         quote_ident('table'), quote_ident('col'))
>
> curs.execute(query, (id_value,))

Agreed on that (point 1 above), with the note that we should handle
the fact that the libpq function takes the connection too as argument.


-- Daniele