RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

On Mon, Mar 13, 2023 10:16 PM Önder Kalacı <onderkalaci(at)gmail(dot)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