| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Daniil Davydov <3danissimo(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Wrong comment for ReplicationSlotCreate |
| Date: | 2026-01-05 07:55:55 |
| Message-ID: | AC9B87F1-ED04-4547-B85C-9443B4253A08@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Jan 5, 2026, at 13:51, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Jan 5, 2026 at 9:46 AM Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>>
>> Dear Daniil, Chao,
>>
>> I was the main author of 1462aad2. It is enough to remove outdated comments atop
>> the definition. In other words, your patch looks good to me.
>>
>> If needed, we can also notify developers that the two-phase option should not be
>> altered while decoding WAL records. In logical replication, we ensure that the
>> subscription is disabled and there are no apply workers. However, I don't think
>> such comments can be atop the ReplicationSlotCreate(). Maybe around
>> ReplicationSlotAlter(), but it may be out of scope of the initial motivation.
>>
>
> I think it is better if we add some comments atop
> ReplicationSlotAlter() as you are suggesting. What do you think of the
> attached?
>
> --
> With Regards,
> Amit Kapila.
> <v1_improve_alter_slot_comments.patch>
Hi Amit,
While reviewing your change, I find the other typo in slot.c:
```
- /* Check if the slot exits with the given name. */
+ /* Check if the slot exists with the given name. */
s = SearchNamedReplicationSlot(name, false);
if (s == NULL || !s->in_use)
```
“Exits” and “exists” have totally different meanings, thus might lead to misunderstanding. Attached is a trivial diff to fix that.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| slot_typo.diff | application/octet-stream | 465 bytes |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Eisentraut | 2026-01-05 08:16:50 | Re: COPY WHERE clause generated/system column reference |
| Previous Message | Peter Eisentraut | 2026-01-05 07:55:08 | Re: Use Python "Limited API" in PL/Python |