Re: Clean up command argument assembly

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: 2024-02-04 17:48:08
Message-ID: 2be4fee5-738f-4749-b9f8-b452032c7ade@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 05.07.23 07:22, Peter Eisentraut wrote:
>> 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.

I was inspired by a33e17f210 (for pg_rewind) to clean up some more
fixed-buffer command assembly and replace it with extensible buffers and
some more elegant code. And then I remembered this thread, and it's
really a continuation of this.

The first patch deals with pg_regress and pg_isolation_regress. It is
pretty straightforward.

The second patch deals with pg_upgrade. It would require exporting
appendPQExpBufferVA() from libpq, which might be overkill. But this
gets to your point earlier: Should pg_upgrade rather be using
StringInfo instead of PQExpBuffer? (Then we'd use appendStringInfoVA(),
which already exists, but even if not we wouldn't need to change libpq
to get it.) Should anything outside of libpq be using PQExpBuffer?

Attachment Content-Type Size
0001-Use-extensible-buffers-to-assemble-command-lines.patch text/plain 4.8 KB
0002-WIP-pg_upgrade-No-more-command-too-long.patch text/plain 3.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David E. Wheeler 2024-02-04 17:51:29 Patch: Add parse_type Function
Previous Message Andrey M. Borodin 2024-02-04 17:40:17 Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock