| 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/
| 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 |