On 14.03.23 19:30, Gregory Stark (as CFM) wrote:
> FYI this looks like it needs a rebase due to a conflict in copy.c and
> an offset in pgoutput.c.
>
> Is there anything specific that still needs review or do you think
> you've handled all Peter's concerns? In particular, is there "a
> comprehensive description of what it is trying to do"? :)
The latest versions of the patch have pretty much addressed my initial
comments. The patch is structured and explained better now. Most
extraneous or incomplete changes have been addressed.
The problem now is that it's still a quite complicated patch that
introduces a security feature. It still touches a number of subsystems
on different levels of abstraction. This functionality is not of the
kind, "if you don't use it it won't affect you", since it effectively
pokes holes into the existing privileges checking in order to allow
publication privileges checking to override it in some cases. It will
take significant effort to do a complete analysis and testing on whether
it is secure and robust. I don't think I will have time for that, and I
don't think anyone will want to commit something like this at the last
moment.
We have already taken a number of things from earlier patches and
committed them separately as refactorings. I don't see anything in the
current patch anymore that we might want to take independently like that.
So in summary I think it would be best to keep this patch around for PG17.
