From: | Japin Li <japinli(at)hotmail(dot)com> |
---|---|
To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax |
Date: | 2021-03-23 15:08:43 |
Message-ID: | MEYP282MB16692912422D491945C4A3CCB6649@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, 22 Mar 2021 at 11:14, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> On Sun, Mar 7, 2021 at 7:21 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>> Thank you point out this. Fixed it in v7 patch set.
>>
>> Please consider the v7 patch for futher review.
>
> Thanks for the patches. I just found the following behaviour with the
> new ADD/DROP syntax: when the specified publication list has
> duplicates, the patch is throwing "publication is already present"
> error. It's adding the first instance of the duplicate into the list
> and the second instance is being checked in the added list and
> throwing the "already present error". The error message means that the
> publication is already present in the subscription but it's not true.
> See my testing at [1].
>
> I think we have two cases:
> case 1: the publication/s specified in the new ADD/DROP syntax may/may
> not have already been associated with the subscription, so the error
> "publication is already present"/"publication doesn't exist" error
> makes sense.
> case 2: there can be duplicate publications specified in the new
> ADD/DROP syntax, in this case the error "publication name "mypub2"
> used more than once" makes more sense much like [2].
>
> [1]
> postgres=# select subpublications from pg_subscription;
> subpublications
> -----------------
> {mypub,mypub1}
>
> postgres=# alter subscription mysub add publication mypub2, mypub2;
> ERROR: publication "mypub2" is already present in the subscription
>
> postgres=# select subpublications from pg_subscription;
> subpublications
> -----------------------
> {mypub,mypub1,mypub2}
>
> postgres=# alter subscription mysub drop publication mypub2, mypub2;
> ERROR: publication "mypub2" doesn't exist in the subscription
>
> [2]
> postgres=# alter subscription mysub set publication mypub2, mypub2;
> ERROR: publication name "mypub2" used more than once
>
Thanks for your review.
I check the duplicates for newpublist in merge_publications(). The code is
copied from publicationListToArray().
I do not check for all duplicates because it will make the code more complex.
For example:
ALTER SUBSCRIPTION mysub ADD PUBLICATION mypub2, mypub2, mypub2;
If we record the duplicate publication names in list A, when we find a
duplication in newpublist, we should check whether the publication is
in list A or not, to make the error message make sense (do not have
duplicate publication names in error message).
Thought?
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
Attachment | Content-Type | Size |
---|---|---|
v8-0001-Introduce-a-new-syntax-to-add-drop-publications.patch | text/x-patch | 7.0 KB |
v8-0002-Test-ALTER-SUBSCRIPTION-.-ADD-DROP-PUBLICATION.patch | text/x-patch | 5.9 KB |
v8-0003-Add-documentation-for-ALTER-SUBSCRIPTION.ADD-DROP.patch | text/x-patch | 4.5 KB |
v8-0004-Add-tab-complete-for-ALTER-SUBSCRIPTION.ADD-DROP.patch | text/x-patch | 1.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2021-03-23 15:12:03 | Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view? |
Previous Message | Bruce Momjian | 2021-03-23 14:56:28 | Re: pg_upgrade failing for 200+ million Large Objects |