Feedback-ID: i131946ab:Fastmail
Message-ID: <a7b95f74-e3e3-adae-b472-2e8d24a4e4b8@enterprisedb.com>
Date: Thu, 3 Nov 2022 11:49:51 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.4.0
Subject: Re: Privileges on PUBLICATION
Content-Language: en-US
To: Antonin Houska <ah@cybertec.at>
Cc: Euler Taveira <euler@eulerto.com>, Amit Kapila <amit.kapila16@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <20330.1652105397@antos>
 <8a00e0e1-dc4e-422c-9dcf-664b16b18625@www.fastmail.com>
 <CAA4eK1Lya6BoMa1m8cTZrQ8KvABQpn3EsHDKRurz8UJeHsGnZw@mail.gmail.com>
 <2810.1652171844@antos>
 <d96103fe-99e2-4119-bd76-952d326b7539@www.fastmail.com>
 <5859.1652423797@antos>
 <e6025f13-fefa-4ff2-9364-24f4c77fcf7a@www.fastmail.com>
 <57022.1652867097@antos>
 <4c420c17-8570-4b52-b7f6-9ce3f53d7ef9@www.fastmail.com>
 <14292.1655733695@antos.home>
 <746c9a65-e2cc-2573-a50e-ab5ea884f680@enterprisedb.com>
 <1537.1667454192@antos>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <1537.1667454192@antos>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/a7b95f74-e3e3-adae-b472-2e8d24a4e4b8%40enterprisedb.com>
Precedence: bulk

On 03.11.22 01:43, Antonin Houska wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
> 
>> The CF entry is about privileges on publications.  Please rebase that patch
>> and repost it so that the CF app and the CF bot are up to date.
> 
> The rebased patch (with regression tests added) is attached here.

Some preliminary discussion:

What is the upgrade strategy?  I suppose the options are either that 
publications have a default acl that makes them publicly accessible, 
thus preserving the existing behavior by default, or pg_dump would need 
to create additional GRANT statements when upgrading from pre-PG16.  I 
don't see anything like either of these mentioned in the patch.  What is 
your plan?

You might be interested in this patch, which relates to yours: 
https://commitfest.postgresql.org/40/3955/

Looking at your patch, I would also like to find a way to refactor away 
the ExecGrant_Publication() function.  I'll think about that.

I think you should add some tests under src/test/regress/ for the new 
GRANT and REVOKE statements, just to have some basic coverage that it 
works.  sql/publication.sql would be appropriate, I think.