Message-ID: <0bbe14779f2c37e5a2fbd514861ebedaf8f53df6.camel@cybertec.at>
Subject: Re: BUG #19341: REPLACE() fails to match final character when using
 nondeterministic ICU collation
From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, adam.warland@infor.com,
	pgsql-bugs@lists.postgresql.org
Date: Wed, 03 Dec 2025 08:51:22 +0100
In-Reply-To: <436944.1764708826@sss.pgh.pa.us>
References: <19341-1d9a22915edfec58@postgresql.org>
	 <60b467ac47d5c6bbb9c21c3bdfecce8e5de60cda.camel@cybertec.at>
	 <6935ea1e-cfb1-400b-9057-3d43b55d621a@iki.fi>
	 <9f47c6b24d6cf59002603caf19b8b3f4854176be.camel@cybertec.at>
	 <375807.1764696352@sss.pgh.pa.us>
	 <464a14fcbee9567ace136716714b8c18f9bd87fb.camel@cybertec.at>
	 <436944.1764708826@sss.pgh.pa.us>
Content-Type: multipart/mixed; boundary="=-qqzrHYtVMr5/Xo7D1X1z"
User-Agent: Evolution 3.56.2 (3.56.2-2.fc42) 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/0bbe14779f2c37e5a2fbd514861ebedaf8f53df6.camel%40cybertec.at>
Precedence: bulk

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

On Tue, 2025-12-02 at 15:53 -0500, Tom Lane wrote:
> > The attached patch v3 turns it into a while loop to avoid
> > the problem.
>=20
> Looking at the code overall, I wonder if the outer loop doesn't have
> the same issue.=C2=A0 The comments claim that we should be able to handle
> zero-length matches, but if the overall haystack is of length zero,
> we will fail to check for such a match.

If you can find zero-length matches at all, you could find a
zero-length match in a non-empty haystack.  Perhaps the function is
never called with an empty haystack...

> Also, since we have haystack <=3D haystack_end as a starting condition,
> I think both loops could omit the initial test.=C2=A0 I'd be inclined
> to code them like
>=20
> 	test_ptr =3D start point;
> 	for (;;)
> 	{
> 		...
> 		if (test_ptr >=3D haystack_end)
> 			break;
> 		test_ptr +=3D pg_mblen(test_ptr);
> 	}

True.  The attached v4 patch does it like that.

> On the other hand ... is that comment really right about zero-length
> match being possible?=C2=A0 If it is, the API for this function is in
> need of redesign, because callers that try to find "the next match"
> would go into an infinite loop re-finding the same zero-length
> match over and over.

Right.  I'll see if I can trigger such a case.

Yours,
Laurenz Albe

--=-qqzrHYtVMr5/Xo7D1X1z
Content-Disposition: attachment;
	filename*0=v4-0001-Fix-greedy-substring-search-for-non-deterministic.pat;
	filename*1=ch
Content-Type: text/x-patch;
	name="v4-0001-Fix-greedy-substring-search-for-non-deterministic.patch";
	charset="UTF-8"
Content-Transfer-Encoding: base64

