fix(qe): make UpdateRecord::WithSelection nodes return `RecordSelec…

…tion` (#4508)

Query graph nodes that are not result nodes and fulfill the requirements of other nodes don't inherently need names because their results aren't serialized. However, we still need some name to be able to build RecordSelection because it's required there. Previously the update node with selection that has no name would return a QueryResult::Id instead of QueryResult::RecordSelection to work around that, but it caused problems as the query interpreter relied on QueryResult::Id being the primary identifier and not arbitrary fields.

This PR makes the name required for UpdateRecord::WithSelection and provides a stub name when the node doesn't directly correspond to the client's query. This is consistent with other intermediate nodes that need to build RecordSelection. Some of them use constant placeholder names (like find_children_by_parent for RelatedRecordsQuery), some of them use empty strings (e.g. CreateRecord does this).

I chose an empty string because it needs to be owned here and an empty String::new() doesn't allocate memory, and the specific value doesn't matter anyway because there's likely not even a way to observe this name (it's not used in Display and ToGraphviz impls for UpdateRecord).

Fixes: prisma/prisma#21182

nix/shell.nix

@@ -15,6 +15,7 @@ in
       nodejs.pkgs.pnpm
 
       jq
+      graphviz
       wasm-bindgen-cli
       wasm-pack
     ];

query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/mod.rs

@@ -19,6 +19,7 @@ mod prisma_16760;
 mod prisma_17103;
 mod prisma_18517;
 mod prisma_20799;
+mod prisma_21182;
 mod prisma_21369;
 mod prisma_21901;
 mod prisma_5952;

query-engine/connector-test-kit-rs/query-engine-tests/tests/new/regressions/prisma_21182.rs

@@ -0,0 +1,98 @@
+use query_engine_tests::*;
+
+/// Regression test for https://github.com/prisma/prisma/issues/21182
+#[test_suite(schema(schema))]
+mod regression {
+    use indoc::indoc;
+
+    fn schema() -> String {
+        indoc! {r#"
+            model User {
+              #id(id, Int, @id)
+              email  String  @unique
+              name   String?
+              roleId Int
+              role   Role    @relation(fields: [roleId], references: [id])
+            }
+
+            model Role {
+              #id(id, Int, @id)
+              name  String
+              users User[]
+
+              tagId Int?
+              tag   Tag? @relation(fields: [tagId], references: [id])
+            }
+
+            model Tag {
+              #id(id, Int, @id)
+              name  String
+              roles Role[]
+            }
+        "#}
+        .to_owned()
+    }
+
+    #[connector_test]
+    async fn query_with_normalized_dependencies(runner: Runner) -> TestResult<()> {
+        run_query!(
+            runner,
+            indoc! {r#"
+                mutation {
+                  createOneUser (
+                    data: {
+                      id: 1,
+                      email: "[email protected]",
+                      role: {
+                        create: {
+                          id: 1,
+                          name: "ADMIN",
+                          tag: {
+                            create: {
+                              id: 1,
+                              name: "cs"
+                            }
+                          }
+                        }
+                      }
+                    }
+                  ) {
+                      id,
+                      email,
+                      roleId
+                  }
+                }
+            "#}
+        );
+
+        run_query!(
+            runner,
+            indoc! {r#"
+                mutation {
+                  updateOneUser(
+                    where: {
+                      email: "[email protected]",
+                    },
+                    data: {
+                      role: {
+                        update: {
+                          data: {
+                            tag: {
+                              disconnect: true
+                            }
+                          }
+                        }
+                      }
+                    }
+                  ) {
+                    id,
+                    email,
+                    roleId
+                  }
+                }
+            "#}
+        );
+
+        Ok(())
+    }
+}

query-engine/core/src/interpreter/query_interpreters/write.rs

@@ -81,10 +81,10 @@ async fn update_one(
         .await?;
 
     match q {
-        UpdateRecord::WithSelection(q) if q.name.is_some() => {
+        UpdateRecord::WithSelection(q) => {
             let res = res
                 .map(|res| RecordSelection {
-                    name: q.name.unwrap(),
+                    name: q.name,
                     fields: q.selection_order,
                     scalars: res.into(),
                     nested: vec![],
@@ -95,13 +95,6 @@ async fn update_one(
 
             Ok(QueryResult::RecordSelection(res))
         }
-        UpdateRecord::WithSelection(q) => {
-            let res = res
-                .map(|record| record.extract_selection_result(&q.selected_fields))
-                .transpose()?;
-
-            Ok(QueryResult::Id(res))
-        }
         UpdateRecord::WithoutSelection(_) => {
             let res = res
                 .map(|record| record.extract_selection_result(&q.model().primary_identifier()))

query-engine/core/src/query_ast/write.rs

@@ -206,7 +206,11 @@ impl ToGraphviz for WriteQuery {
         match self {
             Self::CreateRecord(q) => format!("CreateRecord(model: {}, args: {:?})", q.model.name(), q.args),
             Self::CreateManyRecords(q) => format!("CreateManyRecord(model: {})", q.model.name()),
-            Self::UpdateRecord(q) => format!("UpdateRecord(model: {})", q.model().name(),),
+            Self::UpdateRecord(q) => format!(
+                "UpdateRecord(model: {}, selection: {:?})",
+                q.model().name(),
+                q.selected_fields()
+            ),
             Self::DeleteRecord(q) => format!("DeleteRecord: {}, {:?}", q.model.name(), q.record_filter),
             Self::UpdateManyRecords(q) => format!("UpdateManyRecords(model: {}, args: {:?})", q.model.name(), q.args),
             Self::DeleteManyRecords(q) => format!("DeleteManyRecords: {}", q.model.name()),
@@ -303,8 +307,7 @@ impl UpdateRecord {
 
 #[derive(Debug, Clone)]
 pub struct UpdateRecordWithSelection {
-    // Used for serialization. When `None`, the result will only be used to fulfill other nodes requirement.
-    pub name: Option<String>,
+    pub name: String,
     pub model: Model,
     pub record_filter: RecordFilter,
     pub args: WriteArgs,

query-engine/core/src/query_graph_builder/write/update.rs

@@ -200,7 +200,7 @@ where
 
             Query::Write(WriteQuery::UpdateRecord(UpdateRecord::WithSelection(
                 UpdateRecordWithSelection {
-                    name: Some(field.name.to_owned()),
+                    name: field.name,
                     model: model.clone(),
                     record_filter: filter.into(),
                     args,
@@ -215,7 +215,7 @@ where
 
             Query::Write(WriteQuery::UpdateRecord(UpdateRecord::WithSelection(
                 UpdateRecordWithSelection {
-                    name: None,
+                    name: String::new(), // This node will not be serialized so we don't need a name.
                     model: model.clone(),
                     record_filter: filter.into(),
                     args,