On Fri, Dec 31, 2021 at 12:39 AM houzj.f...@fujitsu.com <houzj.f...@fujitsu.com> wrote: > > On Wed, Dec 29, 2021 11:16 AM Tang, Haiying <tanghy.f...@fujitsu.com> wrote: > > On Mon, Dec 27, 2021 9:16 PM houzj.f...@fujitsu.com <houzj.f...@fujitsu.com> > > wrote: > > > > > > On Thur, Dec 23, 2021 4:28 PM Peter Smith <smithpb2...@gmail.com> wrote: > > > > Here is the v54* patch set: > > > > > > Attach the v55 patch set which add the following testcases in 0003 patch. > > > 1. Added a test to cover the case where TOASTed values are not included > > > in the > > > new tuple. Suggested by Euler[1]. > > > > > > Note: this test is temporarily commented because it would fail without > > > applying another bug fix patch in another thread[2] which log the > > detoasted > > > value in old value. I have verified locally that the test pass after > > > applying the bug fix patch[2]. > > > > > > 2. Add a test to cover the case that transform the UPDATE into INSERT. > > Provided > > > by Tang. > > > > > > > Thanks for updating the patches. > > > > A few comments: ... > > 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. ------ Kind Regards, Peter Smith. Fujitsu Australia.
