On Tue, Jan 4, 2022 at 12:15 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Fri, Dec 31, 2021 at 12:39 AM houzj.fnst@fujitsu.com
> <houzj.fnst@fujitsu.com> wrote:
> > > 3) v55-0002
> > > +static bool pgoutput_row_filter_update_check(enum
> > > ReorderBufferChangeType changetype, Relation relation,
> > > +
> > > HeapTuple oldtuple, HeapTuple newtuple,
> > > +
> > > RelationSyncEntry *entry, ReorderBufferChangeType *action);
> > >
> > > Do we need parameter changetype here? I think it could only be
> > > REORDER_BUFFER_CHANGE_UPDATE.
> >
> > I didn't change this, I think it might be better to wait for Ajin's opinion.
>
> I agree with Tang. AFAIK there is no problem removing that redundant
> param as suggested. BTW - the Assert within that function is also
> incorrect because the only possible value is
> REORDER_BUFFER_CHANGE_UPDATE. I will make these fixes in a future
> version.
>
That sounds fine to me too. One more thing is that you don't need to
modify the action in case it remains update as the caller has already
set that value. Currently, we are modifying it as update at two places
in this function, we can remove both of those and keep the comments
intact for the later update.
--
With Regards,
Amit Kapila.