Dear Vignesh,
I played with your patch and found something.
01.
In LogicalRepSyncSequences() and GetSubscriptionRelations(), there is a
possibility
that the sequence on the subscriber could be dropped before opens that.
This can cause `could not open relation with OID %u` error, which is not
user-friendly.
Can we avoid that? Even if it is difficult we should add ereport().
02.
```
/*
* Check that our sequencesync worker has permission to insert
into
* the target sequence.
*/
aclresult = pg_class_aclcheck(RelationGetRelid(sequence_rel),
GetUserId(),
ACL_INSERT);
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult,
get_relkind_objtype(sequence_rel->rd_rel->relkind),
seqname);
```
Hmm, but upcoming SetSequence() needs UPDATE privilege. I feel it should be
checked.
03.
Similar with 1, sequences can be dropped just before entering copy_sequences().
This can cause `cache lookup failed for sequence` error, which cannot be
translated.
Can we avoid that or change the error-function to erport()?
04.
```
if (message_level_is_interesting(DEBUG1))
ereport(DEBUG1,
errmsg_internal("logical replication synchronization for subscription \"%s\",
sequence \"%s.%s\" has finished",
MySubscription->name,
seqinfo->nspname,
seqinfo->seqname));
```
I feel no need to add if-statement because we do not touch additional data here.
05.
```
list_free_deep(sequences_to_copy);
```
IIUC, this function free's each elements and list itself, but they do no-op for
attributes of elements. Can we pfree() for seqname and nspname?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
