Message-ID: <792a0aeb486e240ea34f10f895e0368fc434b8b7.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: Thu, 04 Dec 2025 18:14:51 +0100
In-Reply-To: <640025.1764774727@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>
	 <0bbe14779f2c37e5a2fbd514861ebedaf8f53df6.camel@cybertec.at>
	 <640025.1764774727@sss.pgh.pa.us>
Content-Type: multipart/mixed; boundary="=-LnyTNpmOxSz6OpUllAmU"
User-Agent: Evolution 3.56.2 (3.56.2-2.fc42) 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/792a0aeb486e240ea34f10f895e0368fc434b8b7.camel%40cybertec.at>
Precedence: bulk

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

On Wed, 2025-12-03 at 10:12 -0500, Tom Lane wrote:
> Laurenz Albe <laurenz.albe@cybertec.at> writes:
> > On Tue, 2025-12-02 at 15:53 -0500, Tom Lane wrote:
> > > 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 ha=
ndle
> > > zero-length matches, but if the overall haystack is of length zero,
> > > we will fail to check for such a match.
>=20
>=20
> After further thought, it seems to me that this comment is an
> unjustified extrapolation from what Peter actually said, which was
> that the match substring could be physically shorter than the needle.
> Which is certainly true, for instance case-folding or accent-stripping
> might shorten the string.  But it doesn't follow that a nonempty
> needle could ever match an empty substring; and that does not seem
> like it could be sane behavior to me.  We're considering string
> comparison here, not regexes.
>=20
> We do require callers to eliminate the empty-needle case, and given
> that I think we could assume that match substrings must be at least
> 1 byte long.  That assumption is what justifies the current API for
> these functions, and perhaps we can also simplify this loop by
> using it.

I think I get it.  I don't see an explicit requirement for a non-empty
needle, but all callers of text_position_next_internal() handle that
case separately.

The attached v5 patch simplifies the loop to a do-while loop, assuming
that we cannot find a zero-length match.

I have also updated the comments to no longer mention the possibility
of an empty match, and for good measure I have added an Assert() that
the needle cannot be empty.

Yours,
Laurenz Albe

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

