Re: quoting psql varible as identifier

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: quoting psql varible as identifier
Date: 2010-01-19 20:01:40
Message-ID: 603c8f071001191201n15a5d236s3f0b372de6592d7b@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 19, 2010 at 2:38 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> As long as it's documented it's okay ... probably ... note that
> conditionally inserting E would get us right back to the place where
> an unsafe usage might appear to work fine in light testing.  Maybe
> prepend a space only if prepending E?

That's a bit of a kludge, but maybe it's liveable.

>>> * malloc our own result value instead of expecting caller to provide
>>> space.  This adds another failure case (out of memory) but it's not
>>> really much different from OOM on the caller side.
>
>> I think that would be an anti-improvement - caller might have their
>> own malloc substitute, might want to stack-allocate, etc.  Plus we'd
>> either have to malloc the worst-case buffer size or else read the
>> string twice.  But see below...
>
> We already have multiple functions in the API that do malloc().  I think
> that requiring the caller to estimate the size of the space is a bigger
> anti-feature than using malloc.
>
>> I like those names.  Maybe something like:
>
>> size_t PQescapeLiteral(PGconn *conn, char *to, size_t tolen, char
>> *from, size_t fromlen, int *error);
>> size_t PQescapeIdentifier(PGconn *conn, char *to, size_t tolen, char
>> *from, size_t fromlen, int *error);
>
> This would get a *lot* simpler if we malloced the result space.
>
>        char *PQescapeXXX(PQconn *conn, const char *str, size_t len)
>
> with NULL result used to signal any failure (and a message left in
> the conn).

Well, I have to admit that the simplicity of that API is very
appealing. Someone will probably eventually request
PQescapeXXXButUseMyMalloc(PQConn *conn, const char *str, size_t len,
void (*MyMalloc)(size_t)); ... but at least until we change the
escaping algorithm again, we can always tell that person to use
PQescapeStringConn() and roll their own. So, I'm sold.

Do you think we should malloc 2n+X bytes all the time, or do you want
to scan the string once to determine how much space is needed and then
allocate exactly that much space? It seems to me that it might be
sensible to do it this way:

1. Do a locale-aware scan of the input string and count the number of
characters we need to escape (num_to_escape).
2. If num_to_escape is 0, fast path: allocate len+3 bytes and use
memcpy to copy the input data to the output buffer.
3. Otherwise, allocate len+num_to_escape+5 bytes
(space-E-quote-quote-NUL) and do a second locale-aware scan of the
input string, copying and escaping as we go (or do you only want the
space-E if the escapable character that we find is \ rather than '?).

...Robert

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hitoshi Harada 2010-01-19 20:02:16 Re: review: More frame options in window functions
Previous Message Euler Taveira de Oliveira 2010-01-19 19:59:31 Re: MySQL-ism help patch for psql