Re: Logical Replication of sequences

On Fri, May 23, 2025 at 3:12 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:

>
>
> The attached v20250522 patch has the changes for the same.
>
> Regards,
> Vignesh
>

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