Re: Logical Replication of sequences

Some review comments for patch 0001:

1. In src/backend/commands/sequence.c
in pg_sequence_state()
+ /* open and lock sequence */
+ init_sequence(seq_relid, &elm, &seqrel);
+
+ if (pg_class_aclcheck(elm->relid, GetUserId(),
+  ACL_SELECT | ACL_USAGE) != ACLCHECK_OK)
+ ereport(ERROR,
+ errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied for sequence %s",
+   RelationGetRelationName(seqrel)));
+

How about using aclcheck_error for this, which also supports error messages for specific access errors. Most other objects seem to be using this.
if (aclresult != ACLCHECK_OK)
aclcheck_error(aclresult, get_relkind_objtype(seqrel->relkind),
  RelationGetRelationName(seqrel));
 
2. in function pg_sequence_state()
+
+ UnlockReleaseBuffer(buf);
+ relation_close(seqrel, NoLock);

Ideally the corresponding close for init_sequence is sequence_close() rather than relation_close()

I will post comments for the other patches as well.

regards,

Ajin Cherian

Fujitsu Australia