The fmtId() function used in pg_dump for optional quoting identifiers
has bothered me for a while now. The API is confusing: the returned
value needs to be used before fmtId() is called again, because the
buffer the return value points to is re-used for each call of fmtId().
That leads to bugs for those unaware of this requirement, and clumsy
code for those that are -- for example (pg_dump.c:2911)
appendPQExpBuffer(delq, "DROP TYPE %s.", fmtId(tinfo->typnamespace->nspname,
force_quotes)); appendPQExpBuffer(delq, "%s;\n", fmtId(tinfo->typname, force_quotes));
Should really only be 1 line of code. Similar ugliness occurs in many
places (e.g. several lines down, there is a section of 4 calls to
appendPQExpBuffer() that could be condensed down to 1, if not for
fmtId() ).
Lastly, it has a tendancy to produce memory leaks -- for example,
convertRegProcReference() in pg_dump.c will leak memory when used
with PostgreSQL >= 7.3. When I mentioned this to Tom Lane, he
said that he didn't see a way to fix it without convoluting the
code, and suggested I bring it up on -hackers.
My suggestion is: since fmtId() is almost always used with
appendPQExpBuffer(), we should add a wrapper function to pg_dump
that accepts an extra escape sequence (%S, or %i, perhaps), which
would properly quote the input string before passing it to
appendPQExpBuffer(). That should ensure that there won't be any
leaks from adding quotes to identifiers, but also allow us to avoid
playing games with static buffers, like fmtId() does now.
Any comments?
Cheers,
Neil
--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC