perf: make WHERE col ~~ val engage the bloom_filter index#201
Conversation
📝 WalkthroughWalkthroughThis PR converts three ChangesInlinable LIKE/ILIKE operators with bloom index
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/operators/~~.sql (3)
84-89: 💤 Low valueConversion to inlinable SQL wrappers is correct — minor consistency nit.
All three overloads now match the documented inlining recipe (
LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE, noSET search_path, single-statement body), so the planner can foldeql_v2."~~"(col, val)→eql_v2.like(col, val)→eql_v2.bloom_filter(col) @> eql_v2.bloom_filter(val)and match the functional index. One small consistency nit: theSELECTstatements in the new bodies are missing the trailing semicolons that the surrounding helpers (eql_v2.likeline 28,eql_v2.ilikeline 50) use.Proposed terminator nit
AS $$ - SELECT eql_v2.like(a, b) + SELECT eql_v2.like(a, b); $$;AS $$ - SELECT eql_v2.like(a, b::eql_v2_encrypted) + SELECT eql_v2.like(a, b::eql_v2_encrypted); $$;AS $$ - SELECT eql_v2.like(a::eql_v2_encrypted, b) + SELECT eql_v2.like(a::eql_v2_encrypted, b); $$;Also applies to: 134-139, 173-178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/operators/`~~.sql around lines 84 - 89, Add missing statement terminators: update the SQL wrapper functions (notably eql_v2."~~" and the other overloads at the same pattern) so their single-statement bodies end with a trailing semicolon (i.e., change the body SELECT eql_v2.like(a, b) to SELECT eql_v2.like(a, b);), keeping LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE and no SET search_path so they remain inlinable and consistent with eql_v2.like and eql_v2.ilike wrappers.
53-83: 💤 Low valuePre-existing duplicate
@brieftag in the doc block.The block above the
(enc, enc)overload contains two@brieflines (line 53 and again line 75). Not introduced by this PR, but sincemise run docs:validateruns over this file per the repo's Doxygen coverage rule, worth tidying alongside this change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/operators/`~~.sql around lines 53 - 83, The docblock for the encrypted LIKE operator contains a duplicate `@brief` tag; remove one of the `@brief` lines so there is only a single `@brief` in the comment for the (enc, enc) overload (refer to the operator description mentioning "LIKE operator for encrypted values" / "SQL LIKE operator (~~ operator)" and symbols eql_v2.like and eql_v2.add_search_config), keeping the clearer/most complete brief text, and then re-run the docs:validate check to confirm the duplication is resolved.
84-89: 💤 Low valueFully schema-qualify
eql_v2_encryptedin function signatures and casts for defensive consistency.The new wrapper functions on lines 84–89, 134–139, and 173–178 reference
eql_v2_encryptedwithout schema qualification in both parameter types and casts (::eql_v2_encrypted). While this works correctly because the type exists only in thepublicschema, unqualified type references in casts implicitly depend on the caller'ssearch_path. Usingpublic.eql_v2_encryptedremoves this implicit dependency without affecting inlinability of IMMUTABLE LANGUAGE SQL functions. The helper functionseql_v2.likeandeql_v2.ilikefollow the same unqualified pattern, making this a broader consistency improvement rather than a blocking issue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/operators/`~~.sql around lines 84 - 89, The wrapper functions referencing the type eql_v2_encrypted must use the schema-qualified type to avoid search_path dependency: update the parameter type declarations and any casts (::eql_v2_encrypted) in the wrapper functions (e.g., function eql_v2."~~" and the other wrapper functions that call eql_v2.like and eql_v2.ilike) to use public.eql_v2_encrypted instead of the unqualified eql_v2_encrypted, leaving the IMMUTABLE LANGUAGE SQL definitions otherwise unchanged.tasks/pin_search_path.sql (1)
60-64: 💤 Low valueComment slightly overstates the role of
eql_v2.ilike.The block comment says inlining is required for
eql_v2.like/eql_v2.ilike, but insrc/operators/~~.sqlboth the~~and~~*operators are wired to the sameeql_v2."~~"function which always delegates toeql_v2.like(...)—eql_v2.ilikeis never invoked through the operator path. Includingilikein the allowlist is fine as defensive coverage for direct callers, but the comment reads as if~~*flows througheql_v2.ilike, which it does not. Consider clarifying so future readers don't assume the operator dispatch differs by case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tasks/pin_search_path.sql` around lines 60 - 64, Update the block comment to clarify that both `~~` and `~~*` operators are routed to the same `eql_v2."~~"` function which delegates to `eql_v2.like(...)`, and that `eql_v2.ilike` is not invoked by the operator dispatch path; note that `eql_v2.ilike` may still be allowlisted as defensive coverage for direct calls but is not part of the `~~*` operator flow. Reference the operator entrypoint `eql_v2."~~"` and the helper `eql_v2.like`/`eql_v2.ilike` so readers understand dispatch behavior and why pinning either layer affects planner inlining.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/operators/`~~.sql:
- Around line 84-89: Add missing statement terminators: update the SQL wrapper
functions (notably eql_v2."~~" and the other overloads at the same pattern) so
their single-statement bodies end with a trailing semicolon (i.e., change the
body SELECT eql_v2.like(a, b) to SELECT eql_v2.like(a, b);), keeping LANGUAGE
sql IMMUTABLE STRICT PARALLEL SAFE and no SET search_path so they remain
inlinable and consistent with eql_v2.like and eql_v2.ilike wrappers.
- Around line 53-83: The docblock for the encrypted LIKE operator contains a
duplicate `@brief` tag; remove one of the `@brief` lines so there is only a single
`@brief` in the comment for the (enc, enc) overload (refer to the operator
description mentioning "LIKE operator for encrypted values" / "SQL LIKE operator
(~~ operator)" and symbols eql_v2.like and eql_v2.add_search_config), keeping
the clearer/most complete brief text, and then re-run the docs:validate check to
confirm the duplication is resolved.
- Around line 84-89: The wrapper functions referencing the type eql_v2_encrypted
must use the schema-qualified type to avoid search_path dependency: update the
parameter type declarations and any casts (::eql_v2_encrypted) in the wrapper
functions (e.g., function eql_v2."~~" and the other wrapper functions that call
eql_v2.like and eql_v2.ilike) to use public.eql_v2_encrypted instead of the
unqualified eql_v2_encrypted, leaving the IMMUTABLE LANGUAGE SQL definitions
otherwise unchanged.
In `@tasks/pin_search_path.sql`:
- Around line 60-64: Update the block comment to clarify that both `~~` and
`~~*` operators are routed to the same `eql_v2."~~"` function which delegates to
`eql_v2.like(...)`, and that `eql_v2.ilike` is not invoked by the operator
dispatch path; note that `eql_v2.ilike` may still be allowlisted as defensive
coverage for direct calls but is not part of the `~~*` operator flow. Reference
the operator entrypoint `eql_v2."~~"` and the helper
`eql_v2.like`/`eql_v2.ilike` so readers understand dispatch behavior and why
pinning either layer affects planner inlining.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 895be803-6fd9-407b-a90b-66605ed8b955
📒 Files selected for processing (3)
src/operators/~~.sqltasks/pin_search_path.sqltests/sqlx/tests/bench_plan_tests.rs
The `~~` (LIKE) and `~~*` (ILIKE) operators on `eql_v2_encrypted` could not engage the documented bloom_filter functional index from a bare predicate (`WHERE col ~~ val`). Two issues blocked end-to-end inlining: 1. The `eql_v2."~~"` operator wrappers were `LANGUAGE plpgsql` with `SET search_path` — never inlinable. Flipped to inlinable `LANGUAGE sql IMMUTABLE STRICT PARALLEL SAFE` (matching the helper they delegate to and the documented index target). 2. The `eql_v2.like` / `eql_v2.ilike` helpers — already inlinable since #189 — were getting `SET search_path` retroactively applied by the post-install loop in `tasks/pin_search_path.sql`, which silently re-disables inlining. Adds them to the inline-critical allowlist alongside `~~`'s same-type and cross-type overloads so the planner can inline both layers and reach `eql_v2.bloom_filter(a) @> eql_v2.bloom_filter(b)`. Verified on the 10K-row bench fixture (PG 17, no operator-class indexes to simulate Supabase): `WHERE col ~~ val` drops from 247ms (Seq Scan) to 0.27ms (Bitmap Index Scan via `bench_text_bloom_idx`), matching the explicitly-wrapped form. Adds bench plan assertions `bare_like_uses_bloom_index` and `bare_ilike_uses_bloom_index` to keep this regression-tested.
coderdan
force-pushed
the
fix/like-ilike-inlining
branch
from
029d29f to
cea75b8
Compare
2 participants
Summary
WHERE col ~~ val/~~* valagainst aneql_v2_encryptedcolumn does not engage the documented bloom_filter functional index — the planner stops short of the canonical containment form and falls back to a sequential scan. This PR makes both operators inlinable end-to-end so bare-form queries from PostgREST and ORMs that don't wrap columns themselves match the index.PR #196 (now merged) flipped the
eql_v2."~~"operator wrappers to inlinable SQL alongside the rest of the Phase 1 operators, but the bloom_filter index still didn't engage from a bare predicate because the post-installtasks/pin_search_path.sqlloop pinnedSET search_pathon theeql_v2.like/eql_v2.ilikehelpers the wrappers delegate to — silently re-disabling inlining at the second layer.This PR adds
likeandiliketo the inline-critical allowlist intasks/pin_search_path.sql(with matching justifications intasks/test/splinter.sh) so both inlining layers stay live end-to-end.Mechanism
The functional index
CREATE INDEX ... USING gin (eql_v2.bloom_filter(col))matches the resulting expression and is picked.Verification
10K-row bench fixture, PG 17, EXPLAIN ANALYZE. "Supabase" = same DB with the operator-class btree indexes (
bench_text_ore_idx,bench_int_ore_idx,bench_bigint_ore_idx) dropped to simulate a managed-Postgres install where opclass indexes aren't supported.WHERE col = $1WHERE hmac_256(col) = hmac_256($1)WHERE col ~~ $1(bare LIKE)WHERE col ~~* $1(bare ILIKE)WHERE bloom_filter(col) @> bloom_filter($1)Headline: bare LIKE/ILIKE drops ~111 ms → ~0.20 ms (~560×) and matches the explicitly-wrapped form. Bare equality unchanged (PR #196 already fixed it). Wrapped forms unchanged (sanity).
Test plan
mise run buildproduces a clean release bundle.pg_proc.provolatile = 'i'andproconfig IS NULLfor~~,like,ilikepost-install (confirmed manually).bare_like_uses_bloom_indexandbare_ilike_uses_bloom_indexpass on PG 17.like_operator_tests(5 tests, including the IMMUTABLE assertion from perf: eql_v2.like / eql_v2.ilike are VOLATILE — blocks planner inlining and bloom_filter index match #189) all pass.mise run test:splinterclean — 22/22 findings allowlisted, including the two new entries foreql_v2.likeandeql_v2.ilike.