Обсуждение: pg_dump: Remove trivial usage of PQExpBuffer
I've been looking at ways to reorganize and/or clean up pg_dump.c.
One thing I have noticed is the usage of PQExpBuffer in situations where the query has no optional parts and no string interpolation.
Attached is a patch to replace those usages with the string literal itself. There are still a few cases where a buffer is used for a trivial query and then reset and reused for a more complicated query generation. In those cases, I did not make the change so as to keep this patch simple.
Вложения
> On 16 Dec 2025, at 23:03, Corey Huinker <corey.huinker@gmail.com> wrote: > One thing I have noticed is the usage of PQExpBuffer in situations where the query has no optional parts and no stringinterpolation. - res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); + res = ExecuteSqlQuery(fout, + "SELECT DISTINCT attrelid FROM pg_attribute " + "WHERE attacl IS NOT NULL", + PGRES_TUPLES_OK); I'm not sure I find it an improvement to put have to look after the query text (which can be long) for the ExecStatusType. Having it separated from the query is more readable IMHO (I know we have a mix of both already, but I kind of prefer passing in the buffer). -- Daniel Gustafsson
On Tue, Dec 16, 2025 at 5:44 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> On 16 Dec 2025, at 23:03, Corey Huinker <corey.huinker@gmail.com> wrote:
> One thing I have noticed is the usage of PQExpBuffer in situations where the query has no optional parts and no string interpolation.
- res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ res = ExecuteSqlQuery(fout,
+ "SELECT DISTINCT attrelid FROM pg_attribute "
+ "WHERE attacl IS NOT NULL",
+ PGRES_TUPLES_OK);
I'm not sure I find it an improvement to put have to look after the query text
(which can be long) for the ExecStatusType. Having it separated from the query
is more readable IMHO (I know we have a mix of both already, but I kind of
prefer passing in the buffer).
I considered replacing them all with the pattern where we assign the block text to a char *querystr, and in fact that's done in the patch in a couple of places where the query was an if/else constant. Is that more acceptable?