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.



Reply via email to