Thread

1. pg_dump: Remove trivial usage of PQExpBuffer

   Corey Huinker <corey.huinker@gmail.com> — 2025-12-16T22:03:13Z

   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.
   

2. Re: pg_dump: Remove trivial usage of PQExpBuffer

   Daniel Gustafsson <daniel@yesql.se> — 2025-12-16T22:44:28Z

   > 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).
   
   --
   Daniel Gustafsson
   
   
   
   
   

3. Re: pg_dump: Remove trivial usage of PQExpBuffer

   Corey Huinker <corey.huinker@gmail.com> — 2025-12-16T23:02:21Z

   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?
   

4. Re: pg_dump: Remove trivial usage of PQExpBuffer

   Peter Eisentraut <peter@eisentraut.org> — 2025-12-17T15:15:21Z

   On 16.12.25 23:03, Corey Huinker wrote:
   > 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.
   
   I'm not sure this is better.  It seems better to me to use consistent 
   APIs throughout.  Kind of like using printf even if you don't need to 
   substitute anything, rather than using a mix of printf and puts.
   
   
   
   

5. Re: pg_dump: Remove trivial usage of PQExpBuffer

   Andres Freund <andres@anarazel.de> — 2025-12-17T16:50:32Z

   Hi,
   
   On 2025-12-17 16:15:21 +0100, Peter Eisentraut wrote:
   > On 16.12.25 23:03, Corey Huinker wrote:
   > > 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.
   > 
   > I'm not sure this is better.  It seems better to me to use consistent APIs
   > throughout.  Kind of like using printf even if you don't need to substitute
   > anything, rather than using a mix of printf and puts.
   
   It also just seems like a pain for backpatching. If this were a huge
   improvement or if the code in question was being newly added, it'd perhaps be
   a different story, but as is...
   
   Greetings,
   
   Andres Freund
   
   
   
   

6. Re: pg_dump: Remove trivial usage of PQExpBuffer

   Corey Huinker <corey.huinker@gmail.com> — 2025-12-17T19:38:27Z

   >
   >
   > It also just seems like a pain for backpatching. If this were a huge
   > improvement or if the code in question was being newly added, it'd perhaps
   > be
   > a different story, but as is...
   
   
   Yeah, it's not a "wow" level cleanup. I'll consider this one dropped.