Re: Logical Replication - detail message with names of missing columns

On Tue, Oct 6, 2020 at 5:24 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Oct 6, 2020 at 12:14 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > On Mon, Oct 5, 2020 at 9:39 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > >
> > > 3. The patch doesn't seem to be freeing the memory allocated for missingattsbuf.
> > >
> >
> > I don't think we need to do that. We are passing missingattsbuf.data to ereport and we are safe without freeing up missingattsbuf(we don't reach the code after ereprot(ERROR,...)as the table sync worker anyways goes away after throwing missing attributes error( if (sigsetjmp(local_sigjmp_buf, 1) != 0) in StartBackgroundWorker and then proc_exit(1)). Each time a new table sync bg worker is spawned.
> >
>
> Okay, by that logic, we don't even need to free memory for missingatts.
>
> I have made a few changes, (a) moved free of missingatts in the caller
> where we are allocating it. (b) added/edited/removed comments, (c) ran
> pgindent.
>

Thanks Amit. v8 patch looks good to me.

>
> Shall we backpatch this? I don't see any particular need to backpatch
> this as this is a minor error message improvement and nobody has
> reported any problem due to this. What do you think?
>

IMO, no backpatch is required as this is not a bug or something
reported by anyone.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com