Re: [PATCH] Use Boyer-Moore-Horspool for simple LIKE contains patterns - Mailing list pgsql-hackers

From Greg Sabino Mullane
Subject Re: [PATCH] Use Boyer-Moore-Horspool for simple LIKE contains patterns
Date
Msg-id CAKAnmm+xYBdm3vVXQhrqRu2-q=RiPBz6m87uCPSnzHdQaQTqrQ@mail.gmail.com
Whole thread
In response to [PATCH] Use Boyer-Moore-Horspool for simple LIKE contains patterns  (小川淳 <atsushi.ogawa001@gmail.com>)
Responses Re: [PATCH] Use Boyer-Moore-Horspool for simple LIKE contains patterns
List 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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: document the dangers of granting TRIGGER or REFERENCES
Next
From: Masahiko Sawada
Date:
Subject: Re: uuidv7 improperly accepts dates before 1970-01-01