From 83fa2d07cdd7d7fc2fa55b74c2c558a51058dcc9 Mon Sep 17 00:00:00 2001 From: James Fuller Date: Thu, 2 Jul 2026 16:51:11 +0200 Subject: [PATCH] perf: remove superfluous order by in query backing api/v3/license. --- migration/src/lib.rs | 2 + ...m0002230_sbom_source_doc_covering_index.rs | 54 +++++++++++++++++++ .../fundamental/src/license/service/mod.rs | 53 ++++++++---------- 3 files changed, 77 insertions(+), 32 deletions(-) create mode 100644 migration/src/m0002230_sbom_source_doc_covering_index.rs diff --git a/migration/src/lib.rs b/migration/src/lib.rs index fe74055d9..20196cbcc 100644 --- a/migration/src/lib.rs +++ b/migration/src/lib.rs @@ -64,6 +64,7 @@ mod m0002190_vulnerability_base_score_advisory; mod m0002200_source_document_ingested_index; mod m0002210_sbom_node_name_index; mod m0002220_drop_qualified_purl_gist_indexes; +mod m0002230_sbom_source_doc_covering_index; pub trait MigratorExt: Send { fn build_migrations() -> Migrations; @@ -143,6 +144,7 @@ impl MigratorExt for Migrator { .normal(m0002200_source_document_ingested_index::Migration) .normal(m0002210_sbom_node_name_index::Migration) .normal(m0002220_drop_qualified_purl_gist_indexes::Migration) + .normal(m0002230_sbom_source_doc_covering_index::Migration) } } diff --git a/migration/src/m0002230_sbom_source_doc_covering_index.rs b/migration/src/m0002230_sbom_source_doc_covering_index.rs new file mode 100644 index 000000000..78f65ea16 --- /dev/null +++ b/migration/src/m0002230_sbom_source_doc_covering_index.rs @@ -0,0 +1,54 @@ +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + // Drop the plain index on source_document_id — it is subsumed by the + // covering index below (same leading column, plus INCLUDE payload). + manager + .get_connection() + .execute_unprepared("DROP INDEX IF EXISTS sbom_source_document_id_idx") + .await + .map(|_| ())?; + + // Covering index lets the planner drive from source_document (sorted by + // ingested DESC) into sbom via source_document_id, then join sbom_node + // using the INCLUDEd columns — avoiding a full sort for + // ORDER BY ingested DESC LIMIT N queries. + manager + .get_connection() + .execute_unprepared( + r#" + CREATE INDEX IF NOT EXISTS sbom_source_doc_id_covering_idx + ON sbom (source_document_id) + INCLUDE (sbom_id, node_id) + "#, + ) + .await + .map(|_| ())?; + + Ok(()) + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .get_connection() + .execute_unprepared("DROP INDEX IF EXISTS sbom_source_doc_id_covering_idx") + .await + .map(|_| ())?; + + // Restore the original plain index. + manager + .get_connection() + .execute_unprepared( + "CREATE INDEX IF NOT EXISTS sbom_source_document_id_idx ON sbom (source_document_id)", + ) + .await + .map(|_| ())?; + + Ok(()) + } +} diff --git a/modules/fundamental/src/license/service/mod.rs b/modules/fundamental/src/license/service/mod.rs index 4f6e7fbf5..5b66d0ec9 100644 --- a/modules/fundamental/src/license/service/mod.rs +++ b/modules/fundamental/src/license/service/mod.rs @@ -309,29 +309,13 @@ impl LicenseService { .distinct() .column_as(expanded_license::Column::ExpandedText, LICENSE_TEXT); - // Build query for licenses not yet linked to any SBOM: includes both - // (a) pre-loaded SPDX dictionary entries with no SBOM connection yet, AND - // (b) licenses from older SBOMs ingested before license expansion was implemented. - // Use NOT EXISTS instead of LEFT JOIN + IS NULL to find licenses without SBOMs. - // On large tables, LEFT JOIN scans all rows while NOT EXISTS - // uses a Nested Loop Anti Join with index-only scan. - let exists_subquery = sea_query::Query::select() - .expr(Expr::val(1)) - .from(sbom_license_expanded::Entity) - .and_where( - Expr::col(( - sbom_license_expanded::Entity, - sbom_license_expanded::Column::LicenseId, - )) - .equals((license::Entity, license::Column::Id)), - ) - .to_owned(); - + // Include all license texts. The UNION (not UNION ALL) with + // expanded_license above already deduplicates, so no need to + // filter out licenses that have entries in sbom_license_expanded. let mut non_sbom_query = license::Entity::find() .select_only() .distinct() - .column_as(license::Column::Text, LICENSE_TEXT) - .filter(Expr::exists(exists_subquery).not()); + .column_as(license::Column::Text, LICENSE_TEXT); // Apply filtering to both queries (without sorting - that's applied to the UNION result) let filter_only = Query { @@ -359,23 +343,28 @@ impl LicenseService { // Union the two queries QueryTrait::query(&mut spdx_query).union(UnionType::Distinct, non_sbom_query.into_query()); - // Add an expression for the license field and use it as the default sort - let expr = SimpleExpr::Custom(LICENSE_TEXT.into()); - spdx_query = spdx_query - .filtering_with( - q("").sort(&search.sort), - Columns::default().add_expr("license", expr.clone(), sea_orm::ColumnType::Text), - )? - .order_by_asc(expr); - - let mut union_query = spdx_query.into_query(); - // Count total results + // Clone for count BEFORE adding ORDER BY (sorting is unnecessary for COUNT) + let count_subquery = spdx_query.clone().into_query(); let count_query = sea_query::Query::select() .expr_as(Func::count(Expr::col(Asterisk)), "num_items") - .from_subquery(union_query.clone(), "subquery") + .from_subquery(count_subquery, "subquery") .to_owned(); + // Add an expression for the license field and use it as the default sort + let expr = SimpleExpr::Custom(LICENSE_TEXT.into()); + spdx_query = spdx_query.filtering_with( + q("").sort(&search.sort), + Columns::default().add_expr("license", expr.clone(), sea_orm::ColumnType::Text), + )?; + // Only add default sort when the user didn't provide one + // (avoids duplicate ORDER BY text ASC, text ASC) + if search.sort.is_empty() { + spdx_query = spdx_query.order_by_asc(expr); + } + + let mut union_query = spdx_query.into_query(); + #[derive(Debug, Default, Clone, Serialize, Deserialize, ToSchema, FromQueryResult)] struct Count { num_items: i64,