On Mon, Mar 13, 2023 10:16 PM Önder Kalacı <onderkal...@gmail.com> wrote: > > Attaching v47. >
Thanks for updating the patch. Here are some comments. 1. in RemoteRelContainsLeftMostColumnOnIdx(): + if (indexInfo->ii_NumIndexAttrs < 1) + return false; Did you see any cases that the condition is true? I think there is at least one column in the index. If so, we can use an Assert(). + if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol)) + return false; Similarly, I think `attrmap->maplen` is the number of columns and it is always greater than keycol. If you agree, we can check it with an Assert(). Besides, It seems we don't need AttrNumberGetAttrOffset(). 2. +# make sure that the subscriber has the correct data after the update UPDATE "update UPDATE" seems to be a typo. 3. +# now, drop the index with the expression, and re-create index on column lastname The comment says "re-create index on column lastname" but it seems we didn't do that, should it be modified to something like: # now, drop the index with the expression, we will use sequential scan Besides these, the patch LGTM. Regards, Shi Yu
