server: Fix panic due to database collation
When the database's collation is not "C", the DB order is not the same as the Rust order. As such, asserting that the elements are in increasing order fails. However, since both queries get the order from the database, they should be in the same order. With too many users, the query had a giant filter `IN (u1, u2, u3, ...)`. In PostgreSQL, we can pass the users as an array instead, but that doesn't work with SQLite. Instead, we repeat the filter from the previous query to get the same users/groups, as a subquery.
This commit is contained in:
committed by
Austin Alvarado
parent
64140b4939
commit
442c70b6d2
@@ -79,25 +79,24 @@ fn get_group_filter_expr(filter: GroupRequestFilter) -> Cond {
|
||||
impl GroupListerBackendHandler for SqlBackendHandler {
|
||||
#[instrument(skip(self), level = "debug", ret, err)]
|
||||
async fn list_groups(&self, filters: Option<GroupRequestFilter>) -> Result<Vec<Group>> {
|
||||
let filters = filters
|
||||
.map(|f| {
|
||||
GroupColumn::GroupId
|
||||
.in_subquery(
|
||||
model::Group::find()
|
||||
.find_also_linked(model::memberships::GroupToUser)
|
||||
.select_only()
|
||||
.column(GroupColumn::GroupId)
|
||||
.filter(get_group_filter_expr(f))
|
||||
.into_query(),
|
||||
)
|
||||
.into_condition()
|
||||
})
|
||||
.unwrap_or_else(|| SimpleExpr::Value(true.into()).into_condition());
|
||||
let results = model::Group::find()
|
||||
.order_by_asc(GroupColumn::GroupId)
|
||||
.find_with_related(model::Membership)
|
||||
.filter(
|
||||
filters
|
||||
.map(|f| {
|
||||
GroupColumn::GroupId
|
||||
.in_subquery(
|
||||
model::Group::find()
|
||||
.find_also_linked(model::memberships::GroupToUser)
|
||||
.select_only()
|
||||
.column(GroupColumn::GroupId)
|
||||
.filter(get_group_filter_expr(f))
|
||||
.into_query(),
|
||||
)
|
||||
.into_condition()
|
||||
})
|
||||
.unwrap_or_else(|| SimpleExpr::Value(true.into()).into_condition()),
|
||||
)
|
||||
.filter(filters.clone())
|
||||
.all(&self.sql_pool)
|
||||
.await?;
|
||||
let mut groups: Vec<_> = results
|
||||
@@ -110,9 +109,16 @@ impl GroupListerBackendHandler for SqlBackendHandler {
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
let group_ids = groups.iter().map(|u| &u.id);
|
||||
let attributes = model::GroupAttributes::find()
|
||||
.filter(model::GroupAttributesColumn::GroupId.is_in(group_ids))
|
||||
.filter(
|
||||
model::GroupAttributesColumn::GroupId.in_subquery(
|
||||
model::Group::find()
|
||||
.filter(filters)
|
||||
.select_only()
|
||||
.column(model::groups::Column::GroupId)
|
||||
.into_query(),
|
||||
),
|
||||
)
|
||||
.order_by_asc(model::GroupAttributesColumn::GroupId)
|
||||
.order_by_asc(model::GroupAttributesColumn::AttributeName)
|
||||
.all(&self.sql_pool)
|
||||
@@ -120,12 +126,6 @@ impl GroupListerBackendHandler for SqlBackendHandler {
|
||||
let mut attributes_iter = attributes.into_iter().peekable();
|
||||
use itertools::Itertools; // For take_while_ref
|
||||
for group in groups.iter_mut() {
|
||||
assert!(attributes_iter
|
||||
.peek()
|
||||
.map(|u| u.group_id >= group.id)
|
||||
.unwrap_or(true),
|
||||
"Attributes are not sorted, groups are not sorted, or previous group didn't consume all the attributes");
|
||||
|
||||
group.attributes = attributes_iter
|
||||
.take_while_ref(|u| u.group_id == group.id)
|
||||
.map(AttributeValue::from)
|
||||
|
||||
@@ -104,23 +104,22 @@ impl UserListerBackendHandler for SqlBackendHandler {
|
||||
// To simplify the query, we always fetch groups. TODO: cleanup.
|
||||
_get_groups: bool,
|
||||
) -> Result<Vec<UserAndGroups>> {
|
||||
let filters = filters
|
||||
.map(|f| {
|
||||
UserColumn::UserId
|
||||
.in_subquery(
|
||||
model::User::find()
|
||||
.find_also_linked(model::memberships::UserToGroup)
|
||||
.select_only()
|
||||
.column(UserColumn::UserId)
|
||||
.filter(get_user_filter_expr(f))
|
||||
.into_query(),
|
||||
)
|
||||
.into_condition()
|
||||
})
|
||||
.unwrap_or_else(|| SimpleExpr::Value(true.into()).into_condition());
|
||||
let mut users: Vec<_> = model::User::find()
|
||||
.filter(
|
||||
filters
|
||||
.map(|f| {
|
||||
UserColumn::UserId
|
||||
.in_subquery(
|
||||
model::User::find()
|
||||
.find_also_linked(model::memberships::UserToGroup)
|
||||
.select_only()
|
||||
.column(UserColumn::UserId)
|
||||
.filter(get_user_filter_expr(f))
|
||||
.into_query(),
|
||||
)
|
||||
.into_condition()
|
||||
})
|
||||
.unwrap_or_else(|| SimpleExpr::Value(true.into()).into_condition()),
|
||||
)
|
||||
.filter(filters.clone())
|
||||
.order_by_asc(UserColumn::UserId)
|
||||
.find_with_linked(model::memberships::UserToGroup)
|
||||
.order_by_asc(SimpleExpr::Column(
|
||||
@@ -134,10 +133,18 @@ impl UserListerBackendHandler for SqlBackendHandler {
|
||||
groups: Some(groups.into_iter().map(Into::<GroupDetails>::into).collect()),
|
||||
})
|
||||
.collect();
|
||||
|
||||
// At this point, the users don't have attributes, we need to populate it with another query.
|
||||
let user_ids = users.iter().map(|u| &u.user.user_id);
|
||||
let attributes = model::UserAttributes::find()
|
||||
.filter(model::UserAttributesColumn::UserId.is_in(user_ids))
|
||||
.filter(
|
||||
model::UserAttributesColumn::UserId.in_subquery(
|
||||
model::User::find()
|
||||
.filter(filters)
|
||||
.select_only()
|
||||
.column(model::users::Column::UserId)
|
||||
.into_query(),
|
||||
),
|
||||
)
|
||||
.order_by_asc(model::UserAttributesColumn::UserId)
|
||||
.order_by_asc(model::UserAttributesColumn::AttributeName)
|
||||
.all(&self.sql_pool)
|
||||
@@ -145,12 +152,6 @@ impl UserListerBackendHandler for SqlBackendHandler {
|
||||
let mut attributes_iter = attributes.into_iter().peekable();
|
||||
use itertools::Itertools; // For take_while_ref
|
||||
for user in users.iter_mut() {
|
||||
assert!(attributes_iter
|
||||
.peek()
|
||||
.map(|u| u.user_id >= user.user.user_id)
|
||||
.unwrap_or(true),
|
||||
"Attributes are not sorted, users are not sorted, or previous user didn't consume all the attributes");
|
||||
|
||||
user.user.attributes = attributes_iter
|
||||
.take_while_ref(|u| u.user_id == user.user.user_id)
|
||||
.map(AttributeValue::from)
|
||||
|
||||
Reference in New Issue
Block a user