On Mon, Mar 13, 2023 10:16 PM Önder Kalacı <onderkalaci@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