diff --git a/apps/backend/services/library-search.service.ts b/apps/backend/services/library-search.service.ts index 5737f810..69f830d1 100644 --- a/apps/backend/services/library-search.service.ts +++ b/apps/backend/services/library-search.service.ts @@ -67,6 +67,24 @@ const SECONDARY_SORT: Record = { date: sql`${library_artist_view.artist_name}`, }; +// Bare-column variants for use in an outer ORDER BY after a UNION ALL — once +// the union closes, the outer query sees the combined column set by SELECT +// alias, not by underlying table. The qualified `library_artist_view.col` +// reference in SORT_COLUMNS / SECONDARY_SORT wouldn't resolve at that level. +const SORT_COLUMNS_UNQUALIFIED: Record = { + artist: sql`artist_name`, + album: sql`album_title`, + plays: sql`plays`, + date: sql`add_date`, +}; + +const SECONDARY_SORT_UNQUALIFIED: Record = { + artist: sql`album_title`, + album: sql`artist_name`, + plays: sql`artist_name`, + date: sql`artist_name`, +}; + export const MIN_CASCADE_QUERY_LENGTH = 4; export const MAX_CASCADE_CONDITIONS = 6; @@ -96,52 +114,64 @@ export async function searchLibrary( await validateEnumFilters(params.genre, params.format); const conditions = parseSearchQuery(params.q, CATALOG_PARSER_CONFIG); - // Alias is keyed on the raw `q` (matched as a single string by the LATERAL). - // Read once per request so a `getConfig()` invalidation mid-call doesn't - // give the SELECT projection and the WHERE predicate different views of - // the flag — alias rows would surface in the SELECT but get re-filtered - // out by the WHERE. + // Alias is keyed on the raw `q` (matched as a single string in the + // alias_hits CTE). Read once per request so a `getConfig()` invalidation + // mid-call doesn't give the SELECT projection and the WHERE predicate + // different views of the flag — alias rows would surface in the SELECT + // but get re-filtered out by the WHERE. // - // Also gate on `hasAllField`: only `field === 'all'` conditions ever inject - // the alias OR predicate via buildAllFieldMatch. A pure field-specific - // query (`artist:foo`, `album:bar`) would otherwise still pay the LATERAL - // cost per candidate row with no chance of an alias-only hit surviving - // WHERE. The result set is identical to the flag-off path in that case, so - // skip the join entirely. + // Also gate on `hasAllField`: alias substrate matching is only routed + // for queries that include at least one `field === 'all'` condition. + // Field-specific queries (`artist:foo`, `album:bar`) stay on the legacy + // single-SELECT plan — they're narrow searches where alias expansion + // would surface canonical-name-mismatched rows that the user explicitly + // scoped against. Preserves pre-#1318 behavior for those paths; opening + // alias expansion to field-specific queries is a future product call. const hasAllFieldCondition = conditions.some((c) => c.field === 'all'); const aliasActive = getCatalogSearchAliasConfig().enabled && params.q.trim().length > 0 && hasAllFieldCondition; - const queryWhere = buildWhereClause(conditions, aliasActive); const filterWhere = buildFilterClause(params); - const where = combineWhere(queryWhere, filterWhere); - const orderDirection = params.order === 'asc' ? sql`ASC` : sql`DESC`; - const orderBy = sql`${SORT_COLUMNS[params.sort]} ${orderDirection}, ${SECONDARY_SORT[params.sort]} ASC, ${library_artist_view.id} ASC`; - const offset = params.page * params.limit; - const aliasJoin = aliasActive - ? sql`LEFT JOIN LATERAL ( - SELECT MAX(similarity(asa.variant, ${params.q})) AS max_sim, - (array_agg(asa.variant ORDER BY similarity(asa.variant, ${params.q}) DESC))[1] AS matched_variant, - (array_agg(asa.source ORDER BY similarity(asa.variant, ${params.q}) DESC))[1] AS matched_source - FROM ${artist_search_alias} asa - WHERE asa.artist_id = ${library_artist_view.artist_id} - AND asa.variant % ${params.q} - ) alias_hit ON true` - : sql``; - const fromClause = where - ? sql`FROM ${library_artist_view} ${aliasJoin} WHERE ${where}` - : sql`FROM ${library_artist_view} ${aliasJoin}`; - - const aliasProjection = aliasActive - ? sql`, - alias_hit.max_sim AS alias_max_sim, - alias_hit.matched_variant AS alias_matched_variant, - alias_hit.matched_source AS alias_matched_source` - : sql``; - - const dataQuery = sql` - SELECT + + let dataQuery: SQL; + let countQuery: SQL; + if (aliasActive) { + // ALT1 UNION ALL (BS#1318). The CTE runs the trigram bitmap scan over + // `artist_search_alias` once, then we emit two branches: + // (a) byte-identical to the alias-OFF path — query conditions + // evaluated WITHOUT the alias-OR — so the planner can pick the + // same per-column GIN trigram / ILIKE plan it picks today, and + // LIMIT pushdown stays intact. + // (b) alias-only hits, INNER JOIN'd to alias_hits on artist_id, with + // a dedupe predicate that excludes anything already in (a). + // + // The (a)-shaped WHERE is referenced by reference equality in (b)'s + // dedupe so the two branches can't drift on what counts as a match. + const queryWhereAliasOff = buildWhereClause(conditions); + const branchAWhere = combineWhere(queryWhereAliasOff, filterWhere); + // Branch (b): the row satisfies `filterWhere` AND its artist_id has an + // alias hit AND the row would NOT have matched branch (a). When + // queryWhereAliasOff is null (defensive — aliasActive gates on + // hasAllFieldCondition so this is unreachable today), the NOT becomes + // vacuously false and (b) emits nothing, which is what we want. + const dedupeWhere = queryWhereAliasOff ? sql`NOT ${queryWhereAliasOff}` : sql`FALSE`; + const branchBWhere = combineWhere(dedupeWhere, filterWhere); + + const orderBy = sql`${SORT_COLUMNS_UNQUALIFIED[params.sort]} ${orderDirection}, ${SECONDARY_SORT_UNQUALIFIED[params.sort]} ASC, id ASC`; + + const cte = sql`WITH alias_hits AS ( + SELECT + asa.artist_id, + MAX(similarity(asa.variant, ${params.q})) AS max_sim, + (array_agg(asa.variant ORDER BY similarity(asa.variant, ${params.q}) DESC))[1] AS matched_variant, + (array_agg(asa.source ORDER BY similarity(asa.variant, ${params.q}) DESC))[1] AS matched_source + FROM ${artist_search_alias} asa + WHERE asa.variant % ${params.q} + GROUP BY asa.artist_id + )`; + + const branchAProjection = sql` ${library_artist_view.id} AS id, ${library_artist_view.add_date} AS add_date, ${library_artist_view.album_title} AS album_title, @@ -156,13 +186,87 @@ export async function searchLibrary( ${library_artist_view.rotation_bin} AS rotation_bin, ${library_artist_view.plays} AS plays, ${library_artist_view.on_streaming} AS on_streaming, - ${library_artist_view.album_artist} AS album_artist - ${aliasProjection} - ${fromClause} - ORDER BY ${orderBy} - LIMIT ${params.limit} OFFSET ${offset} - `; - const countQuery = sql`SELECT COUNT(*)::int AS total ${fromClause}`; + ${library_artist_view.album_artist} AS album_artist, + NULL::real AS alias_max_sim, + NULL::text AS alias_matched_variant, + NULL::text AS alias_matched_source`; + const branchBProjection = sql` + ${library_artist_view.id} AS id, + ${library_artist_view.add_date} AS add_date, + ${library_artist_view.album_title} AS album_title, + ${library_artist_view.artist_name} AS artist_name, + ${library_artist_view.code_letters} AS code_letters, + ${library_artist_view.code_number} AS code_number, + ${library_artist_view.code_artist_number} AS code_artist_number, + ${library_artist_view.format_name} AS format_name, + ${library_artist_view.genre_name} AS genre_name, + ${library_artist_view.label} AS label, + ${library_artist_view.label_id} AS label_id, + ${library_artist_view.rotation_bin} AS rotation_bin, + ${library_artist_view.plays} AS plays, + ${library_artist_view.on_streaming} AS on_streaming, + ${library_artist_view.album_artist} AS album_artist, + alias_hits.max_sim AS alias_max_sim, + alias_hits.matched_variant AS alias_matched_variant, + alias_hits.matched_source AS alias_matched_source`; + + const branchAFrom = branchAWhere + ? sql`FROM ${library_artist_view} WHERE ${branchAWhere}` + : sql`FROM ${library_artist_view}`; + const branchBFrom = branchBWhere + ? sql`FROM ${library_artist_view} INNER JOIN alias_hits ON alias_hits.artist_id = ${library_artist_view.artist_id} WHERE ${branchBWhere}` + : sql`FROM ${library_artist_view} INNER JOIN alias_hits ON alias_hits.artist_id = ${library_artist_view.artist_id}`; + + const unionBody = sql`( + SELECT ${branchAProjection} + ${branchAFrom} + ) + UNION ALL + ( + SELECT ${branchBProjection} + ${branchBFrom} + )`; + + dataQuery = sql` + ${cte} + ${unionBody} + ORDER BY ${orderBy} + LIMIT ${params.limit} OFFSET ${offset} + `; + countQuery = sql` + ${cte} + SELECT COUNT(*)::int AS total FROM (${unionBody}) alias_search + `; + } else { + // Alias OFF (legacy path). Single SELECT against library_artist_view, + // no CTE, no UNION ALL — byte-identical to pre-#1318 behavior. + const queryWhere = buildWhereClause(conditions); + const where = combineWhere(queryWhere, filterWhere); + const fromClause = where ? sql`FROM ${library_artist_view} WHERE ${where}` : sql`FROM ${library_artist_view}`; + const orderBy = sql`${SORT_COLUMNS[params.sort]} ${orderDirection}, ${SECONDARY_SORT[params.sort]} ASC, ${library_artist_view.id} ASC`; + dataQuery = sql` + SELECT + ${library_artist_view.id} AS id, + ${library_artist_view.add_date} AS add_date, + ${library_artist_view.album_title} AS album_title, + ${library_artist_view.artist_name} AS artist_name, + ${library_artist_view.code_letters} AS code_letters, + ${library_artist_view.code_number} AS code_number, + ${library_artist_view.code_artist_number} AS code_artist_number, + ${library_artist_view.format_name} AS format_name, + ${library_artist_view.genre_name} AS genre_name, + ${library_artist_view.label} AS label, + ${library_artist_view.label_id} AS label_id, + ${library_artist_view.rotation_bin} AS rotation_bin, + ${library_artist_view.plays} AS plays, + ${library_artist_view.on_streaming} AS on_streaming, + ${library_artist_view.album_artist} AS album_artist + ${fromClause} + ORDER BY ${orderBy} + LIMIT ${params.limit} OFFSET ${offset} + `; + countQuery = sql`SELECT COUNT(*)::int AS total ${fromClause}`; + } const [dataRows, countRows] = await Promise.all([db.execute(dataQuery), db.execute(countQuery)]); @@ -322,11 +426,11 @@ function toAlbumSearchResultRow(row: RawRow): AlbumSearchResultRow { return projected; } -function buildWhereClause(conditions: SearchCondition[], aliasActive: boolean): SQL | null { +function buildWhereClause(conditions: SearchCondition[]): SQL | null { if (conditions.length === 0) return null; const fragments = conditions - .map((c) => ({ operator: c.operator, fragment: buildConditionFragment(c, aliasActive) })) + .map((c) => ({ operator: c.operator, fragment: buildConditionFragment(c) })) .filter((f): f is { operator: 'AND' | 'OR'; fragment: SQL } => f.fragment !== null); if (fragments.length === 0) return null; @@ -339,10 +443,9 @@ function buildWhereClause(conditions: SearchCondition[], aliasActi return sql`(${result})`; } -function buildConditionFragment(condition: SearchCondition, aliasActive: boolean): SQL | null { +function buildConditionFragment(condition: SearchCondition): SQL | null { const { field, value, exact, negated } = condition; - const fragment = - field === 'all' ? buildAllFieldMatch(value, exact, aliasActive) : buildColumnMatch(field, value, exact); + const fragment = field === 'all' ? buildAllFieldMatch(value, exact) : buildColumnMatch(field, value, exact); return negated ? sql`NOT (${fragment})` : fragment; } @@ -354,7 +457,7 @@ function buildColumnMatch(field: CatalogField, value: string, exact: boolean): S return sql`${col} ILIKE ${'%' + value + '%'}`; } -function buildAllFieldMatch(value: string, exact: boolean, aliasActive: boolean): SQL { +function buildAllFieldMatch(value: string, exact: boolean): SQL { if (exact) { // Exact matching skips the alias path — alias variants are normalized // strings, not exact matches against the canonical name. @@ -364,12 +467,13 @@ function buildAllFieldMatch(value: string, exact: boolean, aliasActive: boolean) // deferred follow-up flagged in the plan (add `label` to library.search_doc // first, then route this branch through it); v1 stays correct and reviewable // with the same path the flowsheet trigram fallback uses. + // + // Alias substrate matching does not route through here — under the + // UNION ALL design (BS#1318) alias-only hits surface from branch (b)'s + // INNER JOIN against the `alias_hits` CTE, not from an OR predicate + // injected into this fragment. const pattern = '%' + value + '%'; - // When alias is active the LATERAL JOIN in the FROM clause exposes - // `alias_hit.max_sim`; OR it into the all-field branch so an alias-only - // hit (variant trigram match but no canonical-name match) still surfaces. - const aliasOr = aliasActive ? sql` OR alias_hit.max_sim IS NOT NULL` : sql``; - return sql`(${library_artist_view.artist_name} ILIKE ${pattern} OR ${library_artist_view.album_title} ILIKE ${pattern} OR ${library_artist_view.label} ILIKE ${pattern}${aliasOr})`; + return sql`(${library_artist_view.artist_name} ILIKE ${pattern} OR ${library_artist_view.album_title} ILIKE ${pattern} OR ${library_artist_view.label} ILIKE ${pattern})`; } function buildFilterClause(params: LibraryQueryParams): SQL | null { diff --git a/apps/backend/services/library.service.ts b/apps/backend/services/library.service.ts index 36781df8..f0f86d5a 100644 --- a/apps/backend/services/library.service.ts +++ b/apps/backend/services/library.service.ts @@ -67,10 +67,10 @@ type ReconciledIdentityKey = (typeof RECONCILED_IDENTITY_KEYS)[number]; * A library_artist_view row that may carry an attached `matched_via` hint when * the cascade's CTA or LML `/lookup` fallback surfaced it (catalog-track-search * plan §5.1), or a `matched_via_alias` hint when the artist-search-alias - * LATERAL JOIN surfaced it (artist-search-alias plan §PR 5). Wraps - * `LibraryArtistViewEntry` rather than replacing it so downstream functions - * (enrichWithArtwork, serializeReconciledIdentity) accept tagged rows without - * signature changes. + * UNION ALL branch surfaced it (artist-search-alias plan §PR 5, post-#1318). + * Wraps `LibraryArtistViewEntry` rather than replacing it so downstream + * functions (enrichWithArtwork, serializeReconciledIdentity) accept tagged + * rows without signature changes. */ export type TaggedLibraryViewEntry = LibraryArtistViewEntry & { matched_via?: TrackMatchHint[]; @@ -78,9 +78,9 @@ export type TaggedLibraryViewEntry = LibraryArtistViewEntry & { }; /** - * Raw projection fields appended to the SELECT when the alias LATERAL is on. - * Returned as nullable columns so callers can detect a no-hit row by - * `alias_max_sim === null` without forcing a separate query. + * Raw projection fields appended to the SELECT when the alias UNION ALL + * branch is on (BS#1318). Returned as nullable columns so callers can detect + * a no-hit row by `alias_max_sim === null` without forcing a separate query. */ type AliasHitFields = { alias_max_sim: number | null; @@ -89,16 +89,16 @@ type AliasHitFields = { }; /** - * Attach `matched_via_alias` to a tagged row when the LATERAL JOIN surfaced a - * non-null hit. No-op when all three alias fields are null (the row matched - * via the underlying trigram predicate, not via the alias cache). + * Attach `matched_via_alias` to a tagged row when the alias UNION ALL branch + * surfaced a non-null hit. No-op when all three alias fields are null (the + * row matched via the underlying trigram predicate, not via the alias cache). */ function attachAliasHint(row: R): TaggedLibraryViewEntry { // Strip the alias-only fields off the wire-shape; matched_via_alias carries // the same information in the public sibling-type shape. const { alias_max_sim, alias_matched_variant, alias_matched_source, ...rest } = row; // Defensive: require non-nullish numeric and truthy variant + source. The - // LATERAL JOIN should never emit empty-string fields, but mirror the same + // alias branch should never emit empty-string fields, but mirror the same // guard `toAlbumSearchResultRow` uses in library-search.service so the // catalog (/library/query) and request-line surfaces agree on what counts // as an alias hit. @@ -970,8 +970,8 @@ const LIBRARY_VIEW_PROJECTION = { /** * Raw SQL mirror of `libraryViewQuery(false)`'s join chain. Used when the * caller needs a query shape Drizzle's chained builder can't express - * (`LEFT JOIN LATERAL` for the alias path). Schema is named once here so a - * future column change is a single edit. + * (the UNION ALL alias path emitted by `buildAliasHitsCte`). Schema is + * named once here so a future column change is a single edit. */ const LIBRARY_VIEW_JOINS_RAW = sql` INNER JOIN ${artists} ON ${artists.id} = ${library.artist_id} @@ -1014,33 +1014,44 @@ const LIBRARY_VIEW_PROJECTION_RAW = sql` `; /** - * The alias LATERAL JOIN fragment plus its associated SELECT projection, - * WHERE-list contribution, and ORDER BY ranking term. Keyed on - * `library.artist_id` (the catalog read paths all project it through their - * underlying `FROM library` joins). Multiple cached variants for the same - * artist collapse to a single row via MAX/array_agg ORDER BY similarity - * (the highest-similarity variant + its source win). + * Build the `alias_hits` CTE used by the UNION ALL alias-aware search paths + * (BS#1318). The CTE runs the trigram bitmap scan over `artist_search_alias` + * exactly once and groups by `artist_id`, yielding (artist_id, max_sim, + * matched_variant, matched_source) for every artist whose alias substrate + * matches `query`. Callers join this CTE on `artist_id` rather than running + * a correlated LATERAL per candidate row. + * + * Replacement for the previous `LEFT JOIN LATERAL` design: the LATERAL was + * correlated on `asa.artist_id = library.artist_id`, which steered the + * planner onto the PK btree and filtered `variant % q` row-by-row, never + * touching the GIN trigram index. The CTE form lets the planner pick the + * trigram bitmap scan once and hash-join into the outer query. */ -function buildAliasLateralFragments(query: string) { - return { - projection: sql`, - alias_hit.max_sim AS alias_max_sim, - alias_hit.matched_variant AS alias_matched_variant, - alias_hit.matched_source AS alias_matched_source`, - join: sql` - LEFT JOIN LATERAL ( - SELECT MAX(similarity(asa.variant, ${query})) AS max_sim, - (array_agg(asa.variant ORDER BY similarity(asa.variant, ${query}) DESC))[1] AS matched_variant, - (array_agg(asa.source ORDER BY similarity(asa.variant, ${query}) DESC))[1] AS matched_source - FROM ${artist_search_alias} asa - WHERE asa.artist_id = ${library.artist_id} - AND asa.variant % ${query} - ) alias_hit ON true`, - predicate: sql`OR alias_hit.max_sim IS NOT NULL`, - rankTerm: sql`, COALESCE(alias_hit.max_sim, 0)`, - }; +function buildAliasHitsCte(query: string) { + return sql`WITH alias_hits AS ( + SELECT + asa.artist_id, + MAX(similarity(asa.variant, ${query})) AS max_sim, + (array_agg(asa.variant ORDER BY similarity(asa.variant, ${query}) DESC))[1] AS matched_variant, + (array_agg(asa.source ORDER BY similarity(asa.variant, ${query}) DESC))[1] AS matched_source + FROM ${artist_search_alias} asa + WHERE asa.variant % ${query} + GROUP BY asa.artist_id + )`; } +/** Projection columns emitted by the alias branch of a UNION ALL search query. */ +const ALIAS_HITS_PROJECTION = sql`, + alias_hits.max_sim AS alias_max_sim, + alias_hits.matched_variant AS alias_matched_variant, + alias_hits.matched_source AS alias_matched_source`; + +/** NULL placeholders for the alias-shape columns on the non-alias UNION ALL branch. */ +const ALIAS_HITS_PROJECTION_NULLS = sql`, + NULL::real AS alias_max_sim, + NULL::text AS alias_matched_variant, + NULL::text AS alias_matched_source`; + /** * Build the `FROM library` query shape with the joins needed to project the * `LibraryArtistViewEntry` columns. `withPlays` adds the `album_plays` @@ -1126,26 +1137,48 @@ async function searchLibraryByTrigramBoth( .limit(n) as unknown as Promise; } - // Alias-enabled path: raw SQL with `LEFT JOIN LATERAL` against the alias - // cache. The OR-trigram predicate stays, plus the alias hit; rank widens - // to GREATEST(artist_sim, album_sim, COALESCE(alias_sim, 0)). - const alias = buildAliasLateralFragments(query); + // Alias-enabled path: ALT1 UNION ALL (BS#1318). The CTE runs the trigram + // bitmap scan once over `artist_search_alias`; branch (a) is the + // byte-identical alias-OFF trigram predicate against `library`, branch (b) + // joins the CTE on `artist_id` and dedupes against branch (a) via NOT-of + // the same trigram predicate. + // + // The trigram OR-predicate is evaluated twice — once positively in (a), + // once negated in (b) — but each is bitmap-indexable on its own, so the + // total cost is dominated by the substrate scan + LIMIT pushdown on the + // outer LAV scan rather than the correlated LATERAL's per-row scan. + // + // The UNION ALL is wrapped in a subquery so the outer ORDER BY can + // reference both the trigram similarity and the CTE-provided + // `alias_max_sim`. Postgres forbids expression-shaped ORDER BY directly on + // a UNION result (column names only); the subquery lifts the union's + // columns into a scope where `similarity(...)` is a legal ORDER BY term. const streamingClause = on_streaming !== undefined ? sql`AND ${library.on_streaming} = ${on_streaming}` : sql``; + const trigramPredicate = sql`(${library.artist_name} % ${query} OR ${library.album_title} % ${query})`; const rows = (await db.execute(sql` - SELECT ${LIBRARY_VIEW_PROJECTION_RAW}${alias.projection} - FROM ${library} - ${LIBRARY_VIEW_JOINS_RAW} - ${alias.join} - WHERE ( - ${library.artist_name} % ${query} - OR ${library.album_title} % ${query} - ${alias.predicate} - ) - ${streamingClause} + ${buildAliasHitsCte(query)} + SELECT * FROM ( + ( + SELECT ${LIBRARY_VIEW_PROJECTION_RAW}${ALIAS_HITS_PROJECTION_NULLS} + FROM ${library} + ${LIBRARY_VIEW_JOINS_RAW} + WHERE ${trigramPredicate} + ${streamingClause} + ) + UNION ALL + ( + SELECT ${LIBRARY_VIEW_PROJECTION_RAW}${ALIAS_HITS_PROJECTION} + FROM ${library} + ${LIBRARY_VIEW_JOINS_RAW} + INNER JOIN alias_hits ON alias_hits.artist_id = ${library.artist_id} + WHERE NOT ${trigramPredicate} + ${streamingClause} + ) + ) alias_search ORDER BY GREATEST( - similarity(${library.artist_name}, ${query}), - similarity(${library.album_title}, ${query}) - ${alias.rankTerm} + similarity(artist_name, ${query}), + similarity(album_title, ${query}), + COALESCE(alias_max_sim, 0) ) DESC LIMIT ${n} `)) as unknown as (LibraryArtistViewEntry & AliasHitFields)[]; @@ -1881,22 +1914,36 @@ export async function searchByArtist(artistName: string, limit = 5): Promise enrichLibraryResult(viewRowToLibraryResult(row))); } - // Alias-enabled: single-column trigram on artist_name, OR'd with the - // LATERAL alias hit. GREATEST collapses to MAX of two terms since there's - // no album_title predicate here. - const alias = buildAliasLateralFragments(artistName); + // Alias-enabled: ALT1 UNION ALL (BS#1318). Single-column trigram on + // `library.artist_name` as branch (a); branch (b) joins the CTE on + // `artist_id` and excludes rows already matched by (a). GREATEST collapses + // to MAX of two terms since there's no album_title predicate here. + // + // Wrap the UNION ALL in a subquery so the outer ORDER BY can use + // expression-shaped terms (`similarity(...)`); Postgres forbids + // expression ORDER BY directly on a UNION result. + const trigramPredicate = sql`${library.artist_name} % ${artistName}`; const rows = (await db.execute(sql` - SELECT ${LIBRARY_VIEW_PROJECTION_RAW}${alias.projection} - FROM ${library} - ${LIBRARY_VIEW_JOINS_RAW} - ${alias.join} - WHERE ( - ${library.artist_name} % ${artistName} - ${alias.predicate} - ) + ${buildAliasHitsCte(artistName)} + SELECT * FROM ( + ( + SELECT ${LIBRARY_VIEW_PROJECTION_RAW}${ALIAS_HITS_PROJECTION_NULLS} + FROM ${library} + ${LIBRARY_VIEW_JOINS_RAW} + WHERE ${trigramPredicate} + ) + UNION ALL + ( + SELECT ${LIBRARY_VIEW_PROJECTION_RAW}${ALIAS_HITS_PROJECTION} + FROM ${library} + ${LIBRARY_VIEW_JOINS_RAW} + INNER JOIN alias_hits ON alias_hits.artist_id = ${library.artist_id} + WHERE NOT ${trigramPredicate} + ) + ) alias_search ORDER BY GREATEST( - similarity(${library.artist_name}, ${artistName}) - ${alias.rankTerm} + similarity(artist_name, ${artistName}), + COALESCE(alias_max_sim, 0) ) DESC LIMIT ${limit} `)) as unknown as (LibraryArtistViewEntry & AliasHitFields)[]; diff --git a/tests/unit/services/library-search-alias.test.ts b/tests/unit/services/library-search-alias.test.ts index 6d6b785a..caf3c1d4 100644 --- a/tests/unit/services/library-search-alias.test.ts +++ b/tests/unit/services/library-search-alias.test.ts @@ -292,12 +292,13 @@ describe('catalog search — alias-aware LATERAL JOIN (PR 5)', () => { expect(results[0].matched_via_alias).toBeUndefined(); }); - it('flag on + only field-specific conditions: LATERAL is suppressed (no all-field branch to OR into)', async () => { + it('flag on + only field-specific conditions: alias path is suppressed (gated on all-field condition)', async () => { // A pure `artist:foo` query parses to one field=='artist_name' - // condition. buildAllFieldMatch is never called, so the alias OR is - // never added to WHERE; the LATERAL would compute similarity for every - // candidate row with no chance of an alias-only hit surviving. Skip - // the join entirely — result set is identical to the flag-off path. + // condition with no `field === 'all'` member, so `hasAllFieldCondition` + // is false and `aliasActive` is false. The catalog query falls through + // to the legacy single-SELECT path — no CTE, no UNION ALL, no alias + // substrate join — preserving pre-#1318 behavior for field-specific + // queries. process.env.CATALOG_SEARCH_ALIAS_ENABLED = 'true'; resetCatalogSearchAliasConfig(); stubGenreFormatLookups(); @@ -309,13 +310,51 @@ describe('catalog search — alias-aware LATERAL JOIN (PR 5)', () => { expect(results).toHaveLength(1); expect(results[0].matched_via_alias).toBeUndefined(); // Both calls are issued via Promise.all (dataQuery + countQuery). The - // load-bearing assertion is that neither SQL embedded `alias_hit`. + // load-bearing assertion is that neither SQL embedded the alias + // substrate join (LATERAL alias_hit or CTE alias_hits). const dataCall = db.execute.mock.calls[0]?.[0]; const countCall = db.execute.mock.calls[1]?.[0]; const renderedData = JSON.stringify(dataCall ?? ''); const renderedCount = JSON.stringify(countCall ?? ''); expect(renderedData).not.toContain('alias_hit'); expect(renderedCount).not.toContain('alias_hit'); + expect(renderedData).not.toContain('alias_hits'); + expect(renderedCount).not.toContain('alias_hits'); + }); + + it('flag on: emits ALT1 UNION ALL shape with alias_hits CTE (BS#1318)', async () => { + // BS#1318 regression pin: the alias-aware catalog path must use the + // CTE + UNION ALL form, not the LATERAL JOIN. The LATERAL had a 3-6.5× + // p95 regression on selective queries because it picked the PK btree + // and filtered `variant % q` row-by-row, never touching the GIN + // trigram index. UNION ALL with a CTE lets the planner run the + // trigram bitmap scan once. + process.env.CATALOG_SEARCH_ALIAS_ENABLED = 'true'; + resetCatalogSearchAliasConfig(); + stubGenreFormatLookups(); + db.execute.mockReset(); + db.execute.mockResolvedValueOnce([baseQueryRow]).mockResolvedValueOnce([{ total: 1 }]); + + await searchCatalogQuery(baseQueryParams); + + const dataCall = db.execute.mock.calls[0]?.[0]; + const countCall = db.execute.mock.calls[1]?.[0]; + const renderedData = JSON.stringify(dataCall ?? ''); + const renderedCount = JSON.stringify(countCall ?? ''); + + // Positive: CTE + UNION ALL appear in both queries (data + count). + expect(renderedData).toContain('alias_hits'); + expect(renderedData).toContain('UNION ALL'); + expect(renderedCount).toContain('alias_hits'); + expect(renderedCount).toContain('UNION ALL'); + + // Negative: the LATERAL JOIN must be gone — that's the whole point. + // `alias_hit` (singular) was the LATERAL alias; with UNION ALL we + // project from `alias_hits` (the CTE name) instead. + expect(renderedData).not.toContain('alias_hit ON true'); + expect(renderedCount).not.toContain('alias_hit ON true'); + expect(renderedData).not.toContain('LEFT JOIN LATERAL'); + expect(renderedCount).not.toContain('LEFT JOIN LATERAL'); }); }); @@ -355,5 +394,57 @@ describe('catalog search — alias-aware LATERAL JOIN (PR 5)', () => { const hit = results[0] as { matched_via_alias?: Array<{ matched_variant: string; source: string }> }; expect(hit.matched_via_alias).toEqual([{ matched_variant: 'Thee Oh Sees', source: 'wxyc_library_alt' }]); }); + + it('flag on: emits ALT1 UNION ALL shape with alias_hits CTE (BS#1318)', async () => { + // BS#1318 regression pin for the request-line single-column trigram + // path. Same root cause as the catalog `/library/query` path: the + // LATERAL was correlated on artist_id and never used the GIN trigram + // index. UNION ALL with a CTE fixes the plan. + process.env.CATALOG_SEARCH_ALIAS_ENABLED = 'true'; + resetCatalogSearchAliasConfig(); + db.select.mockReset(); + db.execute.mockReset(); + db.execute.mockResolvedValue([]); + + await searchByArtist('Thee Oh Sees'); + + const call = db.execute.mock.calls[0]?.[0]; + const rendered = JSON.stringify(call ?? ''); + expect(rendered).toContain('alias_hits'); + expect(rendered).toContain('UNION ALL'); + expect(rendered).not.toContain('alias_hit ON true'); + expect(rendered).not.toContain('LEFT JOIN LATERAL'); + }); + }); + + describe('searchLibrary (Both-mode trigram path) — ALT1 UNION ALL shape (BS#1318)', () => { + it('flag on: emits ALT1 UNION ALL shape with alias_hits CTE', async () => { + // BS#1318 regression pin for the Both-mode trigram fallback in + // `searchLibraryByTrigramBoth`. The tsvector tier returns 0 rows so + // the trigram tier fires; assert the SQL uses the new CTE + UNION ALL + // form rather than the LATERAL JOIN. + process.env.CATALOG_SEARCH_ALIAS_ENABLED = 'true'; + resetCatalogSearchAliasConfig(); + + const tsvectorChain = createMockQueryChain([]); + tsvectorChain.limit = jest.fn().mockResolvedValue([]); + db.select.mockReset(); + db.select.mockReturnValue(tsvectorChain); + db.execute.mockReset(); + db.execute.mockResolvedValue([]); + + await searchLibrary('Thee Oh Sees'); + + // db.execute is called once for the alias-aware trigram query (and + // potentially for `checkLibraryArtistNameHealth` — but only the + // alias-aware call embeds the CTE/UNION ALL keywords). + const aliasCall = db.execute.mock.calls.find((call) => JSON.stringify(call[0] ?? '').includes('alias_hits')); + expect(aliasCall).toBeDefined(); + const rendered = JSON.stringify(aliasCall?.[0] ?? ''); + expect(rendered).toContain('alias_hits'); + expect(rendered).toContain('UNION ALL'); + expect(rendered).not.toContain('alias_hit ON true'); + expect(rendered).not.toContain('LEFT JOIN LATERAL'); + }); }); });