RnJvbSBlMjY1ZmM1NzkwZDRjODIyNGJiNzU4NTE4NDcwYjI0MjQ0MThkOTEzIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBMYXVyZW56IEFsYmUgPGxhdXJlbnouYWxiZUBjeWJlcnRlYy5h
dD4KRGF0ZTogVGh1LCA0IERlYyAyMDI1IDE4OjA3OjA2ICswMTAwClN1YmplY3Q6IFtQQVRDSCB2
NV0gRml4IGdyZWVkeSBzdWJzdHJpbmcgc2VhcmNoIGZvciBub24tZGV0ZXJtaW5pc3RpYwogY29s
bGF0aW9ucwoKRHVlIHRvIGFuIG9mZi1ieS1vbmUgZXJyb3IsIHRoZSBjb2RlIGZhaWxlZCB0byBm
aW5kIG1hdGNoZXMgYXQgdGhlCmVuZCBvZiB0aGUgaGF5c3RhY2suICBGaXggYnkgcmV3cml0aW5n
IHRoZSBsb29wLgoKV2hpbGUgYXQgaXQsIGZpeCBhIGNvbW1lbnQgdGhhdCBjbGFpbWVkIHRoYXQg
dGhlIGZ1bmN0aW9uIGNvdWxkIGZpbmQKYSB6ZXJvLWxlbmd0aCBtYXRjaC4gIFN1Y2ggYSBtYXRj
aCBjb3VsZCBzZW5kIGEgY2FsbGVyIGludG8gYW4KZW5kbGVzcyBsb29wLiAgSG93ZXZlciwgemVy
by1sZW5ndGggbWF0Y2hlcyBjYW4gb25seSBvY2N1ciB3aXRoIGFuCmVtcHR5IHNlYXJjaCBzdHJp
bmcsIGFuZCB0aGF0IGNhc2UgaXMgZXhwbGljaXRseSBleGNsdWRlZCBieSBhbGwKY2FsbGVycy4g
IFRvIG1ha2Ugc3VyZSBpdCBzdGF5cyB0aGF0IHdheSwgYWRkIGFuIEFzc2VydC4KCkF1dGhvcjog
TGF1cmVueiBBbGJlIDxsYXVyZW56LmFsYmVAY3liZXJ0ZWMuYXQ+ClJlcG9ydGVkLUJ5OiBBZGFt
IFdhcmxhbmQgPGFkYW0ud2FybGFuZEBpbmZvci5jb20+ClJldmlld2VkLUJ5OiBIZWlra2kgTGlu
bmFrYW5nYXMgPGhsaW5uYWthQGlraS5maT4KUmV2aWV3ZWQtQnk6IFRvbSBMYW5lIDx0Z2xAc3Nz
LnBnaC5wYS51cz4KRGlzY3Vzc2lvbjogaHR0cHM6Ly9wb3N0Z3IuZXMvbS8xOTM0MS0xZDlhMjI5
MTVlZGZlYzU4JTQwcG9zdGdyZXNxbC5vcmcKLS0tCiBzcmMvYmFja2VuZC91dGlscy9hZHQvdmFy
bGVuYS5jICAgICAgICAgICAgICAgfCAyNCArKysrKysrKysrKystLS0tLS0tCiAuLi4vcmVncmVz
cy9leHBlY3RlZC9jb2xsYXRlLmljdS51dGY4Lm91dCAgICAgfCAgNyArKysrKysKIHNyYy90ZXN0
L3JlZ3Jlc3Mvc3FsL2NvbGxhdGUuaWN1LnV0Zjguc3FsICAgICB8ICAzICsrKwogMyBmaWxlcyBj
aGFuZ2VkLCAyNSBpbnNlcnRpb25zKCspLCA5IGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL3Ny
Yy9iYWNrZW5kL3V0aWxzL2FkdC92YXJsZW5hLmMgYi9zcmMvYmFja2VuZC91dGlscy9hZHQvdmFy
bGVuYS5jCmluZGV4IDM4OTQ0NTdhYjQwLi4yNmZiYjllOTY5YiAxMDA2NDQKLS0tIGEvc3JjL2Jh
Y2tlbmQvdXRpbHMvYWR0L3ZhcmxlbmEuYworKysgYi9zcmMvYmFja2VuZC91dGlscy9hZHQvdmFy
bGVuYS5jCkBAIC0xMTEwLDEwICsxMTEwLDEwIEBAIHRleHRfcG9zaXRpb25fbmV4dF9pbnRlcm5h
bChjaGFyICpzdGFydF9wdHIsIFRleHRQb3NpdGlvblN0YXRlICpzdGF0ZSkKIAljb25zdCBjaGFy
ICpoYXlzdGFja19lbmQgPSAmaGF5c3RhY2tbaGF5c3RhY2tfbGVuXTsKIAljb25zdCBjaGFyICpo
cHRyOwogCi0JQXNzZXJ0KHN0YXJ0X3B0ciA+PSBoYXlzdGFjayAmJiBzdGFydF9wdHIgPD0gaGF5
c3RhY2tfZW5kKTsKLQogCXN0YXRlLT5sYXN0X21hdGNoX2xlbl90bXAgPSBuZWVkbGVfbGVuOwog
CisJQXNzZXJ0KHN0YXJ0X3B0ciA+PSBoYXlzdGFjayAmJiBzdGFydF9wdHIgPD0gaGF5c3RhY2tf
ZW5kICYmIG5lZWRsZV9sZW4gPiAwKTsKKwogCWlmICghc3RhdGUtPmxvY2FsZS0+ZGV0ZXJtaW5p
c3RpYykKIAl7CiAJCS8qCkBAIC0xMTIzLDkgKzExMjMsOSBAQCB0ZXh0X3Bvc2l0aW9uX25leHRf
aW50ZXJuYWwoY2hhciAqc3RhcnRfcHRyLCBUZXh0UG9zaXRpb25TdGF0ZSAqc3RhdGUpCiAJCSAq
IG5lZWRsZSB1bmRlciB0aGUgZ2l2ZW4gY29sbGF0aW9uLgogCQkgKgogCQkgKiBOb3RlLCB0aGUg
Zm91bmQgc3Vic3RyaW5nIGNvdWxkIGhhdmUgYSBkaWZmZXJlbnQgbGVuZ3RoIHRoYW4gdGhlCi0J
CSAqIG5lZWRsZSwgaW5jbHVkaW5nIGJlaW5nIGVtcHR5LiAgQ2FsbGVycyB0aGF0IHdhbnQgdG8g
c2tpcCBvdmVyIHRoZQotCQkgKiBmb3VuZCBzdHJpbmcgbmVlZCB0byByZWFkIHRoZSBsZW5ndGgg
b2YgdGhlIGZvdW5kIHN1YnN0cmluZyBmcm9tCi0JCSAqIGxhc3RfbWF0Y2hfbGVuIHJhdGhlciB0
aGFuIGp1c3QgdXNpbmcgdGhlIGxlbmd0aCBvZiB0aGVpciBuZWVkbGUuCisJCSAqIG5lZWRsZS4g
IENhbGxlcnMgdGhhdCB3YW50IHRvIHNraXAgb3ZlciB0aGUgZm91bmQgc3RyaW5nIG5lZWQgdG8K
KwkJICogcmVhZCB0aGUgbGVuZ3RoIG9mIHRoZSBmb3VuZCBzdWJzdHJpbmcgZnJvbSBsYXN0X21h
dGNoX2xlbiByYXRoZXIKKwkJICogdGhhbiBqdXN0IHVzaW5nIHRoZSBsZW5ndGggb2YgdGhlaXIg
bmVlZGxlLgogCQkgKgogCQkgKiBNb3N0IGNhbGxlcnMgd2lsbCByZXF1aXJlICJncmVlZHkiIHNl
bWFudGljcywgbWVhbmluZyB0aGF0IHdlIG5lZWQKIAkJICogdG8gZmluZCB0aGUgbG9uZ2VzdCBz
dWNoIHN1YnN0cmluZywgbm90IHRoZSBzaG9ydGVzdC4gIEZvciBjYWxsZXJzCkBAIC0xMTM2LDYg
KzExMzYsOCBAQCB0ZXh0X3Bvc2l0aW9uX25leHRfaW50ZXJuYWwoY2hhciAqc3RhcnRfcHRyLCBU
ZXh0UG9zaXRpb25TdGF0ZSAqc3RhdGUpCiAJCWhwdHIgPSBzdGFydF9wdHI7CiAJCXdoaWxlICho
cHRyIDwgaGF5c3RhY2tfZW5kKQogCQl7CisJCQljb25zdCBjaGFyICp0ZXN0X2VuZDsKKwogCQkJ
LyoKIAkJCSAqIEZpcnN0IGNoZWNrIHRoZSBjb21tb24gY2FzZSB0aGF0IHRoZXJlIGlzIGEgbWF0
Y2ggaW4gdGhlCiAJCQkgKiBoYXlzdGFjayBvZiBleGFjdGx5IHRoZSBsZW5ndGggb2YgdGhlIG5l
ZWRsZS4KQEAgLTExNDYsMTEgKzExNDgsMTQgQEAgdGV4dF9wb3NpdGlvbl9uZXh0X2ludGVybmFs
KGNoYXIgKnN0YXJ0X3B0ciwgVGV4dFBvc2l0aW9uU3RhdGUgKnN0YXRlKQogCQkJCXJldHVybiAo
Y2hhciAqKSBocHRyOwogCiAJCQkvKgotCQkJICogRWxzZSBjaGVjayBpZiBhbnkgb2YgdGhlIHBv
c3NpYmxlIHN1YnN0cmluZ3Mgc3RhcnRpbmcgYXQgaHB0cgotCQkJICogYXJlIGVxdWFsIHRvIHRo
ZSBuZWVkbGUuCisJCQkgKiBFbHNlIGNoZWNrIGlmIGFueSBvZiB0aGUgcG9zc2libGUgbm9uLWVt
cHR5IHN1YnN0cmluZ3Mgc3RhcnRpbmcKKwkJCSAqIGF0IGhwdHIgYXJlIGVxdWFsIHRvIHRoZSBu
ZWVkbGUuCiAJCQkgKi8KLQkJCWZvciAoY29uc3QgY2hhciAqdGVzdF9lbmQgPSBocHRyOyB0ZXN0
X2VuZCA8IGhheXN0YWNrX2VuZDsgdGVzdF9lbmQgKz0gcGdfbWJsZW4odGVzdF9lbmQpKQorCQkJ
dGVzdF9lbmQgPSBocHRyOworCQkJZG8KIAkJCXsKKwkJCQl0ZXN0X2VuZCArPSBwZ19tYmxlbih0
ZXN0X2VuZCk7CisKIAkJCQlpZiAocGdfc3RybmNvbGwoaHB0ciwgKHRlc3RfZW5kIC0gaHB0ciks
IG5lZWRsZSwgbmVlZGxlX2xlbiwgc3RhdGUtPmxvY2FsZSkgPT0gMCkKIAkJCQl7CiAJCQkJCXN0
YXRlLT5sYXN0X21hdGNoX2xlbl90bXAgPSAodGVzdF9lbmQgLSBocHRyKTsKQEAgLTExNTgsNyAr
MTE2Myw4IEBAIHRleHRfcG9zaXRpb25fbmV4dF9pbnRlcm5hbChjaGFyICpzdGFydF9wdHIsIFRl
eHRQb3NpdGlvblN0YXRlICpzdGF0ZSkKIAkJCQkJaWYgKCFzdGF0ZS0+Z3JlZWR5KQogCQkJCQkJ
YnJlYWs7CiAJCQkJfQotCQkJfQorCQkJfSB3aGlsZSAodGVzdF9lbmQgPCBoYXlzdGFja19lbmQp
OworCiAJCQlpZiAocmVzdWx0X2hwdHIpCiAJCQkJYnJlYWs7CiAKZGlmZiAtLWdpdCBhL3NyYy90
ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQvY29sbGF0ZS5pY3UudXRmOC5vdXQgYi9zcmMvdGVzdC9yZWdy
ZXNzL2V4cGVjdGVkL2NvbGxhdGUuaWN1LnV0Zjgub3V0CmluZGV4IGI4NTc5YTFlZmM2Li4wYTE0
YzFkOTNmZiAxMDA2NDQKLS0tIGEvc3JjL3Rlc3QvcmVncmVzcy9leHBlY3RlZC9jb2xsYXRlLmlj
dS51dGY4Lm91dAorKysgYi9zcmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVkL2NvbGxhdGUuaWN1LnV0
Zjgub3V0CkBAIC0xNDg0LDYgKzE0ODQsMTMgQEAgU0VMRUNUIGFycmF5X3NvcnQoJ3thLEJ9Jzo6
dGV4dFtdIENPTExBVEUgIkMiKTsKICB7QixhfQogKDEgcm93KQogCistLSB0ZXN0IHJlcGxhY2Uo
KSBhdCB0aGUgZW5kIG9mIHRoZSBzdHJpbmcgKGJ1ZyAjMTkzNDEpCitTRUxFQ1QgcmVwbGFjZSgn
dGVzdFgnIENPTExBVEUgY2FzZV9pbnNlbnNpdGl2ZSwgJ3gnIENPTExBVEUgY2FzZV9pbnNlbnNp
dGl2ZSwgJ2VzJyk7CisgcmVwbGFjZSAKKy0tLS0tLS0tLQorIHRlc3RlcworKDEgcm93KQorCiAt
LSB0ZXN0IGxhbmd1YWdlIHRhZ3MKIENSRUFURSBDT0xMQVRJT04gbHRfaW5zZW5zaXRpdmUgKHBy
b3ZpZGVyID0gaWN1LCBsb2NhbGUgPSAnZW4tdS1rcy1sZXZlbDEnLCBkZXRlcm1pbmlzdGljID0g
ZmFsc2UpOwogU0VMRUNUICdhQmNEJyBDT0xMQVRFIGx0X2luc2Vuc2l0aXZlID0gJ0FiQ2QnIENP
TExBVEUgbHRfaW5zZW5zaXRpdmU7CmRpZmYgLS1naXQgYS9zcmMvdGVzdC9yZWdyZXNzL3NxbC9j
b2xsYXRlLmljdS51dGY4LnNxbCBiL3NyYy90ZXN0L3JlZ3Jlc3Mvc3FsL2NvbGxhdGUuaWN1LnV0
Zjguc3FsCmluZGV4IDZmNWFiYWMwZGMwLi41ZTNmZWY5YjM4MSAxMDA2NDQKLS0tIGEvc3JjL3Rl
c3QvcmVncmVzcy9zcWwvY29sbGF0ZS5pY3UudXRmOC5zcWwKKysrIGIvc3JjL3Rlc3QvcmVncmVz
cy9zcWwvY29sbGF0ZS5pY3UudXRmOC5zcWwKQEAgLTU2OCw2ICs1NjgsOSBAQCBTRUxFQ1QgJ2Fi
YycgPD0gJ0FCQycgQ09MTEFURSBjYXNlX2luc2Vuc2l0aXZlLCAnYWJjJyA+PSAnQUJDJyBDT0xM
QVRFIGNhc2VfaW5zZQogU0VMRUNUIGFycmF5X3NvcnQoJ3thLEJ9Jzo6dGV4dFtdIENPTExBVEUg
Y2FzZV9pbnNlbnNpdGl2ZSk7CiBTRUxFQ1QgYXJyYXlfc29ydCgne2EsQn0nOjp0ZXh0W10gQ09M
TEFURSAiQyIpOwogCistLSB0ZXN0IHJlcGxhY2UoKSBhdCB0aGUgZW5kIG9mIHRoZSBzdHJpbmcg
KGJ1ZyAjMTkzNDEpCitTRUxFQ1QgcmVwbGFjZSgndGVzdFgnIENPTExBVEUgY2FzZV9pbnNlbnNp
dGl2ZSwgJ3gnIENPTExBVEUgY2FzZV9pbnNlbnNpdGl2ZSwgJ2VzJyk7CisKIC0tIHRlc3QgbGFu
Z3VhZ2UgdGFncwogQ1JFQVRFIENPTExBVElPTiBsdF9pbnNlbnNpdGl2ZSAocHJvdmlkZXIgPSBp
Y3UsIGxvY2FsZSA9ICdlbi11LWtzLWxldmVsMScsIGRldGVybWluaXN0aWMgPSBmYWxzZSk7CiBT
RUxFQ1QgJ2FCY0QnIENPTExBVEUgbHRfaW5zZW5zaXRpdmUgPSAnQWJDZCcgQ09MTEFURSBsdF9p
bnNlbnNpdGl2ZTsKLS0gCjIuNTIuMAoK


--=-LnyTNpmOxSz6OpUllAmU--