On Monday, October 13, 2025 3:00 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sat, Oct 11, 2025 at 7:42 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > The attached patch has the changes for the same.
> >
>
> 5.
> AlterSubscription_refresh()
> - sub_remove_rels[remove_rel_len].relid = relid;
> - sub_remove_rels[remove_rel_len++].state = state;
> …
> - char originname[NAMEDATALEN];
> + SubRemoveRels *rel = palloc(sizeof(SubRemoveRels));
> +
> + rel->relid = relid;
> + rel->state = state;
> +
> + sub_remove_rels = lappend(sub_remove_rels, rel);
>
> Why do we change an offset based array into list? It looks slightly
> odd that in the same function one of the other similar array
> pubrel_local_oids is not converted when the above is converted. And
> even if we do so, I don't think we need a retail free
> (list_free_deep(sub_remove_rels);) as the memory allocation here is in
> portal context which should be reset by end of the current statement
> execution.
when sub_remove_rels was Array type, we used subrel_count as the initial size of
this array, but this count means the number of both the table and sequences. So
it allocates some unnecessary space for it, which is why we changed it to List.
Based on above, I kept the current style for now but removed the list_free.
All other comments have been addressed in the latest version.
Best Regards,
Hou zj
