Hi,
I couldn't test the patch as it does not apply cleanly on master.
Please find below some review comments:
1. Would it better to throw a warning at the time of dropping the
REPLICA IDENTITY
index that it would also drop the REPLICA IDENTITY of the parent table?
2. CCI is used after calling SetRelationReplIdent from index_drop() but not
after following call
relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
bool is_internal)
+ /* update relreplident if necessary */
+ SetRelationReplIdent(RelationGetRelid(rel), ri_type);
3. I think the former variable names `pg_class_tuple` and
`pg_class_form`provided better clarity
+ HeapTuple tuple;
+ Form_pg_class classtuple;
- relreplident is switched to REPLICA_IDENTITY_NOTHING, which is the
behavior we have now after an index is dropped, even if there is a
primary key.
4. I am not aware of the reason of the current behavior, however it
seems better
to switch to REPLICA_IDENTITY_DEFAULT. Can't think of a reason why user
would not be
fine with using primary key as replica identity when replica identity
index is dropped
Thank you,
