Re: pg_stop_backup() v2 incorrectly marked as proretset

From: David Steele <david(at)pgmasters(dot)net>
To: Chapman Flack <chap(at)anastigmatix(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_stop_backup() v2 incorrectly marked as proretset
Date: 2022-03-02 17:24:24
Message-ID: c81de7f1-2551-5fa6-ccb9-fd9aada86cd9@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 3/2/22 11:04, Chapman Flack wrote:
> On 03/02/22 02:46, Michael Paquier wrote:
>> system function marked as proretset while it builds and returns only
>> one record. And this is a popular one: pg_stop_backup(), labelled
>> v2.
>
> I had just recently noticed that while reviewing [0], but shrugged,
> as I didn't know what the history was.
>
> Is this best handled as a separate patch, or folded into [0], which is
> going to be altering and renaming that function anyway?
>
>
> On 03/02/22 09:31, Robert Haas wrote:
>> On Wed, Mar 2, 2022 at 5:25 AM Aleksander Alekseev
>>> Since it doesn't seem to be used for anything except these two array
>>> declarations I suggest keeping simply "3" here.
>>
>> I think we do this kind of thing in various places in similar
>> situations, and I think it is good style. It makes it easier to catch
>> everything if you ever need to update the code.
>
>
> I've been known (in other projects) to sometimes accomplish the same
> thing with, e.g.,
>
> Datum values[3];
> bool nulls[sizeof values / sizeof *values];

I also use this pattern, though I would generally write it as:

bool nulls[sizeof(values) / sizeof(Datum)];

Chap's way makes it possible to use a macro, though, so that's a plus.

Regards,
-David

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-03-02 17:28:17 Re: Commitfest 2022-03 Patch Triage Part 1a.i
Previous Message David Steele 2022-03-02 17:17:34 Re: Allow root ownership of client certificate key