diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 23961d940c..b7e5df8d71 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -582,12 +582,16 @@ re-authorizes every candidate hit before returning it. **Key behaviors:** - `SearchService::new(pool)` wraps a `PgPool`; `search(&SearchQuery)` runs a parameterized FTS query against the `events.search_tsv` GIN index and returns - `SearchResult` (candidate `SearchHit`s). + `SearchResult` (candidate `SearchHit`s). Prefix-mode relevance (outside + profile search) ranks only the newest 1000 matches, which Postgres may read + from the primary key backward instead of the GIN index. - `ChannelScope` makes the channel constraint explicit (`Any` / `ChannelLessOnly` / `Channels` / `ChannelsOrChannelLess`), closing the ambiguity the old `Option> + bool` matrix could not express. -- Every query carries `community_id`; the FTS predicate is BitmapAnd-ed with - the community-leading btree filters so a query never crosses tenants. +- Every query carries `community_id`, and every access path leads with it (the + FTS predicate BitmapAnd-ed with community-leading btree filters, or the + `(community_id, created_at, id)` primary key read backward), so a query never + crosses tenants. - Permission filtering is **caller's responsibility** — `buzz-search` returns candidate hits; the relay re-authorizes each one (channel membership, `#p`, owner gates) before delivering it. @@ -959,7 +963,10 @@ kinds via `CASE WHEN kind IN (1059, 30300, 30622) THEN NULL`, so those rows are storage-level unsearchable (a `NULL` tsvector never matches `@@`). A GIN index (`idx_events_search_tsv`) backs the `@@` probe; in multi-community mode the community-leading btree filters BitmapAnd with the GIN probe so every query is -fenced to its `community_id`. +fenced to its `community_id`. Prefix-mode relevance ranks only the newest 1000 +matches; for a common prefix Postgres reads the `(community_id, created_at, id)` +primary key backward and stops there, still fenced by its leading +`community_id`. --- diff --git a/crates/buzz-search/src/cancel.rs b/crates/buzz-search/src/cancel.rs index 1ca4bf2090..9549d39ba2 100644 --- a/crates/buzz-search/src/cancel.rs +++ b/crates/buzz-search/src/cancel.rs @@ -57,13 +57,15 @@ pub const CANCEL_SQL: &str = concat!( " = $2" ); -/// Run `query` on a connection from `pool`, cancelling its statement on the -/// server if the returned future is dropped before it completes. +/// Run `query` on `conn`, a connection from `pool`, cancelling its statement +/// on the server if the returned future is dropped before it completes. +/// Callers pass the connection so earlier steps of the same search reuse one +/// pool checkout. pub(crate) async fn fetch_all( pool: &PgPool, + mut conn: PoolConnection, query: Query<'_, Postgres, PgArguments>, ) -> Result, sqlx::Error> { - let mut conn = pool.acquire().await?; let backend: (i32, i64) = sqlx::query_as(BACKEND_SQL).fetch_one(&mut *conn).await?; let mut armed = CancelOnDrop { pool: pool.clone(), diff --git a/crates/buzz-search/src/query.rs b/crates/buzz-search/src/query.rs index facd9a7f37..07ecf29277 100644 --- a/crates/buzz-search/src/query.rs +++ b/crates/buzz-search/src/query.rs @@ -62,17 +62,20 @@ pub enum SearchMode { /// Prefix-match the trailing normalized query token (`pro` matches `project`). /// /// Intended for bounded typeahead surfaces such as the desktop topbar. The - /// relay still refetches and re-authorizes every hit; this mode changes only - /// the candidate tsquery, not the access boundary. A trailing token that - /// yields a lexeme shorter than three characters returns no hits unless the - /// query is kind:0-only (see `PREFIX_MIN_CHARS`). + /// relay still refetches and re-authorizes every hit; this mode changes the + /// candidate tsquery and, for relevance outside profile search, ranks only + /// the newest 1000 matches. It does not change the access boundary. A + /// trailing token that yields a lexeme shorter than three characters + /// returns no hits unless the query is kind:0-only (see `PREFIX_MIN_CHARS`). Prefix, } /// Result ordering for a search. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SearchSort { - /// `ts_rank_cd` relevance, newest first among equal ranks. + /// `ts_rank_cd` relevance, newest first among equal ranks. In + /// [`SearchMode::Prefix`] outside profile search, only the newest 1000 + /// matches are ranked, so pages end with them. Relevance, /// Newest first, ignoring relevance; ties ordered by event id. Newest, @@ -151,6 +154,13 @@ const SEARCH_TEXT_MAX_CHARS: usize = 4096; /// bridge uses page 1), but clamp here too so a future caller cannot accidentally /// wire untrusted input into a multi-trillion-row OFFSET. const PAGE_MAX: u32 = 1000; +/// How many of the newest matches a prefix-mode relevance search ranks. +/// The GIN index returns matches in no useful order, so ranking every match +/// before `LIMIT` costs time proportional to the match count, and a short +/// typeahead prefix matches most of a community. Reading candidates newest +/// first lets Postgres stop at this many instead. Every page ranks the same +/// window, so pages stay consistent and end with it. +const PREFIX_RANK_WINDOW: i64 = 1000; /// Shortest lexeme [`SearchMode::Prefix`] gives `:*` for anything but a /// kind:0-only (profile) query. If the trailing token yields a shorter one, /// the tsquery is NULL and the search returns no hits, whatever completed @@ -236,15 +246,18 @@ fn normalized_search_text(q: &str) -> Option { /// Execute a community-scoped FTS query. /// -/// SQL shape (always): +/// The mode-specific tsquery is resolved first and bound as a value, so the +/// planner estimates its selectivity instead of assuming a rare match. The +/// search itself is a one-shot statement, planned for that value every time. +/// +/// SQL shape: /// ```sql /// SELECT id, kind, pubkey, channel_id, EXTRACT(EPOCH FROM created_at)::bigint AS created_at_s, -/// ts_rank_cd(search_tsv, query) AS rank -/// FROM events, -/// AS query +/// ts_rank_cd(search_tsv, $q) AS rank +/// FROM events -- prefix relevance: the newest PREFIX_RANK_WINDOW matches /// WHERE community_id = $ctx /// AND deleted_at IS NULL -/// AND search_tsv @@ query +/// AND search_tsv @@ $q /// [+ channel scope, kinds, authors, since, until] /// ORDER BY rank DESC, created_at DESC, id -- SearchSort::Newest: created_at DESC, id /// LIMIT $per_page OFFSET (($page - 1) * $per_page) @@ -279,18 +292,42 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result = QueryBuilder::new("SELECT ("); + push_tsquery(&mut resolve, query.mode, &search_text, min_prefix_chars); + resolve.push(")::text"); + let tsquery: Option = resolve.build_query_scalar().fetch_one(&mut *conn).await?; + let Some(tsquery) = tsquery.filter(|tsquery| !tsquery.is_empty()) else { + // A too-short trailing prefix (NULL) or nothing searchable after + // normalization (empty) matches nothing. + return Ok(SearchResult { + hits: Vec::new(), + page, + }); + }; let mut qb: QueryBuilder = QueryBuilder::new( "SELECT id, kind, pubkey, channel_id, \ EXTRACT(EPOCH FROM created_at)::bigint AS created_at_s, \ - ts_rank_cd(search_tsv, search_query.query) AS rank \ - FROM events CROSS JOIN LATERAL (SELECT ", + ts_rank_cd(search_tsv, ", ); - let min_prefix_chars = if profile_only { 0 } else { PREFIX_MIN_CHARS }; - push_tsquery(&mut qb, query.mode, &search_text, min_prefix_chars); - qb.push(" AS query) AS search_query WHERE community_id = "); + qb.push_bind(&tsquery); + qb.push("::tsquery) AS rank FROM "); + if rank_window.is_some() { + qb.push("(SELECT id, kind, pubkey, channel_id, created_at, search_tsv FROM "); + } + qb.push("events WHERE community_id = "); qb.push_bind(*query.community.as_uuid()); - qb.push(" AND deleted_at IS NULL AND search_tsv @@ search_query.query"); + qb.push(" AND deleted_at IS NULL AND search_tsv @@ "); + qb.push_bind(&tsquery); + qb.push("::tsquery"); // Search is a current-state surface; old artifact snapshots cannot match. qb.push(" AND (events.kind <> 45010 OR EXISTS (SELECT 1 FROM artifact_heads ah WHERE ah.community_id=events.community_id AND ah.event_id=events.id AND NOT ah.deleted))"); @@ -346,6 +383,14 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result Result = row.try_get("id")?; diff --git a/crates/buzz-search/tests/postgres_fts_integration.rs b/crates/buzz-search/tests/postgres_fts_integration.rs index d79e240764..b68cf1a41f 100644 --- a/crates/buzz-search/tests/postgres_fts_integration.rs +++ b/crates/buzz-search/tests/postgres_fts_integration.rs @@ -745,6 +745,101 @@ async fn prefix_mode_preserves_storage_level_privacy_exclusions() { teardown(pool, &schema).await; } +/// Prefix relevance (typeahead) ranks only the newest 1000 matches, so an +/// older, better-ranked message drops out, and its pages end with that +/// window; full-text and profile relevance still rank every match. +/// Mutation: rank every prefix match → the old message tops the first page; +/// window profiles too → the old profile drops off the first page. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn prefix_relevance_ranks_only_the_newest_matches() { + let (pool, schema) = setup().await; + let c = mk_community(&pool, "rank-window.example").await; + let (old, old_profile) = (rand_bytes32(), rand_bytes32()); + for (id, kind) in [(old, 9), (old_profile, 0)] { + insert_event( + &pool, + c, + id, + rand_bytes32(), + kind, + "planning planning planning planning", + None, + 1_700_000_000, + ) + .await; + // 1000 newer, weaker matches fill the window. + sqlx::query( + "INSERT INTO events (community_id, id, pubkey, created_at, kind, tags, content, sig) \ + SELECT $1, sha256((g || '/' || $2)::bytea), sha256((g || '/' || $2)::bytea), \ + to_timestamp(1700000001 + g), $2, '[]'::jsonb, 'planning notes ' || g, 'signature' \ + FROM generate_series(1, 1000) g", + ) + .bind(c.as_uuid()) + .bind(kind) + .execute(&pool) + .await + .expect("insert newer matches"); + } + + let svc = SearchService::new(pool.clone()); + let query = |q: &str, mode, page| SearchQuery { + community: c, + q: q.into(), + channel_scope: ChannelScope::Any, + kinds: Some(vec![9]), + authors: None, + since: None, + until: None, + page, + per_page: 20, + mode, + sort: buzz_search::SearchSort::Relevance, + }; + let ids = |r: buzz_search::SearchResult| r.hits.iter().map(|h| h.event_id).collect::>(); + + let typeahead = ids(svc + .search(&query("plan", buzz_search::SearchMode::Prefix, 1)) + .await + .expect("prefix")); + assert_eq!(typeahead.len(), 20); + assert!(!typeahead.contains(&old), "outside the newest-1000 window"); + + let full_text = ids(svc + .search(&query("planning", buzz_search::SearchMode::FullText, 1)) + .await + .expect("full text")); + assert_eq!(full_text.first(), Some(&old), "full text ranks every match"); + + // Every page ranks the same window, so prefix pages end with it. + let last = ids(svc + .search(&query("plan", buzz_search::SearchMode::Prefix, 50)) + .await + .expect("page 50")); + assert_eq!(last.len(), 20); + assert!(!last.contains(&old)); + let past = svc + .search(&query("plan", buzz_search::SearchMode::Prefix, 51)) + .await + .expect("page 51"); + assert!(past.hits.is_empty(), "no page past the window"); + + let profiles = ids(svc + .search(&SearchQuery { + kinds: Some(vec![0]), + ..query("plan", buzz_search::SearchMode::Prefix, 1) + }) + .await + .expect("profile prefix")); + assert_eq!( + profiles.first(), + Some(&old_profile), + "profile typeahead ranks every match" + ); + + teardown(pool, &schema).await; +} + #[tokio::test] #[ignore = "requires Postgres"] async fn channel_scope_restricts_results() { @@ -1990,7 +2085,7 @@ async fn waiting_searches(pool: &PgPool) -> i64 { sqlx::query_scalar( "SELECT count(*) FROM pg_stat_activity WHERE pid <> pg_backend_pid() \ AND datname = current_database() \ - AND wait_event_type = 'Lock' AND query LIKE '%search_query.query%'", + AND wait_event_type = 'Lock' AND query LIKE '%ts_rank_cd(%'", ) .fetch_one(pool) .await