Re: quoting psql varible as identifier

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Jan 19, 2010 at 2:01 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> * include boundary quotes (and E too in the literal case). This would
>> imply telling people they should leave whitespace around the value in
>> the constructed query ... or should we insert leading/trailing spaces
>> to prevent that mistake too?

> Does the existing PQescapeStringConn() require E'' rather than just
> ''? It isn't documented so.

It does not expect E'', and would get it wrong if you supplied E'',
which is an anti-feature because it makes it impossible to avoid
escape_string_warning messages. If we were to supply quotes I think
we should also supply E whenever we need to use a backslash.

> I think inserting the whitespace would be overkill, but that's another
> thing that's not mentioned in the present documentation and should be,
> because it would not be difficult to screw it up.

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?

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

> ... Or they can just size the buffer
> according to the documentation and then they're safe.

Wrong, one thing we would absolutely not do is document any hard
guarantee about the maximum space needed. That's isomorphic to
documenting the exact escaping algorithm, and doing that has already
bit us more times than you know. If we're going to alter the API at
all I think we absolutely *must* get rid of that design error.

Keep in mind that a large part of wanting to have these functions at all
is for simplicity in the calling code. Complicating their API for
marginal gains doesn't seem to me to be the right design direction.

regards, tom lane