Re: BUG #18558: ALTER PUBLICATION fails with unhelpful error on attempt to use system column

On Thu, Aug 15, 2024 at 8:28 AM David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Wed, 14 Aug 2024 at 16:52, Peter Smith <smithpb2250@gmail.com> wrote:
> > On Mon, Aug 12, 2024 at 5:45 PM David Rowley <dgrowleyml@gmail.com> wrote:
> > > Here's the patch updated to do the validation inside AlterPublicationTables().
> >
> > I think we've come full circle -- your fix is now pretty much the same
> > as my v3 patch [1].
>
> Thanks for having a look at the patch.
>
> I don't agree that this is full circle.

I meant only that the bugfix part is essentially the same -- both
calling the common validating function, from the same place.

> My motivation for the
> additional changes is the API cleanliness of the function you're
> making extern. I mentioned this in my 2nd paragraph in [1]. I didn't
> like that you wanted to make the external function populate an array
> and a Bitmapset with the same information. I think just having the
> Bitmapset is fine and the array adds needless overhead and confusion.

It's not because I wanted to make a strange API; I was only trying to
touch less code by tweaking what was already there instead of
re-writing it.

Your patch is better.

> As for this being backpatched or not. For me, about 51% of me thinks
> this is fine for a master-only fix.

+1

======
Kind Regards,
Peter Smith.
Fujitsu Australia.