On Tue, May 12, 2026 at 8:36 PM Etsuro Fujita <[email protected]> wrote:
> On Fri, May 1, 2026 at 3:04 AM Corey Huinker <[email protected]> wrote:
> > The second gets rid of the string buffers entirely, and instead frees the 
> > nested object.
> >
> > I have no preference between the two, though I suspect that the nested-free 
> > solution will be preferred by the community.
>
> ISTM that the second solution is the right direction to go in.
>
> +static void
> +free_remattrmap(RemoteAttributeMapping *map, int len)
> +{
> +   if (!map)
> +       return;
> +
> +   for (int i = 0; i < len; i++)
> +   {
> +       if (map[i].local_attname)
> +           pfree(map[i].local_attname);
> +       if (map[i].remote_attname)
> +           pfree(map[i].remote_attname);
> +   }
>
> The if tests for local_attname/remote_attname should be assertions?

I mean that by the definition of build_remattrmap(), the
local_attname/remote_attname cannot be NULL if the given map isn't
NULL, so the if tests are just a waste of cycles.  So I modified the
patch as such.  Updated patch attached.  I also removed an extra
change made to RemoteAttributeMapping, which seemed irrelevant to this
issue to me.

Best regards,
Etsuro Fujita

Attachment: v1-0001-Replace-name-buffers-from-RemoteAttributeMapping-efujita.patch
Description: Binary data

Reply via email to