Thread

1. Simplify the way of appending comma to stringInfo

   Chao Li <li.evan.chao@gmail.com> — 2025-12-08T08:37:30Z

   Hi Hackers,
   
   In a lot places, there are logic of appending comma separators in a pattern
   like:
   
   ```
   for (int i = 0; i < len; i ++)
   {
       if (i > 0)
          appendStringInfoString(", ");
       appendStringInfo(some-item);
   }
   
   ```
   This pattern uses an "if" check and two appendStringInfoString() to build a
   comma-delimited string.
   
   This can be simplified as:
   
   ```
   const char *sep = "";
   for (int i = 0; i < len; i ++)
   {
        appendStringInfo("%s%s", sep, some-item);
        sep = ", ";
   }
   ```
   The new pattern avoids the "if" check, and combines two
   appendStringInfoString() into a single appendStringInfo(). I think the new
   pattern is neater and faster.
   
   The old patterns are used in a lot of places, and there are some usages of
   the new pattern as well. Instead of creating a big cleanup patch, I
   just applied the new pattern to a single file for now to see if the hacker
   group likes this change.
   
   Best regards,
   ==
   Chao Li (Evan)
   ---------------------
   HighGo Software Co., Ltd.
   https://www.highgo.com/
   

2. Re: Simplify the way of appending comma to stringInfo

   Pavel Stehule <pavel.stehule@gmail.com> — 2025-12-08T08:41:10Z

   Hi
   
   po 8. 12. 2025 v 9:37 odesílatel Chao Li <li.evan.chao@gmail.com> napsal:
   
   > Hi Hackers,
   >
   > In a lot places, there are logic of appending comma separators in a
   > pattern like:
   >
   > ```
   > for (int i = 0; i < len; i ++)
   > {
   >     if (i > 0)
   >        appendStringInfoString(", ");
   >     appendStringInfo(some-item);
   > }
   >
   > ```
   > This pattern uses an "if" check and two appendStringInfoString() to build
   > a comma-delimited string.
   >
   > This can be simplified as:
   >
   > ```
   > const char *sep = "";
   > for (int i = 0; i < len; i ++)
   > {
   >      appendStringInfo("%s%s", sep, some-item);
   >      sep = ", ";
   > }
   > ```
   > The new pattern avoids the "if" check, and combines two
   > appendStringInfoString() into a single appendStringInfo(). I think the new
   > pattern is neater and faster.
   >
   > The old patterns are used in a lot of places, and there are some usages of
   > the new pattern as well. Instead of creating a big cleanup patch, I
   > just applied the new pattern to a single file for now to see if the hacker
   > group likes this change.
   >
   
   It doesn't look like a simplification.
   
   Regards
   
   Pavel
   
   
   > Best regards,
   > ==
   > Chao Li (Evan)
   > ---------------------
   > HighGo Software Co., Ltd.
   > https://www.highgo.com/
   >
   

3. Re: Simplify the way of appending comma to stringInfo

   Heikki Linnakangas <hlinnaka@iki.fi> — 2025-12-08T08:43:44Z

   On 08/12/2025 10:37, Chao Li wrote:
   > Hi Hackers,
   > 
   > In a lot places, there are logic of appending comma separators in a 
   > pattern like:
   > 
   > ```
   > for (int i = 0; i < len; i ++)
   > {
   >      if (i > 0)
   >         appendStringInfoString(", ");
   >      appendStringInfo(some-item);
   > }
   > 
   > ```
   > This pattern uses an "if" check and two appendStringInfoString() to 
   > build a comma-delimited string.
   > 
   > This can be simplified as:
   > 
   > ```
   > const char *sep = "";
   > for (int i = 0; i < len; i ++)
   > {
   >       appendStringInfo("%s%s", sep, some-item);
   >       sep = ", ";
   > }
   > ```
   > The new pattern avoids the "if" check, and combines two 
   > appendStringInfoString() into a single appendStringInfo(). I think the 
   > new pattern is neater and faster.
   > 
   > The old patterns are used in a lot of places, and there are some usages 
   > of the new pattern as well. Instead of creating a big cleanup patch, I 
   > just applied the new pattern to a single file for now to see if the 
   > hacker group likes this change.
   
   It's a matter of taste, but I personally prefer the "old" pattern with 
   an explicit if() statement more. And I don't think it's worth the code 
   churn to change existing code either way.
   
   - Heikki
   
   
   
   
   

4. Re: Simplify the way of appending comma to stringInfo

   David Geier <geidav.pg@gmail.com> — 2025-12-08T08:46:44Z

   
   On 08.12.2025 09:43, Heikki Linnakangas wrote:
   > On 08/12/2025 10:37, Chao Li wrote:
   >> Hi Hackers,
   >>
   >> In a lot places, there are logic of appending comma separators in a
   >> pattern like:
   >>
   >> ```
   >> for (int i = 0; i < len; i ++)
   >> {
   >>      if (i > 0)
   >>         appendStringInfoString(", ");
   >>      appendStringInfo(some-item);
   >> }
   >>
   >> ```
   >> This pattern uses an "if" check and two appendStringInfoString() to
   >> build a comma-delimited string.
   >>
   >> This can be simplified as:
   >>
   >> ```
   >> const char *sep = "";
   >> for (int i = 0; i < len; i ++)
   >> {
   >>       appendStringInfo("%s%s", sep, some-item);
   >>       sep = ", ";
   >> }
   >> ```
   >> The new pattern avoids the "if" check, and combines two
   >> appendStringInfoString() into a single appendStringInfo(). I think the
   >> new pattern is neater and faster.
   >>
   >> The old patterns are used in a lot of places, and there are some
   >> usages of the new pattern as well. Instead of creating a big cleanup
   >> patch, I just applied the new pattern to a single file for now to see
   >> if the hacker group likes this change.
   > 
   > It's a matter of taste, but I personally prefer the "old" pattern with
   > an explicit if() statement more. And I don't think it's worth the code
   > churn to change existing code either way.
   > 
   > - Heikki
   
   It's also very likely not faster because now appendStringInfo() is
   called one more time and appendStringInfo() also needs to check if the
   string is empty or not, which requires an if in some form or shape (e.g.
   a loop that bails immediately).
   
   --
   David Geier
   
   
   
   
   

5. Re: Simplify the way of appending comma to stringInfo

   Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-08T15:09:15Z

   Heikki Linnakangas <hlinnaka@iki.fi> writes:
   > On 08/12/2025 10:37, Chao Li wrote:
   >> The old patterns are used in a lot of places, and there are some usages 
   >> of the new pattern as well. Instead of creating a big cleanup patch, I 
   >> just applied the new pattern to a single file for now to see if the 
   >> hacker group likes this change.
   
   > It's a matter of taste, but I personally prefer the "old" pattern with 
   > an explicit if() statement more. And I don't think it's worth the code 
   > churn to change existing code either way.
   
   Yeah, the "code churn" objection is a big one in my mind.  Any
   cosmetic change like this creates merge hazards for back-patching
   bug fixes in nearby code.  Of course any one such change has only a
   very low risk of causing back-patching pain, but if you are proposing
   to do it in dozens or hundreds of places then the risk becomes much
   greater.  Cosmetic changes also make it more painful to identify the
   origin of code segments via "git blame".  I don't think this is such a
   huge improvement as to justify those costs.
   
   			regards, tom lane