mirror of
https://github.com/block/buzz.git
synced 2026-11-09 02:41:57 +00:00
fix(search): parse kind-0 content once and name the PostgreSQL 16 floor
Addresses the three findings from the review of ca974eb29; no behaviour
changes beyond what it asks.
The kind-0 `name_match` select column parsed `content` four times per
candidate row (one `pg_input_is_valid` plus three `::jsonb` casts), and it
ran for every row the WHERE admitted because the inner DISTINCT ON subquery
has no LIMIT. On a brownfield database the candidate set is exactly the
noisy one this branch fixes, so a short typeahead could spend hundreds of
milliseconds per keystroke in `jsonb_in`. Parse once instead: a
`CROSS JOIN LATERAL (SELECT CASE WHEN pg_input_is_valid(content, 'jsonb')
THEN content::jsonb END AS j OFFSET 0) AS profile` joins only for
`kinds == [0]`, and `name_match` reads `profile.j ->> 'name'`,
`'display_name'`, and `'displayName'`. `OFFSET 0` stops the planner pulling
the subquery up and re-expanding the cast at each reference. A NULL `j`
(invalid JSON) or a non-object document yields NULL from `->>`, an empty
vector, and `false`, so `name_match` is still never NULL and the ORDER BY is
unchanged. Every changed `push` sits inside the `profile_query` branch and
the split literal concatenates to the previous bytes, so the SQL emitted for
non-kind-0 searches is byte-identical.
Verified on PostgreSQL 17.9: with a notice-raising IMMUTABLE stand-in for
the cast, the lateral shape evaluates it 7 times over 7 rows and the
pulled-up shape without `OFFSET 0` evaluates it 21 times; `name_match` is
`false`, not NULL, for `not json`, `[1,2]`, `"wes"`, and `null` content.
Migration 0056 is the first to require PostgreSQL 16, and
`check_function_bodies` made it fail on an older server with a bare
"function pg_input_is_valid(text, unknown) does not exist". A preflight
`DO` block at the top now raises "Buzz requires PostgreSQL 16 or later
(migration 0056 uses pg_input_is_valid); this server is <version>" before
CREATE FUNCTION runs. The embedded-migrator content test asserts the
preflight precedes CREATE FUNCTION. README.md and the CONTRIBUTING.md
prerequisites table record the floor, noting that both Compose bundles ship
17 so it only matters for a database you run yourself. 0056 is unmerged, so
only development databases that already applied this branch's earlier
version see its checksum change.
`scripts/maintenance/nip_rs_search_allowlist.sql` now runs the same
`to_regprocedure('profile_search_tsv(text)')` precheck as
`profile_search_text_fields.sql`, after BEGIN and lock_timeout but before
ALTER TABLE, so a database that has not started on 0056 fails with the
readable instruction before taking ACCESS EXCLUSIVE. Exercised against an
empty scratch database: BEGIN, SET, the instruction, exit 3, no lock.
Verified with the lld linker workaround for sqlx proc-macros on this
machine: `scripts/postgres-test-run.sh -p buzz-search --tests` (23 passed,
including the four kind-0 tests and the two pre-existing ones);
`scripts/postgres-test-run.sh -p buzz-db --lib -E 'test(migration::postgres_tests)'`
(12 passed); `cargo test -p buzz-db --lib runtime::migration` (14 passed);
`cargo fmt --all --check`; and `cargo clippy -p buzz-search -p buzz-db
--all-targets -- -D warnings`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
This commit is contained in:
@@ -85,6 +85,7 @@ We review as capacity allows — focused PRs that follow this guide move fastest
|
||||
| pnpm | 10+ | Required for desktop app commands and `just ci` |
|
||||
| Flutter | 3.41+ | Required for mobile app — install via [flutter.dev](https://docs.flutter.dev/get-started/install) |
|
||||
| Docker | 24+ | For Postgres, Redis, MinIO |
|
||||
| PostgreSQL | 16+ | Docker provides 17; a database you run yourself must be 16 or later because migration 0056 uses `pg_input_is_valid` |
|
||||
| `just` | latest | Task runner — `cargo install just` |
|
||||
| `lefthook` | 2.1.3 (Hermit-pinned) | Auto-installed by `just hooks` — no manual install needed |
|
||||
| `sqlx` migrations | workspace crate | `just migrate` applies embedded migrations from `migrations/` |
|
||||
|
||||
@@ -177,6 +177,8 @@ For a split-terminal workflow (relay logs separate from Vite output), use `just
|
||||
|
||||
Want a single-node / VPS relay instead of the local-dev stack? Use the production Compose bundle in [`deploy/compose/`](deploy/compose/README.md) (`docker compose` + Postgres, Redis, MinIO, optional Caddy/TLS). The root [`docker-compose.yml`](docker-compose.yml) is for day-to-day development only.
|
||||
|
||||
The relay needs PostgreSQL 16 or later (its migrations use `pg_input_is_valid`). Both Compose bundles ship 17, so this only matters when `DATABASE_URL` points at a database you run yourself; an older server fails migration 0056 at startup with a message naming the requirement.
|
||||
|
||||
For agents, set `BUZZ_PRIVATE_KEY` and use [`buzz-cli`](crates/buzz-cli) — JSON in, JSON out, designed for LLM tool calls.
|
||||
|
||||
---
|
||||
|
||||
@@ -960,6 +960,15 @@ mod postgres_tests {
|
||||
let profile_search = migrations[55].sql.as_str();
|
||||
assert!(profile_search.contains("CREATE FUNCTION profile_search_tsv(content TEXT)"));
|
||||
assert!(profile_search.contains("pg_input_is_valid(content, 'jsonb')"));
|
||||
// PostgreSQL 16 floor: the preflight must run before CREATE FUNCTION so
|
||||
// an older server reports the requirement, not a missing-function error.
|
||||
let preflight = profile_search
|
||||
.find("current_setting('server_version_num')::int < 160000")
|
||||
.expect("0056 preflights the PostgreSQL 16 floor");
|
||||
let create_function = profile_search
|
||||
.find("CREATE FUNCTION profile_search_tsv")
|
||||
.expect("0056 creates profile_search_tsv");
|
||||
assert!(preflight < create_function);
|
||||
assert!(profile_search.contains("IF NOT EXISTS (SELECT 1 FROM events LIMIT 1)"));
|
||||
assert!(profile_search.contains("CASE WHEN kind = 0 THEN profile_search_tsv(content)"));
|
||||
assert!(!migrations[0].sql.as_str().contains("profile_search_tsv"));
|
||||
|
||||
@@ -207,10 +207,11 @@ fn normalized_search_text(q: &str) -> Option<String> {
|
||||
/// SELECT [DISTINCT ON (pubkey)] id, kind, pubkey, channel_id,
|
||||
/// EXTRACT(EPOCH FROM created_at)::bigint AS created_at_s,
|
||||
/// ts_rank_cd(search_tsv, query) AS rank
|
||||
/// [, created_at, <profile name fields as tsvector> @@ query AS name_match]
|
||||
/// [, created_at, <profile.j name fields as tsvector> @@ query AS name_match]
|
||||
/// [, search_tsv] -- prefix mode, ≤ 2 chars
|
||||
/// FROM events,
|
||||
/// <mode-specific tsquery> AS query
|
||||
/// [, LATERAL (SELECT <content::jsonb, NULL if invalid> AS j OFFSET 0) AS profile]
|
||||
/// WHERE community_id = $ctx
|
||||
/// AND deleted_at IS NULL
|
||||
/// AND search_tsv @@ query
|
||||
@@ -276,18 +277,15 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result<SearchResult,
|
||||
if profile_query {
|
||||
// `created_at` feeds the outer ORDER BY through the wrapper.
|
||||
// `name_match` applies the same tsquery as the WHERE clause to the
|
||||
// name fields only, so both sides normalize identically. `CASE` skips
|
||||
// the `THEN` arm when the guard is false, so rows that predate
|
||||
// ingest-side JSON validation never raise on `::jsonb`; a non-object
|
||||
// document yields NULL from `->>`, an empty vector, and `false`.
|
||||
// `pg_input_is_valid` needs PostgreSQL 16 or later.
|
||||
// name fields only, so both sides normalize identically. `profile.j`
|
||||
// is the row's content parsed once by the LATERAL below: NULL for
|
||||
// invalid JSON, and `->>` is NULL for a non-object document, so either
|
||||
// yields an empty vector and `false`; `name_match` is never NULL.
|
||||
qb.push(
|
||||
", created_at, CASE WHEN pg_input_is_valid(content, 'jsonb') \
|
||||
THEN to_tsvector('simple', concat_ws(' ', \
|
||||
content::jsonb ->> 'name', \
|
||||
content::jsonb ->> 'display_name', \
|
||||
content::jsonb ->> 'displayName')) @@ search_query.query \
|
||||
ELSE false END AS name_match",
|
||||
", created_at, to_tsvector('simple', concat_ws(' ', \
|
||||
profile.j ->> 'name', \
|
||||
profile.j ->> 'display_name', \
|
||||
profile.j ->> 'displayName')) @@ search_query.query AS name_match",
|
||||
);
|
||||
if prioritize_exact_profile_lexeme {
|
||||
// The outer exact-lexeme term reads `search_tsv` through the wrapper.
|
||||
@@ -296,7 +294,20 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result<SearchResult,
|
||||
}
|
||||
qb.push(" FROM events CROSS JOIN LATERAL (SELECT ");
|
||||
push_tsquery(&mut qb, query.mode, &search_text);
|
||||
qb.push(" AS query) AS search_query WHERE community_id = ");
|
||||
qb.push(" AS query) AS search_query");
|
||||
if profile_query {
|
||||
// Parse `content` once per candidate row rather than once per
|
||||
// reference. `CASE` skips the cast when the guard is false, so rows
|
||||
// that predate ingest-side JSON validation never raise on `::jsonb`.
|
||||
// `OFFSET 0` keeps the planner from pulling the subquery up and
|
||||
// re-expanding the cast at every `profile.j` reference.
|
||||
// `pg_input_is_valid` needs PostgreSQL 16 or later.
|
||||
qb.push(
|
||||
" CROSS JOIN LATERAL (SELECT CASE WHEN pg_input_is_valid(content, 'jsonb') \
|
||||
THEN content::jsonb END AS j OFFSET 0) AS profile",
|
||||
);
|
||||
}
|
||||
qb.push(" WHERE community_id = ");
|
||||
qb.push_bind(*query.community.as_uuid());
|
||||
qb.push(" AND deleted_at IS NULL AND search_tsv @@ search_query.query");
|
||||
|
||||
|
||||
@@ -4,6 +4,19 @@
|
||||
-- make a hundred profiles match a name prefix that none of them carry and push
|
||||
-- the person actually named that off the first page.
|
||||
--
|
||||
-- This is the first migration to require PostgreSQL 16. `check_function_bodies`
|
||||
-- validates the SQL body at CREATE FUNCTION, so on an older server the function
|
||||
-- below would fail with a bare "function pg_input_is_valid(text, unknown) does
|
||||
-- not exist" and the relay would refuse to start without saying why. Name the
|
||||
-- real cause first.
|
||||
DO $$
|
||||
BEGIN
|
||||
IF current_setting('server_version_num')::int < 160000 THEN
|
||||
RAISE EXCEPTION 'Buzz requires PostgreSQL 16 or later (migration 0056 uses pg_input_is_valid); this server is %',
|
||||
current_setting('server_version');
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
-- Requires PostgreSQL 16 or later: `pg_input_is_valid` guards the
|
||||
-- `content::jsonb` cast so rows that predate ingest-side JSON validation never
|
||||
-- raise. It is STABLE, so this IMMUTABLE SQL wrapper is what lets a
|
||||
|
||||
@@ -8,11 +8,21 @@
|
||||
--
|
||||
-- The result is the expression migration 0056 gives empty installations:
|
||||
-- kind 0 routes through profile_search_tsv(content) (created by 0056 at relay
|
||||
-- startup, so run this after the relay has started on 0056). That keeps this
|
||||
-- rewrite from undoing scripts/maintenance/profile_search_text_fields.sql.
|
||||
-- startup, so run this after the relay has started on 0056; the guard below
|
||||
-- refuses otherwise). That keeps this rewrite from undoing
|
||||
-- scripts/maintenance/profile_search_text_fields.sql.
|
||||
BEGIN;
|
||||
SET LOCAL lock_timeout = '5s';
|
||||
|
||||
-- Fail with an instruction before ALTER TABLE takes ACCESS EXCLUSIVE, rather
|
||||
-- than inside the rewrite with a bare "function does not exist".
|
||||
DO $$
|
||||
BEGIN
|
||||
IF to_regprocedure('profile_search_tsv(text)') IS NULL THEN
|
||||
RAISE EXCEPTION 'profile_search_tsv(text) is missing; start the relay once so migration 0056 creates it, then rerun';
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
ALTER TABLE events DROP COLUMN search_tsv;
|
||||
ALTER TABLE events ADD COLUMN search_tsv TSVECTOR GENERATED ALWAYS AS (
|
||||
CASE WHEN kind = 0 THEN profile_search_tsv(content)
|
||||
|
||||
Reference in New Issue
Block a user