Re: Use stack-allocated StringInfoData

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Mats Kindahl <mats(dot)kindahl(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Use stack-allocated StringInfoData
Date: 2025-11-05 14:40:20
Message-ID: 7222B031-72E6-43A1-859A-FF2FA6F67A2D@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Nov 5, 2025, at 19:01, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> It's not your fault, but check_publications_origin_tables() looks like
> a mess. It seems like excessive use of additional code just to avoid
> two separate ereport() calls. Might be worth a follow-up patch to
> clean that up.
>
> The patch looks good to me. The only thing I'd adjust is to get rid of
> the "data" variable from build_backup_content(), which I think you
> mentioned above. I can take care of that one.
>
> I can push this tomorrow morning UTC+13. I'll leave it til then in
> case anyone else has any comments.

No objection. But I just find that the patch missed one place in check_publications():

```
if (list_length(publicationsCopy))
{
/* Prepare the list of non-existent publication(s) for error message. */
StringInfo pubnames = makeStringInfo();

GetPublicationsStr(publicationsCopy, pubnames, false);
ereport(WARNING,
errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg_plural("publication %s does not exist on the publisher",
"publications %s do not exist on the publisher",
list_length(publicationsCopy),
pubnames->data));
}
```

This “pubnames” can be replaced as well, and “pfree(pubnames.data)” should be added after “ereport”. As one place in the function has been replaced in this patch, I guess we should not leave this place to a separate patch.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2025-11-05 14:44:48 Re: Logical Replication of sequences
Previous Message Tomas Vondra 2025-11-05 14:05:48 Re: Changing the state of data checksums in a running cluster