Re: Skipping logical replication transactions on subscriber side

On 21.01.22 04:08, Masahiko Sawada wrote:
>> I think the superuser check in AlterSubscription() might no longer be
>> appropriate.  Subscriptions can now be owned by non-superusers.  Please
>> check that.
> 
> IIUC we don't allow non-superuser to own the subscription yet. We
> still have the following superuser checks:
> 
> In CreateSubscription():
> 
>      if (!superuser())
>          ereport(ERROR,
>                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>                   errmsg("must be superuser to create subscriptions")));
> 
> and in AlterSubscriptionOwner_internal();
> 
>      /* New owner must be a superuser */
>      if (!superuser_arg(newOwnerId))
>          ereport(ERROR,
>                  (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>                   errmsg("permission denied to change owner of
> subscription \"%s\"",
>                          NameStr(form->subname)),
>                   errhint("The owner of a subscription must be a superuser.")));
> 
> Also, doing superuser check here seems to be consistent with
> pg_replication_origin_advance() which is another way to skip
> transactions and also requires superuser permission.

I'm referring to commit a2ab9c06ea15fbcb2bfde570986a06b37f52bcca.  You 
still have to be superuser to create a subscription, but you can change 
the owner to a nonprivileged user and it will observe table permissions 
on the subscriber.

Assuming my understanding of that commit is correct, I think it would be 
sufficient in your patch to check that the current user is the owner of 
the subscription.