Skip to content

Commit 16e8d71

Browse files
committed
Auto merge of #1749 - sgrif:sg-fix-slow-query-more, r=pietroalbini
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.
2 parents 638f37a + 8f14232 commit 16e8d71

File tree

1 file changed

+7
-3
lines changed

1 file changed

+7
-3
lines changed

src/controllers/krate/search.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ use crate::models::krate::{canon_crate_name, ALL_COLUMNS};
3333
/// function out to cover the different use cases, and create unit tests
3434
/// for them.
3535
pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
36-
use diesel::sql_types::Bool;
36+
use diesel::dsl::sql;
37+
use diesel::sql_types::{Bool, Text};
3738

3839
let conn = req.db_conn()?;
3940
let (offset, limit) = req.pagination(10, 100)?;
@@ -59,9 +60,12 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
5960
if !q_string.is_empty() {
6061
let sort = params.get("sort").map(|s| &**s).unwrap_or("relevance");
6162

62-
let q = plainto_tsquery(q_string);
63+
let q = sql::<TsQuery>("plainto_tsquery('english', ")
64+
.bind::<Text, _>(q_string)
65+
.sql(")");
6366
query = query.filter(
64-
q.matches(crates::textsearchable_index_col)
67+
q.clone()
68+
.matches(crates::textsearchable_index_col)
6569
.or(Crate::like_name(&q_string)),
6670
);
6771

0 commit comments

Comments
 (0)