Skip to content

Commit f635d52

Browse files
committed
Refactor crates#index to use a single query for crates and count
We should do this all over the application, as there's tons of endpoints which are doing a separate query for the count when a window function would achieve the same thing more efficiently.
1 parent 838acf9 commit f635d52

File tree

1 file changed

+56
-55
lines changed

1 file changed

+56
-55
lines changed

src/krate.rs

Lines changed: 56 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use conduit::{Request, Response};
1010
use conduit_router::RequestParams;
1111
use curl::easy::Easy;
1212
use diesel::prelude::*;
13-
use diesel::pg::{Pg, PgConnection};
13+
use diesel::pg::PgConnection;
1414
use diesel::pg::upsert::*;
1515
use diesel_full_text_search::*;
1616
use license_exprs;
@@ -638,68 +638,71 @@ impl Model for Crate {
638638
/// Handles the `GET /crates` route.
639639
#[allow(trivial_casts)]
640640
pub fn index(req: &mut Request) -> CargoResult<Response> {
641+
use diesel::expression::dsl::sql;
642+
use diesel::types::BigInt;
643+
641644
let conn = req.db_conn()?;
642645
let (offset, limit) = req.pagination(10, 100)?;
643646
let params = req.query();
647+
let sort = params.get("sort").map(|s| &**s).unwrap_or("alpha");
644648

645-
// This is a bit of a hack, but Boxed queries in Diesel don't implement `.clone()` and we need
646-
// the query twice so we can `.count` it. This function is basically free, but it'd be nice to
647-
// not have to wrap this in a funciton.
648-
fn crates_query<'a>(params: &'a HashMap<String, String>, user: CargoResult<&User>)
649-
-> CargoResult<crates::BoxedQuery<'a, Pg, <AllColumns as Expression>::SqlType>> {
650-
let mut query = crates::table.select(ALL_COLUMNS).into_boxed();
651-
652-
if let Some(q_string) = params.get("q") {
653-
let q = plainto_tsquery(q_string);
654-
query = query.filter(q.matches(crates::textsearchable_index_col));
655-
} else if let Some(letter) = params.get("letter") {
656-
let pattern = format!("{}%", letter.chars().next().unwrap()
657-
.to_lowercase().collect::<String>());
658-
query = query.filter(canon_crate_name(crates::name).like(pattern));
659-
} else if let Some(kw) = params.get("keyword") {
660-
query = query.filter(crates::id.eq_any(
661-
crates_keywords::table.select(crates_keywords::crate_id)
662-
.inner_join(keywords::table)
663-
.filter(lower(keywords::keyword).eq(lower(kw)))
664-
));
665-
} else if let Some(cat) = params.get("category") {
666-
query = query.filter(crates::id.eq_any(
667-
crates_categories::table.select(crates_categories::crate_id)
668-
.inner_join(categories::table)
669-
.filter(categories::category.eq(cat).or(
670-
categories::category.like(format!("{}::%", cat))))
671-
));
672-
} else if let Some(user_id) = params.get("user_id").and_then(|s| s.parse::<i32>().ok()) {
673-
query = query.filter(crates::id.eq_any((
674-
crate_owners::table.select(crate_owners::crate_id)
675-
.filter(crate_owners::owner_id.eq(user_id))
676-
.filter(crate_owners::owner_kind.eq(OwnerKind::User as i32))
677-
)));
678-
} else if params.get("following").is_some() {
679-
query = query.filter(crates::id.eq_any((
680-
follows::table.select(follows::crate_id)
681-
.filter(follows::user_id.eq(user?.id))
682-
)));
683-
}
684-
Ok(query)
649+
let mut query = crates::table
650+
.select((ALL_COLUMNS, sql::<BigInt>("COUNT(*) OVER ()")))
651+
.limit(limit)
652+
.offset(offset)
653+
.into_boxed();
654+
655+
if sort == "downloads" {
656+
query = query.order(crates::downloads.desc())
657+
} else {
658+
query = query.order(crates::name.asc())
685659
}
686660

687-
let mut query = crates_query(&params, req.user())?;
688-
let sort = params.get("sort").map(|s| &**s).unwrap_or("alpha");
689-
match (params.get("q"), sort) {
690-
(Some(q), "downloads") => {
691-
query = query.order((crates::name.eq(q).desc(), crates::downloads.desc()))
692-
}
693-
(Some(q_string), _) => {
694-
let q = plainto_tsquery(q_string);
661+
if let Some(q_string) = params.get("q") {
662+
let q = plainto_tsquery(q_string);
663+
query = query.filter(q.matches(crates::textsearchable_index_col));
664+
665+
let perfect_match = crates::name.eq(q_string).desc();
666+
if sort == "downloads" {
667+
query = query.order((perfect_match, crates::downloads.desc()));
668+
} else {
695669
let rank = ts_rank_cd(crates::textsearchable_index_col, q);
696-
query = query.order((crates::name.eq(q_string).desc(), rank.desc()))
670+
query = query.order((perfect_match, rank.desc()))
697671
}
698-
(None, "downloads") => query = query.order(crates::downloads.desc()),
699-
_ => query = query.order(crates::name.asc()),
672+
} else if let Some(letter) = params.get("letter") {
673+
let pattern = format!("{}%", letter.chars().next().unwrap()
674+
.to_lowercase().collect::<String>());
675+
query = query.filter(canon_crate_name(crates::name).like(pattern));
676+
} else if let Some(kw) = params.get("keyword") {
677+
query = query.filter(crates::id.eq_any(
678+
crates_keywords::table.select(crates_keywords::crate_id)
679+
.inner_join(keywords::table)
680+
.filter(lower(keywords::keyword).eq(lower(kw)))
681+
));
682+
} else if let Some(cat) = params.get("category") {
683+
query = query.filter(crates::id.eq_any(
684+
crates_categories::table.select(crates_categories::crate_id)
685+
.inner_join(categories::table)
686+
.filter(categories::category.eq(cat).or(
687+
categories::category.like(format!("{}::%", cat))))
688+
));
689+
} else if let Some(user_id) = params.get("user_id").and_then(|s| s.parse::<i32>().ok()) {
690+
query = query.filter(crates::id.eq_any((
691+
crate_owners::table.select(crate_owners::crate_id)
692+
.filter(crate_owners::owner_id.eq(user_id))
693+
.filter(crate_owners::owner_kind.eq(OwnerKind::User as i32))
694+
)));
695+
} else if params.get("following").is_some() {
696+
query = query.filter(crates::id.eq_any((
697+
follows::table.select(follows::crate_id)
698+
.filter(follows::user_id.eq(req.user()?.id))
699+
)));
700700
}
701701

702-
let crates = query.limit(limit).offset(offset).load::<Crate>(conn)?;
702+
let data = query.load::<(Crate, i64)>(conn)?;
703+
let total = data.get(0).map(|&(_, t)| t).unwrap_or(0);
704+
let crates = data.into_iter().map(|(c, _)| c).collect::<Vec<_>>();
705+
703706
let versions = Version::belonging_to(&crates)
704707
.load::<Version>(conn)?
705708
.grouped_by(&crates)
@@ -719,8 +722,6 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
719722
Ok(krate.minimal_encodable(max_version, Some(badges)))
720723
}).collect::<Result<_, ::diesel::result::Error>>()?;
721724

722-
let total = crates_query(&params, req.user())?.count().get_result(conn)?;
723-
724725
#[derive(RustcEncodable)]
725726
struct R { crates: Vec<EncodableCrate>, meta: Meta }
726727
#[derive(RustcEncodable)]

0 commit comments

Comments
 (0)