On Wed, Mar 8, 2023 at 2:47 PM Andres Freund <and...@anarazel.de> wrote: > Hm - it still feels wrong that we error out in case of failure, despite the > comment to the function saying: > * Returns NULL on error and fills the err with palloc'ed error message.
I've amended the comment so that it explains why it's done that way. > Other than this, the change looks ready to me. Well, it still needed documentation changes and pg_dump changes. I've added those in the attached version. If nobody's too unhappy with the idea, I plan to commit this soon, both because I think that the feature is useful, and also because I think it's an important security improvement. Since replication is currently run as the subscription owner, any table owner can compromise the subscription owner's account, which is really bad, but if the subscription owner can be a non-superuser, it's a little bit less bad. From a security point of view, I think the right thing to do and what would improve security a lot more is to run replication as the table owner rather than the subscription owner. I've posted a patch for that at http://postgr.es/m/CA+TgmoaSCkg9ww9oppPqqs+9RVqCexYCE6Aq=usypfnoode...@mail.gmail.com and AFAICT everyone agrees with the idea, even if the patch itself hasn't yet attracted any code reviews. But although the two patches are fairly closely related, this seems to be a good idea whether that moves forward or not, and that seems to be a good idea whether this moves forward or not. As this one has had more review and discussion, my current thought is to try to get this one committed first. -- Robert Haas EDB: http://www.enterprisedb.com
v5-0001-Add-new-predefined-role-pg_create_subscriptions.patch
Description: Binary data
