On Tue, Sep 26, 2023 at 01:55:10PM +0000, Zhijie Hou (Fujitsu) wrote:
> On Tuesday, September 26, 2023 4:40 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>> Do we really need a new parameter in above structure? Can't we just use the
>> existing origin in the same structure? Please remember if this needs to be
>> backpatched then it may not be good idea to add new parameter in the
>> structure but apart from that having two members to represent similar
>> information doesn't seem advisable to me. I feel for backbranch we can just use
>> PGOutputData->origin for comparison and for HEAD, we can remove origin
>> and just use a boolean to avoid any extra cost for comparisions for each
>> change.
>
> OK, I agree to remove the origin string on HEAD and we can add that back
> when we support other origin value. I also modified to use the string for comparison
> as suggested for back-branch. I will also test it locally to confirm it doesn't affect
> the perf.
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.
Doing a codesearch in the Debian repos or just github shows that it is
used nowhere else, as well, something not really surprising as the
structure is filled and maintained in the file.
--
Michael
