On Wednesday, September 27, 2023 12:45 PM Amit Kapila <amit.kapil...@gmail.com> > > On Wed, Sep 27, 2023 at 9:46 AM Michael Paquier <mich...@paquier.xyz> > wrote: > > > > On Wed, Sep 27, 2023 at 09:39:19AM +0530, Amit Kapila wrote: > > > On Wed, Sep 27, 2023 at 9:10 AM Michael Paquier <mich...@paquier.xyz> > wrote: > > >> Err, actually, I am going to disagree here for the patch of HEAD. > > >> It seems to me that there is zero need for pgoutput.h and we don't > > >> need to show PGOutputData to the world. The structure is internal > > >> to Pgoutput.c and used only by its internal static routines. > > > > > > Do you disagree with the approach for the PG16 patch or HEAD? You > > > mentioned HEAD but your argument sounds like you disagree with a > > > different approach for PG16. > > > > Only HEAD where the structure should be moved from pgoutput.h to > > pgoutput.c, IMO. > > > > It's like that from the beginning. Now, even if we want to move, your > suggestion is not directly related to this patch as we are just changing one > field, > and that too to fix a bug. We should start a separate thread to gather a > broader > consensus if we want to move the exposed structure to an internal file.
While searching the code, I noticed one postgres fork where the PGoutputData is used in other places, although it's a separate fork, but it seems better to discuss the removal separately. [1] https://github.com/Tencent/TBase/blob/7cf7f8afbcab7290538ad5e65893561710be3dfa/src/backend/replication/walsender.c#L100 Best Regards, Hou zj
