Hi, There is a buildfarm failure on mylodon at [1] because of the new test added. I will analyse and share the findings for the same.
[1] - https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mylodon&dt=2022-09-08%2001%3A40%3A27 Regards, Vignesh On Wed, 7 Sept 2022 at 17:10, vignesh C <vignes...@gmail.com> wrote: > > On Wed, 7 Sept 2022 at 14:34, Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Wed, Sep 7, 2022 at 9:53 AM vignesh C <vignes...@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
