Re: Alter all tables in schema owner fix

2021-12-08 Thread Amit Kapila
On Tue, Dec 7, 2021 at 11:20 PM Bossart, Nathan wrote: > > On 12/7/21, 2:47 AM, "Greg Nancarrow" wrote: > > On Tue, Dec 7, 2021 at 9:01 PM Amit Kapila wrote: > >> > >> Thanks, the patch looks mostly good to me. I have slightly modified it > >> to incorporate one of Michael's suggestions, ran

Re: Alter all tables in schema owner fix

2021-12-07 Thread Bossart, Nathan
On 12/7/21, 2:47 AM, "Greg Nancarrow" wrote: > On Tue, Dec 7, 2021 at 9:01 PM Amit Kapila wrote: >> >> Thanks, the patch looks mostly good to me. I have slightly modified it >> to incorporate one of Michael's suggestions, ran pgindent, and >> modified the commit message. >> > > LGTM, except in

Re: Alter all tables in schema owner fix

2021-12-07 Thread Greg Nancarrow
On Tue, Dec 7, 2021 at 9:01 PM Amit Kapila wrote: > > Thanks, the patch looks mostly good to me. I have slightly modified it > to incorporate one of Michael's suggestions, ran pgindent, and > modified the commit message. > LGTM, except in the patch commit message I'd change "Create Publication"

Re: Alter all tables in schema owner fix

2021-12-07 Thread Amit Kapila
On Tue, Dec 7, 2021 at 8:21 AM vignesh C wrote: > > Thanks for your comments, I have made the changes. Additionally I have > renamed IsSchemaPublication to is_schema_publication for keeping the > naming similar around the code. The attached v3 patch has the changes > for the same. > Thanks, the

Re: Alter all tables in schema owner fix

2021-12-06 Thread vignesh C
On Fri, Dec 3, 2021 at 10:50 PM Bossart, Nathan wrote: > > On 12/2/21, 11:57 PM, "tanghy.f...@fujitsu.com" > wrote: > > Thanks for your patch. > > I tested it and it fixed this problem as expected. It also passed "make > > check-world". > > +1, the patch looks good to me, too. My only other

Re: Alter all tables in schema owner fix

2021-12-06 Thread Amit Kapila
On Mon, Dec 6, 2021 at 11:46 AM Michael Paquier wrote: > > On Fri, Dec 03, 2021 at 05:20:35PM +, Bossart, Nathan wrote: > > On 12/2/21, 11:57 PM, "tanghy.f...@fujitsu.com" > > wrote: > > > Thanks for your patch. > > > I tested it and it fixed this problem as expected. It also passed "make

Re: Alter all tables in schema owner fix

2021-12-05 Thread Michael Paquier
On Fri, Dec 03, 2021 at 05:20:35PM +, Bossart, Nathan wrote: > On 12/2/21, 11:57 PM, "tanghy.f...@fujitsu.com" > wrote: > > Thanks for your patch. > > I tested it and it fixed this problem as expected. It also passed "make > > check-world". > > +1, the patch looks good to me, too. My only

Re: Alter all tables in schema owner fix

2021-12-03 Thread Bossart, Nathan
On 12/2/21, 11:57 PM, "tanghy.f...@fujitsu.com" wrote: > Thanks for your patch. > I tested it and it fixed this problem as expected. It also passed "make > check-world". +1, the patch looks good to me, too. My only other suggestion would be to move IsSchemaPublication() to pg_publication.c

RE: Alter all tables in schema owner fix

2021-12-02 Thread tanghy.f...@fujitsu.com
On Friday, December 3, 2021 1:31 PM vignesh C wrote: > > On Fri, Dec 3, 2021 at 9:53 AM Greg Nancarrow wrote: > > > > On Fri, Dec 3, 2021 at 2:06 PM vignesh C wrote: > > > > > > Currently while changing the owner of ALL TABLES IN SCHEMA > > > publication, it is not checked if the new owner has

Re: Alter all tables in schema owner fix

2021-12-02 Thread vignesh C
On Fri, Dec 3, 2021 at 9:53 AM Greg Nancarrow wrote: > > On Fri, Dec 3, 2021 at 2:06 PM vignesh C wrote: > > > > Currently while changing the owner of ALL TABLES IN SCHEMA > > publication, it is not checked if the new owner has superuser > > permission or not. Added a check to throw an error if

Re: Alter all tables in schema owner fix

2021-12-02 Thread vignesh C
On Fri, Dec 3, 2021 at 9:58 AM Bossart, Nathan wrote: > > On 12/2/21, 7:07 PM, "vignesh C" wrote: > > Currently while changing the owner of ALL TABLES IN SCHEMA > > publication, it is not checked if the new owner has superuser > > permission or not. Added a check to throw an error if the new

Re: Alter all tables in schema owner fix

2021-12-02 Thread Bossart, Nathan
On 12/2/21, 7:07 PM, "vignesh C" wrote: > Currently while changing the owner of ALL TABLES IN SCHEMA > publication, it is not checked if the new owner has superuser > permission or not. Added a check to throw an error if the new owner > does not have superuser permission. > Attached patch has the

Re: Alter all tables in schema owner fix

2021-12-02 Thread Greg Nancarrow
On Fri, Dec 3, 2021 at 2:06 PM vignesh C wrote: > > Currently while changing the owner of ALL TABLES IN SCHEMA > publication, it is not checked if the new owner has superuser > permission or not. Added a check to throw an error if the new owner > does not have superuser permission. > Attached

Alter all tables in schema owner fix

2021-12-02 Thread vignesh C
Hi, Currently while changing the owner of ALL TABLES IN SCHEMA publication, it is not checked if the new owner has superuser permission or not. Added a check to throw an error if the new owner does not have superuser permission. Attached patch has the changes for the same. Thoughts? Regards,