From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
---|---|
To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Clean up command argument assembly |
Date: | 2023-07-05 05:22:33 |
Message-ID: | b555363f-3c56-5761-6f10-e96004a12ad8@eisentraut.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 04.07.23 14:14, Heikki Linnakangas wrote:
> On 26/06/2023 12:33, Peter Eisentraut wrote:
>> This is a small code cleanup patch.
>>
>> Several commands internally assemble command lines to call other
>> commands. This includes initdb, pg_dumpall, and pg_regress. (Also
>> pg_ctl, but that is different enough that I didn't consider it here.)
>> This has all evolved a bit organically, with fixed-size buffers, and
>> various optional command-line arguments being injected with
>> confusing-looking code, and the spacing between options handled in
>> inconsistent ways. This patch cleans all this up a bit to look clearer
>> and be more easily extensible with new arguments and options.
>
> +1
committed
>> We start each command with printfPQExpBuffer(), and then append
>> arguments as necessary with appendPQExpBuffer(). Also standardize on
>> using initPQExpBuffer() over createPQExpBuffer() where possible.
>> pg_regress uses StringInfo instead of PQExpBuffer, but many of the
>> same ideas apply.
>
> It's a bit bogus to use PQExpBuffer for these. If you run out of memory,
> you silently get an empty string instead. StringInfo, which exits the
> process on OOM, would be more appropriate. We have tons of such
> inappropriate uses of PQExpBuffer in all our client programs, though, so
> I don't insist on fixing this particular case right now.
Interesting point. But as you say better dealt with as a separate problem.
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2023-07-05 05:46:42 | Re: doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table. |
Previous Message | Michael Paquier | 2023-07-05 05:10:42 | Re: Add more sanity checks around callers of changeDependencyFor() |