Horton

Re: Handle infinite recursion in logical replication setup

vignesh C <vignesh21@gmail.com>

View thread View on postgr.es
From: vignesh C <vignesh21@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, Peter Smith <smithpb2250@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Dilip Kumar <dilipbalaut@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2022-09-07T11:40:14Z
Lists: pgsql-hackers

Attachments

On Wed, 7 Sept 2022 at 14:34, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Sep 7, 2022 at 9:53 AM vignesh C <vignesh21@gmail.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

Commits

  1. Improved CREATE SUBSCRIPTION message for clarity

  2. Fix the test case introduced by commit 8756930190.

  3. Raise a warning if there is a possibility of data from multiple origins.

  4. Add wait_for_subscription_sync for TAP tests.

  5. Bump catversion for commit d8cd0c6c95c0120168df93aae095df4e0682a08a.

  6. Allow users to skip logical replication of data having origin.

  7. Improve two comments related to a boolean DefElem's value

  8. doc: Fix man page whitespace issues