On Tue, Nov 30, 2021 at 12:56 AM Jeff Davis <pg...@j-davis.com> wrote: > > On Mon, 2021-11-29 at 09:43 +0530, Amit Kapila wrote: > > The first reason is that way it would be consistent with what we can > > see while doing the operations from the backend. > > Logical replication is not interactive, so it doesn't seem quite the > same. > > If you have a long running INSERT INTO SELECT or COPY FROM, the > permission checks just happen at the beginning. As a user, it wouldn't > surprise me if logical replication was similar. > > > operation. Another reason is to make behavior predictable as users > > can > > always expect when exactly the privilege change will be reflected and > > it won't depend on the number of changes in the transaction. > > This patch does detect ownership changes more quickly (at the > transaction boundary) than the current code (only when it reloads for > some other reason). Transaction boundary seems like a reasonable time > to detect the change to me. > > Detecting faster might be nice, but I don't have a strong opinion about > it and I don't see why it necessarily needs to happen before this patch > goes in. >
I think it would be better to do it before we allow subscription owners to be non-superusers. > Also, do you think the cost of doing maybe_reread_subscription() per- > tuple instead of per-transaction would be detectable? > Yeah, it is possible that is why I suggested in one of the emails above to allow changing the owners only for disabled subscriptions. -- With Regards, Amit Kapila.