Skip to content

Commit fd999a5

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 ab6eb95 commit fd999a5

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
@@ -11,7 +11,7 @@ use conduit::{Request, Response};
1111
use conduit_router::RequestParams;
1212
use curl::easy::Easy;
1313
use diesel::prelude::*;
14-
use diesel::pg::{Pg, PgConnection};
14+
use diesel::pg::PgConnection;
1515
use diesel::pg::upsert::*;
1616
use diesel_full_text_search::*;
1717
use license_exprs;
@@ -639,68 +639,71 @@ impl Model for Crate {
639639
/// Handles the `GET /crates` route.
640640
#[allow(trivial_casts)]
641641
pub fn index(req: &mut Request) -> CargoResult<Response> {
642+
use diesel::expression::dsl::sql;
643+
use diesel::types::BigInt;
644+
642645
let conn = req.db_conn()?;
643646
let (offset, limit) = req.pagination(10, 100)?;
644647
let params = req.query();
648+
let sort = params.get("sort").map(|s| &**s).unwrap_or("alpha");
645649

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

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

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

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

0 commit comments

Comments
 (0)