Re: Why ALTER SUBSCRIPTION ... SET (slot_name='none') requires subscription disabled?

From: Japin Li <japinli(at)hotmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
Subject: Re: Why ALTER SUBSCRIPTION ... SET (slot_name='none') requires subscription disabled?
Date: 2021-07-16 08:42:01
Message-ID: MEYP282MB16695EFE450605A578C8F609B6119@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Fri, 16 Jul 2021 at 14:06, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Fri, Jul 9, 2021 at 8:20 AM Japin Li <japinli(at)hotmail(dot)com> wrote:
>>
>> On Thu, 08 Jul 2021 at 18:17, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> > On Thu, Jul 8, 2021 at 3:43 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>>
>> Please consider review v3 patch. v3-0001 adds slot_name verification in
>> parse_subscription_options() and comments for why we need disable subscription
>> where set slot_name to NONE.
>>
>
> I think we back-patch this bug-fix till v10 where it was introduced
> and update the comments only in HEAD. So, accordingly, I moved the
> changes into two patches and changed the comments a bit. Can you
> please test the first patch in back-branches? I'll also do it
> separately.
>

I try to back-patch to v10 stable to v14 stable, and attach two new patches:
one for PG10 & PG11 stable, and the other is for PG12 to PG14 stable.
v4 patch can be applied on HEAD. This modify looks good to me.

How do we back-patch to back-branches? I try to use cherry-pick, but it doesn't
always work (without a doubt, it might be some difference between branches).

>> v3-0002 comes from Ranier Vilela, it reduce the
>> overhead strlen in ReplicationSlotValidateName().
>>
>
> I think this patch has nothing to do with this bug-fix, so I suggest
> you discuss this in a separate patch. Personally, I don't think it
> will help in reducing any overhead but there doesn't appear to be any
> harm in changing it as proposed.

I start a new thread to discuss this [1].

[1] - https://www.postgresql.org/message-id/MEYP282MB16696F6DBA8AE36A648817B2B6119@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

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

Attachment Content-Type Size
v5-0001-Don-t-allow-to-set-replication-slot_name-as-v10-v11.patch text/x-patch 3.1 KB
v5-0001-Don-t-allow-to-set-replication-slot_name-as-v12-v13-v14.patch text/x-patch 3.2 KB
v4-0001-Don-t-allow-to-set-replication-slot_name-as.patch text/x-patch 3.0 KB
v4-0002-Update-comments-for-AlterSubscription.patch text/x-patch 1.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2021-07-16 08:45:59 Re: row filtering for logical replication
Previous Message Japin Li 2021-07-16 08:35:25 Re: Remove redundant strlen call in ReplicationSlotValidateName