[PATCH] Fix for bug #19474: LIKE fails to match literal backslashes with nondeterministic collations

Hello!

I verified that the patch works, but I have one concern:

I didn't check for the end of the
string because that check is already handled in the block above when
checking for escape characters.

The code isn't easy to reason about like this, it relies on specific details of the outer loop, which was only mentioned in the email itself.

This should be also explained in a comment and the commit message, or maybe instead of the current way, the loop could work similarly like how another loop uses an afterescape flag in do_like_escape (in the same file), that form seems less fragile.

Thanks Zsolt for the review.

The code isn't easy to reason about like this, it relies on specific
details of the outer loop, which was only mentioned in the email
itself.

This should be also explained in a comment and the commit message, or
maybe instead of the current way, the loop could work similarly like
how another loop uses an afterescape flag in do_like_escape (in the
same file), that form seems less fragile.

I have changed the code to use an 'afterescape' flag like in
'do_like_escape'. I also realized that 'do_like_escape' uses NextChar
to handle multibyte encodings. So I changed the byte by byte copy to
use NextChar and then copy the whole character. I think byte-by-byte
copying should be enough for most cases, but if an encoding has '\' as
second or third byte, that might not work.

This copying can also be done with CopyAdvChar, as 'do_like_escape'
does, but that macro is not defined for all cases. So for the time
being, I just used NextChar and copied the character myself. We can
also define CopyAdvChar and ust it here for the code to be consistent
across functions.

Let me know your thoughts on the above approaches.

Regards,
Nitin Motiani
Google

Hi,

I have created a commitfest entry for this patch here
https://commitfest.postgresql.org/patch/6844/. Please take a look.

Thanks,
Nitin Motiani
Google

Hi,

I reviewed the v2 patch.

The patch applies cleanly on the current master (4cb2a9863d8). I built with
--enable-cassert and ICU, and confirmed that the three backslash test
cases give wrong results on unpatched master and the expected results
with the patch applied. The full regression suite passes (245/245),
including the updated collate.icu.utf8 test.

Two comments:

1. The commit message describes the symptom as "an incorrect match
failure", but the bug also causes incorrect matches in the other
direction. Since the unescaping logic dropped the literal backslash
from the pattern, a text *without* a backslash could wrongly match a
pattern that requires one:

SELECT 'backslash' COLLATE ignore_accents LIKE 'back\\slash%';
-- unpatched: t (wrong), patched: f (correct)

I think it's worth mentioning this false-positive side of the bug in
the commit message, since silently-too-permissive LIKE filters are
arguably the more dangerous symptom for applications.

2. A small typo in the new comment in like_match.c:
"occurences" should be "occurrences".

Best regards

I reviewed the v2 patch.

Thanks for the feedback.

1. The commit message describes the symptom as "an incorrect match
failure", but the bug also causes incorrect matches in the other
direction. Since the unescaping logic dropped the literal backslash
from the pattern, a text *without* a backslash could wrongly match a
pattern that requires one:

SELECT 'backslash' COLLATE ignore_accents LIKE 'back\\slash%';
-- unpatched: t (wrong), patched: f (correct)

I think it's worth mentioning this false-positive side of the bug in
the commit message, since silently-too-permissive LIKE filters are
arguably the more dangerous symptom for applications.

I have updated the commit message. I also added another test for this
scenario in v3.

2. A small typo in the new comment in like_match.c:
"occurences" should be "occurrences".

Fixed the typo.

Thanks,
Nitin Motiani
Google

Hi Nitin,

Thanks for v3.

I re-applied it on top of current master (4cb2a986) and re-ran the
regression suite: all 245 tests pass, and the collate.icu.utf8 set now
covers both directions of the bug, including the false-positive case
('backslash' LIKE 'back\\slash%' -> f).

LGTM, and changed commitfest status to "Ready for Commiter".

Regards,
Ewan Young

Nitin Motiani <nitinmotiani@google.com> writes:

I have updated the commit message. I also added another test for this
scenario in v3.

I don't think this patch is ready. It is wasting code and cycles to
fix a nonexistent problem. We are working in a backend-safe encoding,
therefore it is not possible for one byte of a multibyte character to
match '\' (nor '_' nor '%'). So the existing logic that pays no
attention to multibyte boundaries is not wrong, and if it were then
the preceding loop that looks for the end of the literal-match
substring would also be wrong (as well as some hundreds of other
places that make similar assumptions).

This code does get consecutive-backslash cases wrong, and we need
to fix that, but we don't need to add complexity to fix something
that's not broken.

regards, tom lane

On Sat, Jul 4, 2026 at 3:03 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't think this patch is ready. It is wasting code and cycles to
fix a nonexistent problem. We are working in a backend-safe encoding,
therefore it is not possible for one byte of a multibyte character to
match '\' (nor '_' nor '%'). So the existing logic that pays no
attention to multibyte boundaries is not wrong, and if it were then
the preceding loop that looks for the end of the literal-match
substring would also be wrong (as well as some hundreds of other
places that make similar assumptions).

This code does get consecutive-backslash cases wrong, and we need
to fix that, but we don't need to add complexity to fix something
that's not broken.

Thanks for the feedback, Tom. I have removed the logic for the
multibyte check and simplified the code. Please take a look at the
latest patch.

Regards,
Nitin Motiani
Google

On Tue, Jul 7, 2026 at 12:25 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Oh ... I simplified the patch and pushed it just moments ago.

Thanks for taking care of it, Tom.