At Wed, 2 Mar 2022 16:46:01 +0900, Michael Paquier wrote
in
> Hi all,
>
> In my hunt looking for incorrect SRFs, I have noticed a new case of a
> system function marked as proretset while it builds and returns only
> one record. And this is a popular one: pg_stop_backup(), labelled
> v2.
>
>
On Wed, Mar 02, 2022 at 05:22:35PM +0900, Kyotaro Horiguchi wrote:
> But the patch forgets to remove an useless variable.
Indeed. I forgot to look at stderr.
>> /* Initialise attributes information in the tuple descriptor */
>> tupdesc = CreateTemplateTupleDesc(PG_STOP_BACKUP_V2_COLS);
Hi Michael,
```
Datum
pg_stop_backup_v2(PG_FUNCTION_ARGS)
{
-ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+#define PG_STOP_BACKUP_V2_COLS 3
TupleDesctupdesc;
-Tuplestorestate *tupstore;
-MemoryContext per_query_ctx;
-MemoryContext oldcontext;
-Datum
On Wed, Mar 2, 2022 at 5:25 AM Aleksander Alekseev
wrote:
> ```
> Datum
> pg_stop_backup_v2(PG_FUNCTION_ARGS)
> {
> -ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
> +#define PG_STOP_BACKUP_V2_COLS 3
> TupleDesctupdesc;
> -Tuplestorestate *tupstore;
> -Memory
Robert Haas writes:
> On Wed, Mar 2, 2022 at 5:25 AM Aleksander Alekseev
> wrote:
>> Declaring a macro inside the procedure body is a bit unconventional.
>> 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 thi
Hi Tom.
Yeah, there's plenty of precedent for that coding if you look around.
> I've not read the whole patch, but this snippet seems fine to me
> if there's also an #undef at the end of the function.
>
No, there is no #undef. With #undef I don't mind it either.
--
Best regards,
Aleksander Alek
On Wed, Mar 02, 2022 at 05:40:00PM +0300, Aleksander Alekseev wrote:
> Hi Tom.
>
> Yeah, there's plenty of precedent for that coding if you look around.
> > I've not read the whole patch, but this snippet seems fine to me
> > if there's also an #undef at the end of the function.
>
> No, there is no
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
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 d
On Thu, Mar 03, 2022 at 12:36:32AM +0800, Julien Rouhaud wrote:
> I don't see strong evidence for that pattern being wildly used with some naive
> grepping:
Yes, I don't recall either seeing the style with an undef a lot when
it came to system functions. I'll move on and apply the fix in a
minute
On Wed, Mar 02, 2022 at 12:04:59PM -0500, Chapman Flack wrote:
> I had just recently noticed that while reviewing [0], but shrugged,
> as I didn't know what the history was.
Okay. I did not see you mention it on the thread, but the discussion
is long so it is easy to miss some of its details.
>
On Wed, Mar 2, 2022 at 9:35 AM Tom Lane wrote:
> Yeah, there's plenty of precedent for that coding if you look around.
> I've not read the whole patch, but this snippet seems fine to me
> if there's also an #undef at the end of the function.
>From later emails, it sounds like that's not the commo
Robert Haas writes:
> On Wed, Mar 2, 2022 at 9:35 AM Tom Lane wrote:
>> I've not read the whole patch, but this snippet seems fine to me
>> if there's also an #undef at the end of the function.
>> From later emails, it sounds like that's not the common practice in
> similar cases, and I don't pe
On 03/03/22 16:40, Tom Lane wrote:
> The point is to make it clear that the macro isn't intended to affect
> code outside the function. Since C lacks block-scoped macros,
> there's no other way to do that.
>
> I concede that a lot of our code is pretty sloppy about this, but
> that doesn't make i
Chapman Flack writes:
> On 03/03/22 16:40, Tom Lane wrote:
>> The point is to make it clear that the macro isn't intended to affect
>> code outside the function. Since C lacks block-scoped macros,
>> there's no other way to do that.
> Would the
> Datum values[3];
> bool nulls[ lengthof(va
On Thu, Mar 03, 2022 at 04:40:42PM -0500, Tom Lane wrote:
> The point is to make it clear that the macro isn't intended to affect
> code outside the function. Since C lacks block-scoped macros,
> there's no other way to do that.
>
> I concede that a lot of our code is pretty sloppy about this, bu
At Fri, 4 Mar 2022 10:09:19 +0900, Michael Paquier wrote
in
> On Thu, Mar 03, 2022 at 04:40:42PM -0500, Tom Lane wrote:
> > The point is to make it clear that the macro isn't intended to affect
> > code outside the function. Since C lacks block-scoped macros,
> > there's no other way to do that
17 matches
Mail list logo
