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().
Actually, it was mostly to guard against any edge cases. I thought similar to tables,
we can have zero column indexes, but it turns out it is not possible. Also,
index_create seems to check that, so changing it asset makes sense:
/*
* check parameters
*/
if (indexInfo->ii_NumIndexAttrs < 1)
elog(ERROR, "must index at least one column");
+ 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().
At this point, I'm really hesitant to make any assumptions. Logical replication
is pretty flexible, and who knows maybe dropped columns will be treated
differently at some point, and this assumption changes?
I really feel more comfortable to keep this as-is. We call this function very infrequently
anyway.
Besides, It
seems we don't need AttrNumberGetAttrOffset().
Why not? We are accessing the AttrNumberGetAttrOffset(keycol) element
of the array attnums?
2.
+# make sure that the subscriber has the correct data after the update UPDATE
"update UPDATE" seems to be a typo.
thanks, fixed
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
Thanks, fixed
I'll add the changes to v49 in the next e-mail.
Thanks,
Onder KALACI
