Missing list_free in publicationcmds.c:OpenTableList

Started by Peter Smith14 days ago5 messageshackers
Jump to latest
#1Peter Smith
smithpb2250@gmail.com

FWIW, I detected a missing `list_free`, which seems to have been
accidentally left out from commit 923def9.

Added:
list_free(relids_with_collist);

Consistent with the existing:
list_free(relids_with_rf);

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v1-0001-Add-missing-list_free.patchapplication/octet-stream; name=v1-0001-Add-missing-list_free.patchDownload+1-1
#2shveta malik
shveta.malik@gmail.com
In reply to: Peter Smith (#1)
Re: Missing list_free in publicationcmds.c:OpenTableList

On Tue, Jul 28, 2026 at 10:30 AM Peter Smith <smithpb2250@gmail.com> wrote:

FWIW, I detected a missing `list_free`, which seems to have been
accidentally left out from commit 923def9.

Added:
list_free(relids_with_collist);

Consistent with the existing:
list_free(relids_with_rf);

Okay since list_free for both relids and relids_with_rf are there, I
think list_free for 'relids_with_rf' also make sense. So LGTM.

thanks
Shveta

#3shihao zhong
zhong950419@gmail.com
In reply to: shveta malik (#2)
Re: Missing list_free in publicationcmds.c:OpenTableList

On Tue, Jul 28, 2026 at 11:44 PM shveta malik <shveta.malik@gmail.com> wrote:

On Tue, Jul 28, 2026 at 10:30 AM Peter Smith <smithpb2250@gmail.com> wrote:

FWIW, I detected a missing `list_free`, which seems to have been
accidentally left out from commit 923def9.

Added:
list_free(relids_with_collist);

Consistent with the existing:
list_free(relids_with_rf);

Okay since list_free for both relids and relids_with_rf are there, I
think list_free for 'relids_with_rf' also make sense. So LGTM.

thanks
Shveta

This is a great find, LGTM!

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: shihao zhong (#3)
Re: Missing list_free in publicationcmds.c:OpenTableList

shihao zhong <zhong950419@gmail.com> writes:

On Tue, Jul 28, 2026 at 11:44 PM shveta malik <shveta.malik@gmail.com> wrote:

On Tue, Jul 28, 2026 at 10:30 AM Peter Smith <smithpb2250@gmail.com> wrote:

FWIW, I detected a missing `list_free`, which seems to have been
accidentally left out from commit 923def9.

Okay since list_free for both relids and relids_with_rf are there, I
think list_free for 'relids_with_rf' also make sense. So LGTM.

AFAICS, that function is executed at most once per DDL command,
in a command-lifetime memory context. I think the right fix
is not to add another list_free but to remove the misleadingly
useless ones that are there.

regards, tom lane

#5Peter Smith
smithpb2250@gmail.com
In reply to: Tom Lane (#4)
Re: Missing list_free in publicationcmds.c:OpenTableList

On Wed, Jul 29, 2026 at 2:35 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

shihao zhong <zhong950419@gmail.com> writes:

On Tue, Jul 28, 2026 at 11:44 PM shveta malik <shveta.malik@gmail.com> wrote:

On Tue, Jul 28, 2026 at 10:30 AM Peter Smith <smithpb2250@gmail.com> wrote:

FWIW, I detected a missing `list_free`, which seems to have been
accidentally left out from commit 923def9.

Okay since list_free for both relids and relids_with_rf are there, I
think list_free for 'relids_with_rf' also make sense. So LGTM.

AFAICS, that function is executed at most once per DDL command,
in a command-lifetime memory context. I think the right fix
is not to add another list_free but to remove the misleadingly
useless ones that are there.

I added list_free to be consistent with the existing logic.

Removing everything is also a consistent treatment for all lists. Done
as suggested in patch v2.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v2-0001-Remove-unnecessary-list_free.patchapplication/octet-stream; name=v2-0001-Remove-unnecessary-list_free.patchDownload+0-4