From 74a758a5296083915f8ac3d7be95b64be4bd8407 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 20 May 2019 14:10:39 -0600 Subject: [PATCH 1/3] Further address performance regression in search Even after #1746, we're still seeing a performance issue with search in production. Now it's limited to searches that are a single letter, or short 2 letter words like 'do'. It's caused by any search that would cause PG to warn that the query contains only stopwords. It appears the code path taken when `plainto_tsquery` returns an empty query is substantially slower than it would be otherwise, even if the query contains stopwords. The reason this has started causing problems now is that #1560 caused the query to change from performing a nested loop join to a hash join. Due to what appears to be a bug in PG, `plainto_tsquery` is getting called once per row when a hash join is performed. When the query is passed as the only argument, the function is declared as `STABLE`, meaning that within a single statement it will always return the same result for the same arguments, so PG should only be calling it once (or at least only a handful of times). There's a second form available where you explicitly pass the language as an argument. This form is marked as `IMMUTABLE`, so the query planner will just replace the call to the function with its results. Unfortunately, PG is picky about how we pass the language. It doesn't consider a cast from `text` to `regconfig` to be `IMMUTABLE`, only `STABLE` (which is valid, since it's based on a `pg_catalog` lookup -- The fact that it accepts a string literal as `IMMUTABLE` actually seems wrong). The actual value is the OID of the row in `pg_ts_config`, which is *not* stable. Since `regconfig('english'::text)` is not considered `IMMUTABLE`, we just need to embed it as a string literal instead. --- src/controllers/krate/search.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index 2d897481194..16566c119ad 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -33,7 +33,10 @@ use crate::models::krate::{canon_crate_name, ALL_COLUMNS}; /// function out to cover the different use cases, and create unit tests /// for them. pub fn search(req: &mut dyn Request) -> CargoResult { - use diesel::sql_types::Bool; + use diesel::sql_types::{Bool, Text}; + use diesel::dsl::sql; + + sql_function!(fn plainto_tsquery(lang: Text, query: Text) -> TsQuery); let conn = req.db_conn()?; let (offset, limit) = req.pagination(10, 100)?; @@ -59,9 +62,11 @@ pub fn search(req: &mut dyn Request) -> CargoResult { if !q_string.is_empty() { let sort = params.get("sort").map(|s| &**s).unwrap_or("relevance"); - let q = plainto_tsquery(q_string); + let q = sql::("plainto_tsquery('english', ") + .bind::(q_string) + .sql(")"); query = query.filter( - q.matches(crates::textsearchable_index_col) + q.clone().matches(crates::textsearchable_index_col) .or(Crate::like_name(&q_string)), ); From e5fe4eba5bc37c941cccc35d0d4332def6abab6d Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 20 May 2019 14:34:43 -0600 Subject: [PATCH 2/3] rustfmt --- src/controllers/krate/search.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index 16566c119ad..8555c57c7e9 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -33,8 +33,8 @@ use crate::models::krate::{canon_crate_name, ALL_COLUMNS}; /// function out to cover the different use cases, and create unit tests /// for them. pub fn search(req: &mut dyn Request) -> CargoResult { - use diesel::sql_types::{Bool, Text}; use diesel::dsl::sql; + use diesel::sql_types::{Bool, Text}; sql_function!(fn plainto_tsquery(lang: Text, query: Text) -> TsQuery); @@ -66,7 +66,8 @@ pub fn search(req: &mut dyn Request) -> CargoResult { .bind::(q_string) .sql(")"); query = query.filter( - q.clone().matches(crates::textsearchable_index_col) + q.clone() + .matches(crates::textsearchable_index_col) .or(Crate::like_name(&q_string)), ); From 8f14232bd3eeb48cbfda2616269f633bd1ef8f93 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 20 May 2019 14:35:12 -0600 Subject: [PATCH 3/3] Remove unused sql_function! --- src/controllers/krate/search.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index 8555c57c7e9..e61ba527646 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -36,8 +36,6 @@ pub fn search(req: &mut dyn Request) -> CargoResult { use diesel::dsl::sql; use diesel::sql_types::{Bool, Text}; - sql_function!(fn plainto_tsquery(lang: Text, query: Text) -> TsQuery); - let conn = req.db_conn()?; let (offset, limit) = req.pagination(10, 100)?; let params = req.query();