Re: Handle infinite recursion in logical replication setup

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>
Subject: Re: Handle infinite recursion in logical replication setup
Date: 2022-09-07 11:40:14
Message-ID: CALDaNm3tv+nWMXO0q39EuwzbXEQyF5thT4Ha1PvfQ+fQgSdi_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 7 Sept 2022 at 14:34, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Sep 7, 2022 at 9:53 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > Thanks for the comments, the attached v47 patch has the changes for the same.
> >
>
> V47-0001* looks good to me apart from below minor things. I would like
> to commit this tomorrow unless there are more comments on it.
>
> Few minor suggestions:
> ==================
> 1.
> + list_free_deep(publist);
> + pfree(pubnames->data);
> + pfree(pubnames);
>
> I don't think we need to free this memory as it will automatically be
> released at end of the command (this memory is allocated in portal
> context). I understand there is no harm in freeing it as well but
> better to leave it as there doesn't appear to be any danger of leaking
> memory for a longer time.

Modified

> 2.
> + res = walrcv_exec(wrconn, cmd.data, 1, tableRow);
> + pfree(cmd.data);
>
> Don't you need to free cmd as well here? I think it is better to avoid
> freeing it due to reasons mentioned in the previous point.

cmd need not be freed here as we use initStringInfo for initialization
and initStringInfo allocates memory for data member only.

The attached patch has the changes for the same.

Regards,
Vignesh

Attachment Content-Type Size
v48-0001-Raise-a-warning-if-there-is-a-possibility-of-dat.patch application/octet-stream 19.8 KB
v48-0002-Document-the-steps-for-replication-between-prima.patch application/octet-stream 20.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Melih Mutlu 2022-09-07 11:51:25 Re: Allow logical replication to copy tables in binary format
Previous Message Amit Kapila 2022-09-07 10:48:48 Re: Column Filtering in Logical Replication