MIME-Version: 1.0
References: 
 <CAMPB6wfe4zLjJL8jiZV5kjjpwBM2=rTRme0UCL7Ra4L8MTVdOg@mail.gmail.com>
 <b9666b93-093b-4c9f-a385-772d6ab5688c@app.fastmail.com>
 <CAMPB6we7+97L72Ru0=WxMDi24xMbZgr2B8Nwoo5i=r=UNuG_gQ@mail.gmail.com>
 <a4ee2198-5309-4b6f-90ef-c08b45a2f572@app.fastmail.com>
 <CAA4eK1JfPPFTmz7mUk26zPH8+qH9UBpkquxw75x7Ngx_D_6XXQ@mail.gmail.com>
 <CAMPB6wfgvWjSvKNPoJkRqaL46geRDoL++Pt_3Czc2QNAdpVQHw@mail.gmail.com>
 <CAA4eK1JC6yB6q52qEZ5dLNWRUEZoO-aa_XKBZ3_mcb=V2z7zug@mail.gmail.com>
 <CAMPB6weUqU6P2w5VUGVSLKWcvU1AQHmW+7O9qc9yD4CB5kEYVA@mail.gmail.com>
 <CAA4eK1Lm_W5j3DPj6PDSTyodGu87QgxpNwwsi-wVR0+B1FSOoA@mail.gmail.com>
 <CAMPB6wckvkKrXVPH5j8Ske2cVedkb-TRLdnOb5e74zYM1CynGw@mail.gmail.com>
 <CAA4eK1+NDjprcKvr0p2GDMTCs9yxFCY41bOd+6avqAm2n+TXdQ@mail.gmail.com>
 <CAMPB6wdc10tc7gpVXG75r51M41zVSabip9Lz7hssWEtyhecWww@mail.gmail.com>
 <OSCPR01MB14966201F1DCB853145912FF1F53DA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <CAMPB6wcOWBURHB1igRgCjD3geAemdoATfkKByMwrMM1TgMN64w@mail.gmail.com>
 <OSCPR01MB14966BF4CA9B767C259DBD9CDF53EA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <CAMPB6wdtKZLEeZ7UW=DYmXWv8Y=uVGrDqXTMhT19Z4VTzo3cfg@mail.gmail.com>
 <CAA4eK1LHVd8wQzauWgeEV436L7btrCfujPH1sR196sY_Mp8zYA@mail.gmail.com>
 <CAMPB6wdPtjbR93oB1XJtYkRtTR64BJG4o5a+0DSSez=puuyuGA@mail.gmail.com>
 <OSCPR01MB14966FC456D053AB00A2EB278F514A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <CAA4eK1++mHd-SsHyJd2ZB26F7kCz--LbtjQLuQ0h3z9mcYK-AQ@mail.gmail.com>
 <OS7PR01MB149681B14B2432A9CEA7A3586F517A@OS7PR01MB14968.jpnprd01.prod.outlook.com>
 <OSCPR01MB14966F65DE462D0A479B8ADD6F516A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
In-Reply-To: 
 <OSCPR01MB14966F65DE462D0A479B8ADD6F516A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Thu, 18 Sep 2025 16:32:56 +0530
Message-ID: 
 <CAA4eK1LeyzuiRPZB+o7mO0pB6_=tpkjoum5Hj+t1SYydS4K2kQ@mail.gmail.com>
Subject: Re: [Patch] add new parameter to pg_replication_origin_session_setup
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Doruk Yilmaz <doruk@mixrank.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="0000000000006de9d7063f114cdd"
Archived-At: 
 <https://www.postgresql.org/message-id/CAA4eK1LeyzuiRPZB%2Bo7mO0pB6_%3Dtpkjoum5Hj%2Bt1SYydS4K2kQ%40mail.gmail.com>
Precedence: bulk

--0000000000006de9d7063f114cdd
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Sep 18, 2025 at 1:07=E2=80=AFPM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear hackers,
>
> > I considered a test, please see attached files.
>

Few comments:
1. +step "s0_compare" {
+    SELECT s0.lsn < s1.lsn
+    FROM local_lsn_store as s0, local_lsn_store as s1
+    WHERE s0.session =3D 0 AND s1.session =3D 1;
+}

This appears to be a bit tricky to compare the values. Doing a
sequential scan won't guarantee the order of rows' appearance. Can't
we somehow get the two rows ordered by session_id and compare their
values?

2.
+ else if (candidate_state->acquired_by !=3D acquired_by)
+ {
+ if (initialized)
+ candidate_state->roident =3D InvalidRepOriginId;
+
  elog(ERROR, "could not find replication state slot for replication
origin with OID %u which was acquired by %d",
  node, acquired_by);
+ }

