Re: [PATCH] Use Boyer-Moore-Horspool for simple LIKE contains patterns

From: Greg Sabino Mullane <htamfids(at)gmail(dot)com>
To: 小川淳 <atsushi(dot)ogawa001(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Use Boyer-Moore-Horspool for simple LIKE contains patterns
Date: 2026-07-14 18:28:56
Message-ID: CAKAnmm+xYBdm3vVXQhrqRu2-q=RiPBz6m87uCPSnzHdQaQTqrQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Great idea, love seeing the speedups! Also appreciate the background,
detailed explanation, and benchmarks. Quick code review:

git grep shows we already use BMH in src/backend/utils/adt/varlena.c
Worth acknowledging that in a code comment somewhere? I didn't see any
obvious advantage to refactoring things out at quick glance, but a mention
might be nice.

> + * by '%' wildcards. Remove backslash escapes while building the search
state.

Slightly off comment. This is for like_bmh_pattern_is_eligible - we are not
removing here, just skipping things when we count.

> if (i + 1 >= plen - 1)

Worth a comment to explain that we are catching the '%foo\%' case here.

> pattern_stable = get_fn_expr_arg_stable(flinfo, 1);
>
> /*
> * ScalarArrayOpExpr invokes the operator once per array element. The
> * array expression can be stable while the pattern passed to this function
> * changes between calls, so it must not use a cached search state.
> */
> if (flinfo->fn_expr != NULL && IsA(flinfo->fn_expr, ScalarArrayOpExpr))
> pattern_stable = false;

My first thought was to make this an if/else so we don't reclobber, but
seeing how later on we check collation every time, I'm wondering if we
shouldn't just check the pattern as well every time via a memcmp like
regexp.c does in RE_compile_and_cache (and remove that block above). So we
store it verbatim in the like_bmh_init() function with memcpy, then make
the check inside like_bmh_match() that looks like this:

unlikely(collation has changed)

into:

unlikely(
collation has changed
OR pattern length has changed
OR pattern itself has changed (e.g. memcmp true)
)

Also means you could then roll get_fn_expr_arg_stable into that big old ||
grouping, and remove pattern_stable entirely.

Hm...that collation test and message is already caught and done by
GenericMatchText, so you could throw !OidIsValid(collation) into that ||
group as well, and remove the ereport section entirely. It then falls
through later to GenericMatchText, which complains about the collation
there.

Anyway, the patch compiled cleanly against d15a6bc2 (Tue Jul 14 10:28:04
2026 +0200)

It did have one test failure:

@@ -151,8 +151,8 @@
p | matched
--------+---------
%abcd% | t
- %b%e% | f
%b_d% | t
+ %b%e% | f
%wxyz% | f
(4 rows)

I think it's from the "Row-varying patterns must use the generic matcher."
test.

Cheers,
Greg

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2026-07-14 19:15:26 Re: uuidv7 improperly accepts dates before 1970-01-01
Previous Message Tom Lane 2026-07-14 18:26:50 Re: document the dangers of granting TRIGGER or REFERENCES