mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(search): normalize profile name tiers in one place (#8334)
🤖 ## Summary Profile name search (`POST /profiles/search`, added in #8301) sorts results into tiers: an exact name first, then a name that starts with the query, then a name with a later word that starts with it. This PR stacks on #8301 and fixes two review items. **Tier normalization in one place.** Before this change, the query was lower-cased in Rust (`to_lowercase`), and the names were lower-cased in SQL (`lower()`). These can disagree. Under a `C` collation, PostgreSQL's `lower()` changes only ASCII letters, while Rust also lower-cases letters like `É`. So a search for `Émile` did not put the profile named `Émile` in the exact tier. Now the query and both name fields go through the same SQL expression: lower-case, collapse whitespace runs to one space, then trim. The query is bound once and both sides compare equal by construction. The trim also moved after the collapse, so surrounding whitespace that the regex turns into a space is now trimmed too. **Ranking wording.** The doc comment and `docs/profile-search.md` said "ties keep the NIP-50 order". That was wrong. Ties are ordered by `ts_rank_cd` over the two name fields only (not the whole profile, as in NIP-50), then newest first, then event id. Out of scope, not changed here: matching names that contain escaped Unicode or newlines, and any new name index. ## Related issue Review on #8301: https://github.com/block/buzz/pull/8301#pullrequestreview-5524491230 ## Testing - New case in `profile_names_match_name_words_only`: profiles `Émile` and a newer `Émilee`, query `Émile`. The exact match must come first. - Ran the Postgres integration tests against a `postgres:17-alpine` database created with `--locale=C`: - this branch: `profile_names_match_name_words_only` and `profile_names_survive_unparsable_content` pass; - #8301 code with the new case: `profile_names_match_name_words_only` fails (`Émilee` sorts before `Émile`). - Nine other tests in that file fail locally on both this branch and #8301 because they reuse one database (duplicate community hosts). CI gives each test its own database. - `cargo fmt --check`, `cargo clippy -p buzz-search --all-targets -D warnings`, and `cargo test -p buzz-search`: clean. - The pre-push hook's `rust-tests` and `desktop-tauri-checks` passed. Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -356,6 +356,14 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result<SearchResult,
|
|||||||
Ok(SearchResult { hits, page })
|
Ok(SearchResult { hits, page })
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The SQL that normalizes a name, and the query, for result tiers, around
|
||||||
|
/// its argument: lower-cased, whitespace runs collapsed to one space, trimmed.
|
||||||
|
const TIER_TEXT: [&str; 2] = ["btrim(regexp_replace(lower(", "), '\\s+', ' ', 'g'))"];
|
||||||
|
|
||||||
|
fn tier_name(column: &str) -> String {
|
||||||
|
format!("{}{column}{}", TIER_TEXT[0], TIER_TEXT[1])
|
||||||
|
}
|
||||||
|
|
||||||
/// Largest page size [`search_profile_names`] accepts.
|
/// Largest page size [`search_profile_names`] accepts.
|
||||||
pub const PROFILE_NAMES_PER_PAGE_MAX: u32 = 100;
|
pub const PROFILE_NAMES_PER_PAGE_MAX: u32 = 100;
|
||||||
|
|
||||||
@@ -369,9 +377,12 @@ pub const PROFILE_NAMES_PER_PAGE_MAX: u32 = 100;
|
|||||||
/// names; the existing `search_tsv` index stays the access path.
|
/// names; the existing `search_tsv` index stays the access path.
|
||||||
///
|
///
|
||||||
/// Order: a name equal to the query, then a name that starts with it, then a
|
/// Order: a name equal to the query, then a name that starts with it, then a
|
||||||
/// name with a word that starts with it, then any other match. Ties keep the
|
/// name with a word that starts with it, then any other match. The query and
|
||||||
/// NIP-50 order (rank, newest first, id), so pages are stable. Pages are
|
/// names are compared lower-cased with whitespace collapsed, both normalized
|
||||||
/// 1-based, like [`SearchQuery::page`].
|
/// by the same SQL. Ties go by relevance of the query to the two names
|
||||||
|
/// (`ts_rank_cd` over the names alone, not the whole profile), then newest
|
||||||
|
/// first, then event id, so pages are stable. Pages are 1-based, like
|
||||||
|
/// [`SearchQuery::page`].
|
||||||
#[datastore_span(name = "search_profile_names", system = "postgresql")]
|
#[datastore_span(name = "search_profile_names", system = "postgresql")]
|
||||||
pub async fn search_profile_names(
|
pub async fn search_profile_names(
|
||||||
pool: &PgPool,
|
pool: &PgPool,
|
||||||
@@ -389,14 +400,8 @@ pub async fn search_profile_names(
|
|||||||
};
|
};
|
||||||
let per_page = per_page.clamp(1, PROFILE_NAMES_PER_PAGE_MAX);
|
let per_page = per_page.clamp(1, PROFILE_NAMES_PER_PAGE_MAX);
|
||||||
let offset = ((page - 1) as i64) * (per_page as i64);
|
let offset = ((page - 1) as i64) * (per_page as i64);
|
||||||
// Tiers compare the query as typed, with whitespace runs collapsed, to
|
// Tiers compare the query and each name after the same SQL normalization
|
||||||
// each lower-cased name (spaces collapsed too).
|
// ([`TIER_TEXT`]), so the two sides agree by construction.
|
||||||
let typed = search_text
|
|
||||||
.split_whitespace()
|
|
||||||
.collect::<Vec<_>>()
|
|
||||||
.join(" ")
|
|
||||||
.to_lowercase();
|
|
||||||
|
|
||||||
let mut qb: QueryBuilder<sqlx::Postgres> = QueryBuilder::new(
|
let mut qb: QueryBuilder<sqlx::Postgres> = QueryBuilder::new(
|
||||||
"SELECT id, kind, pubkey, channel_id, created_at_s, rank FROM (\
|
"SELECT id, kind, pubkey, channel_id, created_at_s, rank FROM (\
|
||||||
SELECT e.id, e.kind, e.pubkey, e.channel_id, \
|
SELECT e.id, e.kind, e.pubkey, e.channel_id, \
|
||||||
@@ -405,13 +410,11 @@ pub async fn search_profile_names(
|
|||||||
LEAST(",
|
LEAST(",
|
||||||
);
|
);
|
||||||
for field in ["names.name", "names.display_name"] {
|
for field in ["names.name", "names.display_name"] {
|
||||||
qb.push(format!("CASE WHEN {field} = "));
|
qb.push(format!(
|
||||||
qb.push_bind(typed.clone());
|
"CASE WHEN {field} = search_query.typed THEN 0 \
|
||||||
qb.push(format!(" THEN 0 WHEN starts_with({field}, "));
|
WHEN starts_with({field}, search_query.typed) THEN 1 \
|
||||||
qb.push_bind(typed.clone());
|
WHEN strpos({field}, ' ' || search_query.typed) > 0 THEN 2 ELSE 3 END"
|
||||||
qb.push(format!(") THEN 1 WHEN strpos({field}, ' ' || "));
|
));
|
||||||
qb.push_bind(typed.clone());
|
|
||||||
qb.push(") > 0 THEN 2 ELSE 3 END");
|
|
||||||
if field == "names.name" {
|
if field == "names.name" {
|
||||||
qb.push(", ");
|
qb.push(", ");
|
||||||
}
|
}
|
||||||
@@ -421,15 +424,20 @@ pub async fn search_profile_names(
|
|||||||
FROM events e CROSS JOIN LATERAL (SELECT ",
|
FROM events e CROSS JOIN LATERAL (SELECT ",
|
||||||
);
|
);
|
||||||
push_tsquery(&mut qb, SearchMode::Prefix, &search_text);
|
push_tsquery(&mut qb, SearchMode::Prefix, &search_text);
|
||||||
qb.push(
|
qb.push(" AS query, ");
|
||||||
" AS query) AS search_query \
|
qb.push(TIER_TEXT[0]);
|
||||||
|
qb.push_bind(search_text.clone());
|
||||||
|
qb.push(TIER_TEXT[1]);
|
||||||
|
qb.push(format!(
|
||||||
|
" AS typed) AS search_query \
|
||||||
CROSS JOIN LATERAL profile_name_fields(e.content) AS fields \
|
CROSS JOIN LATERAL profile_name_fields(e.content) AS fields \
|
||||||
CROSS JOIN LATERAL (SELECT \
|
CROSS JOIN LATERAL (SELECT \
|
||||||
to_tsvector('simple', concat_ws(' ', fields.name, fields.display_name)) AS tsv, \
|
to_tsvector('simple', concat_ws(' ', fields.name, fields.display_name)) AS tsv, \
|
||||||
regexp_replace(lower(btrim(fields.name)), '\\s+', ' ', 'g') AS name, \
|
{} AS name, {} AS display_name) AS names \
|
||||||
regexp_replace(lower(btrim(fields.display_name)), '\\s+', ' ', 'g') AS display_name) AS names \
|
|
||||||
WHERE e.community_id = ",
|
WHERE e.community_id = ",
|
||||||
);
|
tier_name("fields.name"),
|
||||||
|
tier_name("fields.display_name"),
|
||||||
|
));
|
||||||
qb.push_bind(*community.as_uuid());
|
qb.push_bind(*community.as_uuid());
|
||||||
qb.push(
|
qb.push(
|
||||||
" AND e.kind = 0 AND e.channel_id IS NULL AND e.deleted_at IS NULL \
|
" AND e.kind = 0 AND e.channel_id IS NULL AND e.deleted_at IS NULL \
|
||||||
|
|||||||
@@ -1730,6 +1730,15 @@ async fn profile_names_match_name_words_only() {
|
|||||||
assert!(profile_names(&pool, c, "%", 1, 100).await.is_empty());
|
assert!(profile_names(&pool, c, "%", 1, 100).await.is_empty());
|
||||||
assert!(profile_names(&pool, c, " ", 1, 100).await.is_empty());
|
assert!(profile_names(&pool, c, " ", 1, 100).await.is_empty());
|
||||||
|
|
||||||
|
// The query and the names are normalized by the same SQL, so a non-ASCII
|
||||||
|
// exact name is tier 0 whatever the database collation lower-cases.
|
||||||
|
let emile = put_profile(&pool, c, r#"{"name":"Émile"}"#, 1_700_000_200).await;
|
||||||
|
let emilee = put_profile(&pool, c, r#"{"name":"Émilee"}"#, 1_700_000_210).await;
|
||||||
|
assert_eq!(
|
||||||
|
profile_names(&pool, c, "Émile", 1, 100).await,
|
||||||
|
vec![emile, emilee]
|
||||||
|
);
|
||||||
|
|
||||||
teardown(pool, &schema).await;
|
teardown(pool, &schema).await;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -42,8 +42,11 @@ Results are ordered:
|
|||||||
3. a name with a later word that starts with the query;
|
3. a name with a later word that starts with the query;
|
||||||
4. any other match.
|
4. any other match.
|
||||||
|
|
||||||
Ties keep the NIP-50 order (relevance, newest first, event id), so pages do
|
For these tiers the query and both names are lower-cased, with whitespace
|
||||||
not overlap. Profile content that is not a JSON object, or a name field that
|
runs collapsed to one space and trimmed, by the same database expression.
|
||||||
|
Ties go by relevance of the query to the two names alone (not the whole
|
||||||
|
profile, as in NIP-50), then newest first, then event id, so pages do not
|
||||||
|
overlap. Profile content that is not a JSON object, or a name field that
|
||||||
is not a string, never matches. Content that PostgreSQL cannot read as `jsonb`
|
is not a string, never matches. Content that PostgreSQL cannot read as `jsonb`
|
||||||
(for example a `\u0000` escape or very deep nesting) has no names, so it
|
(for example a `\u0000` escape or very deep nesting) has no names, so it
|
||||||
never matches and never makes the search fail
|
never matches and never makes the search fail
|
||||||
|
|||||||
Reference in New Issue
Block a user