Re: More tests with USING INDEX replident and dropped indexes

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,