Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax - Mailing list pgsql-hackers

From Japin Li
Subject Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax
Date
Msg-id MEYP282MB166910079F45B2CE3E2513F5B6769@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Whole thread Raw
In response to Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax
List pgsql-hackers
On Sat, 03 Apr 2021 at 13:20, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
> On Sat, Apr 3, 2021 at 1:29 AM Peter Eisentraut
> <peter.eisentraut@enterprisedb.com> wrote:
>> The code you have in merge_publications() to report all existing
>> publications is pretty messy and is not properly internationalized.  I
>> think what you are trying to do there is excessive.  Compare this
>> similar case:
>>
>> create table t1 (a int, b int);
>> alter table t1 add column a int, add column b int;
>> ERROR:  42701: column "a" of relation "t1" already exists
>>
>> I think you can make both this and the duplicate checking much simpler
>> if you just report the first conflict.
>
> Yes, we are erroring out on the first conflict for both duplicates and
> in merge_publications.
>
>> I think this patch is about ready to commit, but please provide a final
>> version in good time.
>
> I took the liberty to address all the review comments and provide a v9
> patch on top of Japin's v8 patch-set.
>
>> (Also, please combine your patches into a single patch.)
>
> Done.
>
> Attaching v9 patch, please review it.
>

Sorry for the late reply! Thanks for your updating the new patch, and it looks
good to me.

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: subtransaction performance regression [kind of] due to snapshot caching
Next
From: vignesh C
Date:
Subject: Re: typo fix in pgstat.c: "exits should be exists"