From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
cc: Peter Smith <smithpb2250@gmail.com>, exclusion@gmail.com,
        pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18558: ALTER PUBLICATION fails with unhelpful error on
 attempt to use system column
In-reply-to: 
 <CAApHDvo2i1j_iCFcURx5q7jYe70qk4Ca7J+8Dt9_jSMOdooAOA@mail.gmail.com>
References: <18558-411bc81b03592125@postgresql.org>
 <CAHut+Pt88zYZfWLj6jnMrQw1CwxeYtp1TmF7vrZ2F6Cyvb=5yQ@mail.gmail.com>
 <2455740.1722219823@sss.pgh.pa.us>
 <CAHut+PtDehGqTDo24+jYQ_eo33geJ_4X+ObkFpw18SU_a6U6pw@mail.gmail.com>
 <2460423.1722222076@sss.pgh.pa.us>
 <CAHut+PtUPP0gC2HqKT8_ouZWeyxJMzeLOYtWo=UsyN29NY8dew@mail.gmail.com>
 <CAApHDvo2i1j_iCFcURx5q7jYe70qk4Ca7J+8Dt9_jSMOdooAOA@mail.gmail.com>
Comments: In-reply-to David Rowley <dgrowleyml@gmail.com>
	message dated "Mon, 29 Jul 2024 18:14:08 +1200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2580481.1722261962.1@sss.pgh.pa.us>
Date: Mon, 29 Jul 2024 10:06:02 -0400
Message-ID: <2580482.1722261962@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2580482.1722261962%40sss.pgh.pa.us>
Precedence: bulk

David Rowley <dgrowleyml@gmail.com> writes:
> On the whole, I think the API of these functions could be improved as
> it's made the fix for this bug more convoluted than it needs to be.

+1

> I'm on the fence if this should be done as part of this bug fix.  The
> reason I think it might is that you're changing
> publication_translate_columns() to be non-static, and if the above API
> change gets done later, that's then changing the API of an existing
> external function. The reason against is that it's more risky to do in
> the back branches as it's changing more code. What do others think?

If we are going to export a formerly-static function, we should make
its API as clean as we can.  I doubt that that adds meaningful risk,
and it avoids people possibly getting bit by cross-branch differences.

> Is it worth adding an ALTER PUBLICATION test with a duplicate column too?

Probably, just to prove that the non-duplicative representation does
what we want.

			regards, tom lane