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