This doesn't appear neat. Instead, how about checking this case before
setting current_state as shown in attached. If we do that, we
shouldn't even need new variables like current_state and initialized.
Additionally, as shown in attached, it is better to make this a
user-facing error by using ereport.

3. Merge all patches as I don't see the need to do any backpatch here.

--=20
With Regards,
Amit Kapila.

--0000000000006de9d7063f114cdd
Content-Type: text/plain; charset="US-ASCII"; name="v8_amit_1.txt"
Content-Disposition: attachment; filename="v8_amit_1.txt"
Content-Transfer-Encoding: base64
Content-ID: <f_mfpax4hj0>
X-Attachment-Id: f_mfpax4hj0

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL2xvZ2ljYWwvb3JpZ2luLmMgYi9z
cmMvYmFja2VuZC9yZXBsaWNhdGlvbi9sb2dpY2FsL29yaWdpbi5jCmluZGV4IDBiYmM5NmJjZWU1
Li5jOTNiNmViMTc5OCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNh
bC9vcmlnaW4uYworKysgYi9zcmMvYmFja2VuZC9yZXBsaWNhdGlvbi9sb2dpY2FsL29yaWdpbi5j
CkBAIC0xMTY5LDYgKzExNjksMTUgQEAgcmVwbG9yaWdpbl9zZXNzaW9uX3NldHVwKFJlcE9yaWdp
bklkIG5vZGUsIGludCBhY3F1aXJlZF9ieSkKIAkJCQkJCQljdXJzdGF0ZS0+cm9pZGVudCwgY3Vy
c3RhdGUtPmFjcXVpcmVkX2J5KSkpOwogCQl9CiAKKwkJZWxzZSBpZiAoY3Vyc3RhdGUtPmFjcXVp
cmVkX2J5ICE9IDAgJiYKKwkJCQkgY3Vyc3RhdGUtPmFjcXVpcmVkX2J5ICE9IGFjcXVpcmVkX2J5
KQorCQl7CisJCQllcmVwb3J0KEVSUk9SLAorCQkJCQkoZXJyY29kZShFUlJDT0RFX09CSkVDVF9O
T1RfSU5fUFJFUkVRVUlTSVRFX1NUQVRFKSwKKwkJCQkJIGVycm1zZygiY291bGQgbm90IGZpbmQg
cmVwbGljYXRpb24gc3RhdGUgc2xvdCBmb3IgcmVwbGljYXRpb24gb3JpZ2luIHdpdGggT0lEICV1
IHdoaWNoIHdhcyBhY3F1aXJlZCBieSAlZCIsCisJCQkJCQkJbm9kZSwgYWNxdWlyZWRfYnkpKSk7
CisJCX0KKwogCQkvKiBvaywgZm91bmQgc2xvdCAqLwogCQljYW5kaWRhdGVfc3RhdGUgPSBjdXJz
dGF0ZTsKIAkJYnJlYWs7CkBAIC0xMTk2LDE0ICsxMjA1LDggQEAgcmVwbG9yaWdpbl9zZXNzaW9u
X3NldHVwKFJlcE9yaWdpbklkIG5vZGUsIGludCBhY3F1aXJlZF9ieSkKIAogCWlmIChhY3F1aXJl
ZF9ieSA9PSAwKQogCQljYW5kaWRhdGVfc3RhdGUtPmFjcXVpcmVkX2J5ID0gTXlQcm9jUGlkOwot
CWVsc2UgaWYgKGNhbmRpZGF0ZV9zdGF0ZS0+YWNxdWlyZWRfYnkgIT0gYWNxdWlyZWRfYnkpCi0J
ewotCQlpZiAoaW5pdGlhbGl6ZWQpCi0JCQljYW5kaWRhdGVfc3RhdGUtPnJvaWRlbnQgPSBJbnZh
bGlkUmVwT3JpZ2luSWQ7Ci0KLQkJZWxvZyhFUlJPUiwgImNvdWxkIG5vdCBmaW5kIHJlcGxpY2F0
aW9uIHN0YXRlIHNsb3QgZm9yIHJlcGxpY2F0aW9uIG9yaWdpbiB3aXRoIE9JRCAldSB3aGljaCB3
YXMgYWNxdWlyZWQgYnkgJWQiLAotCQkJIG5vZGUsIGFjcXVpcmVkX2J5KTsKLQl9CisJZWxzZQor
CQlBc3NlcnQoY2FuZGlkYXRlX3N0YXRlLT5hY3F1aXJlZF9ieSA9PSBhY3F1aXJlZF9ieSk7CiAK
IAkvKiBDYW5kaWRhdGUgc2xvdCBsb29rcyBvaywgdXNlIGl0ICovCiAJc2Vzc2lvbl9yZXBsaWNh
dGlvbl9zdGF0ZSA9IGNhbmRpZGF0ZV9zdGF0ZTsK
--0000000000006de9d7063f114cdd--