Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: Wrong comment for ReplicationSlotCreate
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAJDiXggZXQZ7bD0QcTizDt6us9aX6ZKK4dWxzgb5x3+TsVHjqQ@mail.gmail.com>
Date: Tue, 30 Dec 2025 17:18:10 +0800
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <00DB978B-828D-44FE-8A0B-1643CE03878E@gmail.com>
References: 
 <CAJDiXggZXQZ7bD0QcTizDt6us9aX6ZKK4dWxzgb5x3+TsVHjqQ@mail.gmail.com>
To: Daniil Davydov <3danissimo@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/00DB978B-828D-44FE-8A0B-1643CE03878E%40gmail.com>
Precedence: bulk


> On Dec 29, 2025, at 21:39, Daniil Davydov <3danissimo@gmail.com> =
wrote:
>=20
> Hi,
>=20
> I noticed that the comment for ReplicationSlotCreate function contains =
this
> description for the "two_phase" option :
>=20
> * two_phase: Allows decoding of prepared transactions. We allow this =
option
> *     to be enabled only at the slot creation time. If we allow this =
option
> *     to be changed during decoding then it is quite possible that we =
skip
> *     prepare first time because this option was not enabled. Now next =
time
> *     during getting changes, if the two_phase option is enabled it =
can skip
> *     prepare because by that time start decoding point has been =
moved. So the
> *     user will only get commit prepared.
>=20
> But commit [1] introduced the ability to alter the "two_phase" option =
for the
> replication slot. Thus, I guess that the comment mentioned above is
> outdated and we should change it.
>=20
> [1] 1462aad2e4474ab61174f8ab00992cd3d6d57c7b
>=20
> --
> Best regards,
> Daniil Davydov
> <0001-Fix-comment-for-ReplicationSlotCreate.patch>

The old comment claimed =E2=80=9CWe allow this option to be enabled only =
at the slot creation time=E2=80=9D that is no longer true after commit =
1462aad2e4474ab61174f8ab00992cd3d6d57c7b, so yes, the old comment need =
updating.

But I think the updated version is too simple. It loses the information =
that enabling two_phase later can result in missing PREPARE.

So, I would suggest something like:
```
* two_phase: If enabled, allows decoding of prepared transactions.
*     Note that enabling this option after decoding has already advanced
*     may result in missing PREPARE records for transactions that were
*     prepared before the option was enabled.

```

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/