RnJvbSA4YTQzNjNhMWE2MjIyYTAzYzQwZTNkZjQxM2ZhYTg0ZjgxMDI1MmVlIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBMYXVyZW56IEFsYmUgPGxhdXJlbnouYWxiZUBjeWJlcnRlYy5h
dD4KRGF0ZTogV2VkLCAzIERlYyAyMDI1IDA4OjQxOjQ1ICswMTAwClN1YmplY3Q6IFtQQVRDSCB2
NF0gRml4IGdyZWVkeSBzdWJzdHJpbmcgc2VhcmNoIGZvciBub24tZGV0ZXJtaW5pc3RpYwogY29s
bGF0aW9ucwoKRHVlIHRvIGFuIG9mZi1ieS1vbmUgZXJyb3IsIHRoZSBjb2RlIGZhaWxlZCB0byBm
aW5kIG1hdGNoZXMgYXQgdGhlCmVuZCBvZiB0aGUgaGF5c3RhY2suCgpBdXRob3I6IExhdXJlbnog
QWxiZSA8bGF1cmVuei5hbGJlQGN5YmVydGVjLmF0PgpSZXBvcnRlZC1CeTogQWRhbSBXYXJsYW5k
IDxhZGFtLndhcmxhbmRAaW5mb3IuY29tPgpSZXZpZXdlZC1CeTogSGVpa2tpIExpbm5ha2FuZ2Fz
IDxobGlubmFrYUBpa2kuZmk+ClJldmlld2VkLUJ5OiBUb20gTGFuZSA8dGdsQHNzcy5wZ2gucGEu
dXM+CkRpc2N1c3Npb246IGh0dHBzOi8vcG9zdGdyLmVzL20vMTkzNDEtMWQ5YTIyOTE1ZWRmZWM1
OCU0MHBvc3RncmVzcWwub3JnCi0tLQogc3JjL2JhY2tlbmQvdXRpbHMvYWR0L3ZhcmxlbmEuYyAg
ICAgICAgICAgICAgICB8IDkgKysrKysrKystCiBzcmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVkL2Nv
bGxhdGUuaWN1LnV0Zjgub3V0IHwgNyArKysrKysrCiBzcmMvdGVzdC9yZWdyZXNzL3NxbC9jb2xs
YXRlLmljdS51dGY4LnNxbCAgICAgIHwgMyArKysKIDMgZmlsZXMgY2hhbmdlZCwgMTggaW5zZXJ0
aW9ucygrKSwgMSBkZWxldGlvbigtKQoKZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2Fk
dC92YXJsZW5hLmMgYi9zcmMvYmFja2VuZC91dGlscy9hZHQvdmFybGVuYS5jCmluZGV4IDM4OTQ0
NTdhYjQwLi5kNGQwYjM1MzFhZiAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvdXRpbHMvYWR0L3Zh
cmxlbmEuYworKysgYi9zcmMvYmFja2VuZC91dGlscy9hZHQvdmFybGVuYS5jCkBAIC0xMTM2LDYg
KzExMzYsOCBAQCB0ZXh0X3Bvc2l0aW9uX25leHRfaW50ZXJuYWwoY2hhciAqc3RhcnRfcHRyLCBU
ZXh0UG9zaXRpb25TdGF0ZSAqc3RhdGUpCiAJCWhwdHIgPSBzdGFydF9wdHI7CiAJCXdoaWxlICho
cHRyIDwgaGF5c3RhY2tfZW5kKQogCQl7CisJCQljb25zdCBjaGFyICp0ZXN0X2VuZDsKKwogCQkJ
LyoKIAkJCSAqIEZpcnN0IGNoZWNrIHRoZSBjb21tb24gY2FzZSB0aGF0IHRoZXJlIGlzIGEgbWF0
Y2ggaW4gdGhlCiAJCQkgKiBoYXlzdGFjayBvZiBleGFjdGx5IHRoZSBsZW5ndGggb2YgdGhlIG5l
ZWRsZS4KQEAgLTExNDksNyArMTE1MSw4IEBAIHRleHRfcG9zaXRpb25fbmV4dF9pbnRlcm5hbChj
aGFyICpzdGFydF9wdHIsIFRleHRQb3NpdGlvblN0YXRlICpzdGF0ZSkKIAkJCSAqIEVsc2UgY2hl
Y2sgaWYgYW55IG9mIHRoZSBwb3NzaWJsZSBzdWJzdHJpbmdzIHN0YXJ0aW5nIGF0IGhwdHIKIAkJ
CSAqIGFyZSBlcXVhbCB0byB0aGUgbmVlZGxlLgogCQkJICovCi0JCQlmb3IgKGNvbnN0IGNoYXIg
KnRlc3RfZW5kID0gaHB0cjsgdGVzdF9lbmQgPCBoYXlzdGFja19lbmQ7IHRlc3RfZW5kICs9IHBn
X21ibGVuKHRlc3RfZW5kKSkKKwkJCXRlc3RfZW5kID0gaHB0cjsKKwkJCWZvciAoOzspCiAJCQl7
CiAJCQkJaWYgKHBnX3N0cm5jb2xsKGhwdHIsICh0ZXN0X2VuZCAtIGhwdHIpLCBuZWVkbGUsIG5l
ZWRsZV9sZW4sIHN0YXRlLT5sb2NhbGUpID09IDApCiAJCQkJewpAQCAtMTE1OCw2ICsxMTYxLDEw
IEBAIHRleHRfcG9zaXRpb25fbmV4dF9pbnRlcm5hbChjaGFyICpzdGFydF9wdHIsIFRleHRQb3Np
dGlvblN0YXRlICpzdGF0ZSkKIAkJCQkJaWYgKCFzdGF0ZS0+Z3JlZWR5KQogCQkJCQkJYnJlYWs7
CiAJCQkJfQorCisJCQkJaWYgKHRlc3RfZW5kID49IGhheXN0YWNrX2VuZCkKKwkJCQkJYnJlYWs7
CisJCQkJdGVzdF9lbmQgKz0gcGdfbWJsZW4odGVzdF9lbmQpOwogCQkJfQogCQkJaWYgKHJlc3Vs
dF9ocHRyKQogCQkJCWJyZWFrOwpkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3Rl
ZC9jb2xsYXRlLmljdS51dGY4Lm91dCBiL3NyYy90ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQvY29sbGF0
ZS5pY3UudXRmOC5vdXQKaW5kZXggYjg1NzlhMWVmYzYuLjBhMTRjMWQ5M2ZmIDEwMDY0NAotLS0g
YS9zcmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVkL2NvbGxhdGUuaWN1LnV0Zjgub3V0CisrKyBiL3Ny
Yy90ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQvY29sbGF0ZS5pY3UudXRmOC5vdXQKQEAgLTE0ODQsNiAr
MTQ4NCwxMyBAQCBTRUxFQ1QgYXJyYXlfc29ydCgne2EsQn0nOjp0ZXh0W10gQ09MTEFURSAiQyIp
OwogIHtCLGF9CiAoMSByb3cpCiAKKy0tIHRlc3QgcmVwbGFjZSgpIGF0IHRoZSBlbmQgb2YgdGhl
IHN0cmluZyAoYnVnICMxOTM0MSkKK1NFTEVDVCByZXBsYWNlKCd0ZXN0WCcgQ09MTEFURSBjYXNl
X2luc2Vuc2l0aXZlLCAneCcgQ09MTEFURSBjYXNlX2luc2Vuc2l0aXZlLCAnZXMnKTsKKyByZXBs
YWNlIAorLS0tLS0tLS0tCisgdGVzdGVzCisoMSByb3cpCisKIC0tIHRlc3QgbGFuZ3VhZ2UgdGFn
cwogQ1JFQVRFIENPTExBVElPTiBsdF9pbnNlbnNpdGl2ZSAocHJvdmlkZXIgPSBpY3UsIGxvY2Fs
ZSA9ICdlbi11LWtzLWxldmVsMScsIGRldGVybWluaXN0aWMgPSBmYWxzZSk7CiBTRUxFQ1QgJ2FC
Y0QnIENPTExBVEUgbHRfaW5zZW5zaXRpdmUgPSAnQWJDZCcgQ09MTEFURSBsdF9pbnNlbnNpdGl2
ZTsKZGlmZiAtLWdpdCBhL3NyYy90ZXN0L3JlZ3Jlc3Mvc3FsL2NvbGxhdGUuaWN1LnV0Zjguc3Fs
IGIvc3JjL3Rlc3QvcmVncmVzcy9zcWwvY29sbGF0ZS5pY3UudXRmOC5zcWwKaW5kZXggNmY1YWJh
YzBkYzAuLjVlM2ZlZjliMzgxIDEwMDY0NAotLS0gYS9zcmMvdGVzdC9yZWdyZXNzL3NxbC9jb2xs
YXRlLmljdS51dGY4LnNxbAorKysgYi9zcmMvdGVzdC9yZWdyZXNzL3NxbC9jb2xsYXRlLmljdS51
dGY4LnNxbApAQCAtNTY4LDYgKzU2OCw5IEBAIFNFTEVDVCAnYWJjJyA8PSAnQUJDJyBDT0xMQVRF
IGNhc2VfaW5zZW5zaXRpdmUsICdhYmMnID49ICdBQkMnIENPTExBVEUgY2FzZV9pbnNlCiBTRUxF
Q1QgYXJyYXlfc29ydCgne2EsQn0nOjp0ZXh0W10gQ09MTEFURSBjYXNlX2luc2Vuc2l0aXZlKTsK
IFNFTEVDVCBhcnJheV9zb3J0KCd7YSxCfSc6OnRleHRbXSBDT0xMQVRFICJDIik7CiAKKy0tIHRl
c3QgcmVwbGFjZSgpIGF0IHRoZSBlbmQgb2YgdGhlIHN0cmluZyAoYnVnICMxOTM0MSkKK1NFTEVD
VCByZXBsYWNlKCd0ZXN0WCcgQ09MTEFURSBjYXNlX2luc2Vuc2l0aXZlLCAneCcgQ09MTEFURSBj
YXNlX2luc2Vuc2l0aXZlLCAnZXMnKTsKKwogLS0gdGVzdCBsYW5ndWFnZSB0YWdzCiBDUkVBVEUg
Q09MTEFUSU9OIGx0X2luc2Vuc2l0aXZlIChwcm92aWRlciA9IGljdSwgbG9jYWxlID0gJ2VuLXUt
a3MtbGV2ZWwxJywgZGV0ZXJtaW5pc3RpYyA9IGZhbHNlKTsKIFNFTEVDVCAnYUJjRCcgQ09MTEFU
RSBsdF9pbnNlbnNpdGl2ZSA9ICdBYkNkJyBDT0xMQVRFIGx0X2luc2Vuc2l0aXZlOwotLSAKMi41
Mi4wCgo=


--=-qqzrHYtVMr5/Xo7D1X1z--