From 5be16045c33f2849eb0be6d9b7fc0b06c68dddab Mon Sep 17 00:00:00 2001 From: Flavian DESVERNE Date: Thu, 12 Aug 2021 18:23:55 +0200 Subject: [PATCH] fix: order by m2m with disconnected records --- .../order_by_aggregation.rs | 81 +++++++++++++++++++ .../sql-query-connector/src/join_utils.rs | 72 +++++------------ 2 files changed, 101 insertions(+), 52 deletions(-) diff --git a/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/order_by_aggregation.rs b/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/order_by_aggregation.rs index 0066b58de768..0f960d5fd6cf 100644 --- a/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/order_by_aggregation.rs +++ b/query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/order_and_pagination/order_by_aggregation.rs @@ -525,6 +525,87 @@ mod order_by_aggr { Ok(()) } + // https://github.com/prisma/prisma/issues/8036 + fn schema_regression_8036() -> String { + let schema = indoc! { + r#"model Post { + id Int @id @default(autoincrement()) + title String + #m2m(LikedPeople, Person[], Int) + } + + model Person { + id Int @id @default(autoincrement()) + name String + #m2m(likePosts, Post[], Int) + } + "# + }; + + schema.to_owned() + } + + // Regression test for: // https://github.com/prisma/prisma/issues/8036 + #[connector_test(schema(schema_regression_8036))] + async fn count_m2m_records_not_connected(runner: Runner) -> TestResult<()> { + run_query!( + runner, + r#"mutation { createOnePerson(data: { name: "Alice" }) { id } }"# + ); + run_query!( + runner, + r#"mutation { createOnePost(data: { title: "First", LikedPeople: { connect: { id: 1 } } }) { id } }"# + ); + run_query!( + runner, + r#"mutation { createOnePost(data: { title: "Second" }) { id } }"# + ); + run_query!(runner, r#"mutation { createOnePost(data: { title: "Third" }) { id } }"#); + run_query!( + runner, + r#"mutation { createOnePost(data: { title: "Fourth" }) { id } }"# + ); + run_query!(runner, r#"mutation { createOnePost(data: { title: "Fifth" }) { id } }"#); + + insta::assert_snapshot!( + run_query!(&runner, r#"{ + findManyPost( + cursor: { id: 1 }, + skip: 1, + take: 4 + orderBy: [{ LikedPeople: { _count: desc } }, { id: asc }] + ) { + id + title + _count { + LikedPeople + } + } + } + "#), + @r###"{"data":{"findManyPost":[{"id":2,"title":"Second","_count":{"LikedPeople":0}},{"id":3,"title":"Third","_count":{"LikedPeople":0}},{"id":4,"title":"Fourth","_count":{"LikedPeople":0}},{"id":5,"title":"Fifth","_count":{"LikedPeople":0}}]}}"### + ); + + insta::assert_snapshot!( + run_query!(&runner, r#"{ + findManyPost( + cursor: { id: 1 } + take: 2 + orderBy: [{ title: asc }, { LikedPeople: { _count: asc } }, { id: asc }] + ) { + id + _count { + LikedPeople + } + } + } + "#), + @r###"{"data":{"findManyPost":[{"id":1,"_count":{"LikedPeople":1}},{"id":4,"_count":{"LikedPeople":0}}]}}"### + ); + + Ok(()) + } + async fn create_test_data(runner: &Runner) -> TestResult<()> { create_row(runner, r#"{ id: 1, name: "Alice", categories: { create: [{ id: 1, name: "Startup" }] }, posts: { create: { id: 1, title: "alice_post_1", categories: { create: [{ id: 2, name: "News" }, { id: 3, name: "Society" }] }} } }"#).await?; create_row(runner, r#"{ id: 2, name: "Bob", categories: { create: [{ id: 4, name: "Computer Science" }, { id: 5, name: "Music" }] }, posts: { create: [{ id: 2, title: "bob_post_1", categories: { create: [{ id: 6, name: "Finance" }] } }, { id: 3, title: "bob_post_2", categories: { create: [{ id: 7, name: "History" }, { id: 8, name: "Gaming" }, { id: 9, name: "Hacking" }] } }] } }"#).await?; diff --git a/query-engine/connectors/sql-query-connector/src/join_utils.rs b/query-engine/connectors/sql-query-connector/src/join_utils.rs index 13f215aad159..f18fa11fbb2d 100644 --- a/query-engine/connectors/sql-query-connector/src/join_utils.rs +++ b/query-engine/connectors/sql-query-connector/src/join_utils.rs @@ -32,7 +32,7 @@ pub fn compute_aggr_join( fn compute_aggr_join_one2m( rf: &RelationFieldRef, - sort_aggregation: AggregationType, + aggregation: AggregationType, aggregator_alias: &str, join_alias: &str, previous_join: Option<&AliasedJoin>, @@ -49,7 +49,7 @@ fn compute_aggr_join_one2m( // + SELECT A.fk FROM A let query = Select::from_table(rf.related_model().as_table()).columns(select_columns); - let aggr_expr = match sort_aggregation { + let aggr_expr = match aggregation { AggregationType::Count { _all } => count(asterisk()), }; @@ -97,67 +97,36 @@ fn compute_aggr_join_m2m( previous_join: Option<&AliasedJoin>, ) -> AliasedJoin { let relation_table = rf.as_table(); - let model_a = rf.model(); - let model_a_alias = format!("{}_A", model_a.db_name()); let a_ids = rf.model().primary_identifier(); - let a_columns: Vec<_> = a_ids.as_columns().map(|c| c.table(model_a_alias.clone())).collect(); - - let model_b = rf.related_model(); - let model_b_alias = format!("{}_B", model_b.db_name()); + let a_columns: Vec<_> = a_ids.as_columns().collect(); let b_ids = rf.related_model().primary_identifier(); - let b_columns: Vec<_> = b_ids.as_columns().map(|c| c.table(model_b_alias.clone())).collect(); - // SQL statements below refers to three tables: - // A (left table): aliased as A_A - // B (right table): aliased as B_B - // _AtoB (join table): not aliased - // A & B are aliased to support m2m self relations, in which case we inner join twice on the same table - // If A & B are the "User" table, they'd be aliased to: User_A & User_B - - // + SELECT A_A.id FROM _AtoB - let query = Select::from_table(relation_table).columns(a_columns.clone()); + // + SELECT A.id FROM A + let query = Select::from_table(model_a.as_table()).columns(a_columns.clone()); let aggr_expr = match aggregation { - AggregationType::Count { _all } => count(asterisk()), + AggregationType::Count { _all } => count(rf.related_field().m2m_columns()), }; - // SELECT A_A.id, - // + COUNT(*) AS - // FROM _AtoB + + // SELECT A.id, + // + COUNT(_AtoB.B) AS + // FROM A let query = query.value(aggr_expr.alias(aggregator_alias.to_owned())); - let conditions_a: Vec = a_columns + let left_join_conditions: Vec = a_columns .clone() .into_iter() .map(|c| c.equals(rf.related_field().m2m_columns()).into()) .collect(); - let conditions_b: Vec = b_columns - .clone() - .into_iter() - .map(|c| c.equals(rf.m2m_columns()).into()) - .collect(); - // SELECT A_A.id, COUNT(*) AS FROM _AtoB - // + INNER JOIN A AS A_A ON A_A.id = _AtoB.A - // + INNER JOIN B AS B_B ON B_B.id = _AtoB.B - let query = query - .inner_join( - model_a - .as_table() - .alias(model_a_alias) - .on(ConditionTree::And(conditions_a)), - ) - .inner_join( - model_b - .as_table() - .alias(model_b_alias) - .on(ConditionTree::And(conditions_b)), - ); - - // SELECT A_A.id, COUNT(*) AS FROM _AtoB - // INNER JOIN A AS A_A ON A_A.id = _AtoB.A - // INNER JOIN B AS B_B ON B_B.id = _AtoB.B - // + GROUP BY A_A.id + // SELECT A.id, COUNT(_AtoB.B) AS FROM A + // + LEFT JOIN _AtoB ON (A.id = _AtoB.B) + let query = query.left_join(relation_table.on(ConditionTree::And(left_join_conditions))); + + // SELECT A.id, COUNT(_AtoB.B) AS FROM A + // LEFT JOIN _AtoB ON (A.id = _AtoB.B) + // + GROUP BY A.id let query = a_columns.into_iter().fold(query, |acc, f| acc.group_by(f.clone())); let (left_fields, right_fields) = (a_ids.scalar_fields(), b_ids.scalar_fields()); @@ -175,9 +144,8 @@ fn compute_aggr_join_m2m( .collect::>(); // + LEFT JOIN ( - // SELECT A_A.id, COUNT(*) AS FROM _AtoB - // INNER JOIN A AS A_A ON (A_A.id = _AtoB.A) - // INNER JOIN B AS B_B ON (B_B.id = _AtoB.B) + // SELECT A.id, COUNT(_AtoB.B) AS FROM A + // LEFT JOIN _AtoB ON (A.id = _AtoB.B) // GROUP BY A.id // + ) AS ON (.id = .id) let join = Table::from(query)