Skip to content

Commit

Permalink
fix: batched nested m2m reverse pagination (#4125)
Browse files Browse the repository at this point in the history 
When fetching m2m related records and the query must be batches, the ordering is done in memory once we've fetched all records. When a negative `take` is passed, the ordering must be reversed so that we take for the end of the result set. That reverse ordering wasn't taken into account.
  • Loading branch information
@Weakky
Weakky authored Aug 10, 2023
1 parent 1366301 commit 9fe3fe4
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 5 deletions.
140 changes: 140 additions & 0 deletions ...or-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/nested_pagination.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,146 @@ mod nested_pagination {
Ok(())
}

// m:n relations, child is connected to many parents, using simple pagination
// A1 <> B1, B2, B3, B4, B5, B6
// A2 <> B2, B3, B5, B7, B8
// A3
// A many-to-many relationship with multiple connected children" should "return all items correctly with skip / take nested pagination
#[connector_test(schema(simple_m2m))]
async fn m2m_many_children_nested_skip_take(runner: Runner) -> TestResult<()> {
// >>> Begin create test data
insta::assert_snapshot!(
run_query!(&runner, r#"mutation {
createOneModelA(
data: {
id: "A1"
manyB: {
connectOrCreate: [
{ where: { id: "B1" }, create: { id: "B1" } }
{ where: { id: "B2" }, create: { id: "B2" } }
{ where: { id: "B3" }, create: { id: "B3" } }
{ where: { id: "B4" }, create: { id: "B4" } }
{ where: { id: "B5" }, create: { id: "B5" } }
{ where: { id: "B6" }, create: { id: "B6" } }
]
}
}
) {
id
manyB {
id
}
}
}"#),
@r###"{"data":{"createOneModelA":{"id":"A1","manyB":[{"id":"B1"},{"id":"B2"},{"id":"B3"},{"id":"B4"},{"id":"B5"},{"id":"B6"}]}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, r#"mutation {
createOneModelA(
data: {
id: "A2"
manyB: {
connectOrCreate: [
{ where: { id: "B2" }, create: { id: "B2" } },
{ where: { id: "B3" }, create: { id: "B3" } }
{ where: { id: "B5" }, create: { id: "B5" } }
{ where: { id: "B7" }, create: { id: "B7" } }
{ where: { id: "B8" }, create: { id: "B8" } }
]
}
}
) {
id
manyB {
id
}
}
}"#),
@r###"{"data":{"createOneModelA":{"id":"A2","manyB":[{"id":"B2"},{"id":"B3"},{"id":"B5"},{"id":"B7"},{"id":"B8"}]}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, r#"mutation{ createOneModelA(data: { id: "A3" }) { id manyB { id } } }"#),
@r###"{"data":{"createOneModelA":{"id":"A3","manyB":[]}}}"###
);
// <<< End create test data

insta::assert_snapshot!(
run_query!(&runner, r#"{
findUniqueModelA(where: { id: "A1" }) {
id
manyB(skip: 1) {
id
}
}
}"#),
@r###"{"data":{"findUniqueModelA":{"id":"A1","manyB":[{"id":"B2"},{"id":"B3"},{"id":"B4"},{"id":"B5"},{"id":"B6"}]}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, r#"{
findManyModelA {
id
manyB(skip: 1) {
id
}
}
}"#),
@r###"{"data":{"findManyModelA":[{"id":"A1","manyB":[{"id":"B2"},{"id":"B3"},{"id":"B4"},{"id":"B5"},{"id":"B6"}]},{"id":"A2","manyB":[{"id":"B3"},{"id":"B5"},{"id":"B7"},{"id":"B8"}]},{"id":"A3","manyB":[]}]}}"###
);

insta::assert_snapshot!(
run_query!(&runner, r#"{
findUniqueModelA(where: { id: "A1" }) {
id
manyB(skip: 1, take: 2) {
id
}
}
}"#),
@r###"{"data":{"findUniqueModelA":{"id":"A1","manyB":[{"id":"B2"},{"id":"B3"}]}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, r#"{
findManyModelA {
id
manyB(skip: 1, take: 2) {
id
}
}
}"#),
@r###"{"data":{"findManyModelA":[{"id":"A1","manyB":[{"id":"B2"},{"id":"B3"}]},{"id":"A2","manyB":[{"id":"B3"},{"id":"B5"}]},{"id":"A3","manyB":[]}]}}"###
);

insta::assert_snapshot!(
run_query!(&runner, r#"{
findUniqueModelA(where: { id: "A1" }) {
id
manyB(skip: 1, take: -2, orderBy: { id: asc }) {
id
}
}
}"#),
@r###"{"data":{"findUniqueModelA":{"id":"A1","manyB":[{"id":"B4"},{"id":"B5"}]}}}"###
);

insta::assert_snapshot!(
run_query!(&runner, r#"{
findManyModelA {
id
manyB(skip: 1, take: -2, orderBy: { id: asc }) {
id
}
}
}"#),
@r###"{"data":{"findManyModelA":[{"id":"A1","manyB":[{"id":"B4"},{"id":"B5"}]},{"id":"A2","manyB":[{"id":"B5"},{"id":"B7"}]},{"id":"A3","manyB":[]}]}}"###
);

Ok(())
}

async fn create_test_data(runner: &Runner) -> TestResult<()> {
create_row(
runner,
Expand Down
2 changes: 1 addition & 1 deletion query-engine/connectors/sql-query-connector/src/database/operations/read.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub(crate) async fn get_many_records(
}

if !order.is_empty() {
records.order_by(&order)
records.order_by(&order, reversed)
}
}
_ => {
Expand Down
12 changes: 8 additions & 4 deletions query-engine/prisma-models/src/record.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl ManyRecords {
}
}

pub fn order_by(&mut self, order_bys: &[OrderBy]) {
pub fn order_by(&mut self, order_bys: &[OrderBy], reversed: bool) {
let field_indices: HashMap<&str, usize> = self
.field_names
.iter()
Expand All @@ -66,9 +66,13 @@ impl ManyRecords {
OrderBy::Scalar(by_scalar) => {
let index = field_indices[by_scalar.field.db_name()];

match by_scalar.sort_order {
SortOrder::Ascending => a.values[index].cmp(&b.values[index]),
SortOrder::Descending => b.values[index].cmp(&a.values[index]),
match (by_scalar.sort_order, reversed) {
(SortOrder::Ascending, false) | (SortOrder::Descending, true) => {
a.values[index].cmp(&b.values[index])
}
(SortOrder::Descending, false) | (SortOrder::Ascending, true) => {
b.values[index].cmp(&a.values[index])
}
}
}
OrderBy::ScalarAggregation(_) => unimplemented!(),
Expand Down

0 comments on commit 9fe3fe4

Please sign in to comment.