feat: support basic deletes (#5147)

* feat: support basic deletes

* test: update delete tests

* refactor: unoption record_filter

query-compiler/query-compiler/src/translate/query/write.rs

@@ -1,5 +1,7 @@
 use query_builder::QueryBuilder;
-use query_core::{RawQuery, UpdateManyRecords, UpdateRecord, UpdateRecordWithSelection, WriteQuery};
+use query_core::{
+    DeleteManyRecords, DeleteRecord, RawQuery, UpdateManyRecords, UpdateRecord, UpdateRecordWithSelection, WriteQuery,
+};
 use query_structure::QueryArguments;
 
 use crate::{expression::Expression, translate::TranslateResult, TranslateError};
@@ -80,12 +82,13 @@ pub(crate) fn translate_write_query(query: WriteQuery, builder: &dyn QueryBuilde
         }
 
         WriteQuery::UpdateRecord(UpdateRecord::WithSelection(UpdateRecordWithSelection {
+            name: _,
             model,
             record_filter,
             args,
             selected_fields,
             // TODO: we're ignoring selection order
-            ..
+            selection_order: _,
         })) => {
             let query = if args.is_empty() {
                 // if there's no args we can just issue a read query
@@ -121,6 +124,36 @@ pub(crate) fn translate_write_query(query: WriteQuery, builder: &dyn QueryBuilde
                 .map_err(TranslateError::QueryBuildFailure)?,
         ),
 
+        WriteQuery::DeleteRecord(DeleteRecord {
+            name: _,
+            model,
+            record_filter,
+            selected_fields,
+        }) => {
+            let selected_fields = selected_fields.as_ref().map(|sf| &sf.fields);
+            let query = builder
+                .build_delete(&model, record_filter, selected_fields)
+                .map_err(TranslateError::QueryBuildFailure)?;
+            if selected_fields.is_some() {
+                Expression::Unique(Box::new(Expression::Query(query)))
+            } else {
+                Expression::Execute(query)
+            }
+        }
+
+        WriteQuery::DeleteManyRecords(DeleteManyRecords {
+            model,
+            record_filter,
+            limit,
+        }) => Expression::Sum(
+            builder
+                .build_deletes(&model, record_filter, limit)
+                .map_err(TranslateError::QueryBuildFailure)?
+                .into_iter()
+                .map(Expression::Execute)
+                .collect::<Vec<_>>(),
+        ),
+
         other => todo!("{other:?}"),
     })
 }

query-compiler/query-compiler/tests/data/delete-many-limit.json

@@ -0,0 +1,12 @@
+{
+  "modelName": "Post",
+  "action": "deleteMany",
+  "query": {
+    "arguments": {
+      "where": { "title": "whatever" }
+    },
+    "selection": {
+      "$scalars": true
+    }
+  }
+}

query-compiler/query-compiler/tests/data/delete-many.json

@@ -0,0 +1,13 @@
+{
+  "modelName": "Post",
+  "action": "deleteMany",
+  "query": {
+    "arguments": {
+      "where": { "title": "whatever" },
+      "limit": 2
+    },
+    "selection": {
+      "$scalars": true
+    }
+  }
+}

query-compiler/query-compiler/tests/data/delete-one.json

@@ -0,0 +1,12 @@
+{
+  "modelName": "Post",
+  "action": "deleteOne",
+  "query": {
+    "arguments": {
+      "where": { "id": 1 }
+    },
+    "selection": {
+      "$scalars": true
+    }
+  }
+}

query-compiler/query-compiler/tests/snapshots/queries__queries@delete-many-limit.json.snap

@@ -0,0 +1,7 @@
+---
+source: query-compiler/query-compiler/tests/queries.rs
+expression: pretty
+input_file: query-compiler/query-compiler/tests/data/delete-many-limit.json
+---
+sum (execute «DELETE FROM "public"."Post" WHERE "public"."Post"."title" = $1»
+     params [const(String("whatever"))])

query-compiler/query-compiler/tests/snapshots/queries__queries@delete-many.json.snap

@@ -0,0 +1,9 @@
+---
+source: query-compiler/query-compiler/tests/queries.rs
+expression: pretty
+input_file: query-compiler/query-compiler/tests/data/delete-many.json
+---
+sum (execute «DELETE FROM "public"."Post" WHERE ("public"."Post"."id") IN
+              (SELECT "public"."Post"."id" FROM "public"."Post" WHERE
+              "public"."Post"."title" = $1 LIMIT $2)»
+     params [const(String("whatever")), const(BigInt(2))])

query-compiler/query-compiler/tests/snapshots/queries__queries@delete-one.json.snap

@@ -0,0 +1,9 @@
+---
+source: query-compiler/query-compiler/tests/queries.rs
+expression: pretty
+input_file: query-compiler/query-compiler/tests/data/delete-one.json
+---
+unique (query «DELETE FROM "public"."Post" WHERE ("public"."Post"."id" = $1 AND
+               1=1) RETURNING "public"."Post"."id", "public"."Post"."title",
+               "public"."Post"."userId"»
+        params [const(BigInt(1))])

query-engine/connectors/sql-query-connector/src/database/operations/write.rs

@@ -9,7 +9,7 @@ use quaint::{
     prelude::{native_uuid, uuid_to_bin, uuid_to_bin_swapped, Aliasable, Select, SqlFamily},
 };
 use query_structure::*;
-use sql_query_builder::{column_metadata, update, write, Context, FilterBuilder, SelectionResultExt, SqlTraceComment};
+use sql_query_builder::{column_metadata, update, write, Context, SelectionResultExt, SqlTraceComment};
 use std::borrow::Cow;
 use std::collections::HashMap;
 use user_facing_errors::query_engine::DatabaseConstraint;
@@ -324,30 +324,20 @@ pub(crate) async fn delete_records(
     limit: Option<usize>,
     ctx: &Context<'_>,
 ) -> crate::Result<usize> {
-    let filter_condition = FilterBuilder::without_top_level_joins().visit_filter(record_filter.clone().filter, ctx);
-
-    // If we have selectors, then we must chunk the mutation into multiple if necessary and add the ids to the filter.
-    let row_count = if let Some(selectors) = record_filter.selectors.as_deref() {
-        let mut row_count = 0;
-        let mut remaining_limit = limit;
-        let slice = &selectors[..remaining_limit.unwrap_or(selectors.len()).min(selectors.len())];
-
-        for delete in write::delete_many_from_ids_and_filter(model, slice, filter_condition, remaining_limit, ctx) {
-            row_count += conn.execute(delete).await?;
-            if let Some(old_remaining_limit) = remaining_limit {
-                // u64 to usize cast here cannot 'overflow' as the number of rows was limited to MAX usize in the first place.
-                let new_remaining_limit = old_remaining_limit - row_count as usize;
-                if new_remaining_limit == 0 {
-                    break;
-                }
-                remaining_limit = Some(new_remaining_limit);
+    let mut row_count = 0;
+    let mut remaining_limit = limit;
+
+    for delete in write::generate_delete_statements(model, record_filter, limit, ctx) {
+        row_count += conn.execute(delete).await?;
+        if let Some(old_remaining_limit) = remaining_limit {
+            // u64 to usize cast here cannot 'overflow' as the number of rows was limited to MAX usize in the first place.
+            let new_remaining_limit = old_remaining_limit - row_count as usize;
+            if new_remaining_limit == 0 {
+                break;
             }
+            remaining_limit = Some(new_remaining_limit);
         }
-        row_count
-    } else {
-        conn.execute(write::delete_many_from_filter(model, filter_condition, limit, ctx))
-            .await?
-    };
+    }
 
     Ok(row_count as usize)
 }
@@ -363,11 +353,15 @@ pub(crate) async fn delete_record(
     // in combination with this operation.
     debug_assert!(!record_filter.has_selectors());
 
-    let filter = FilterBuilder::without_top_level_joins().visit_filter(record_filter.filter, ctx);
     let selected_fields: ModelProjection = selected_fields.into();
 
     let result_set = conn
-        .query(write::delete_returning(model, filter, &selected_fields, ctx))
+        .query(write::delete_returning(
+            model,
+            record_filter.filter,
+            &selected_fields,
+            ctx,
+        ))
         .await?;
 
     let mut result_iter = result_set.into_iter();

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

@@ -272,13 +272,7 @@ async fn delete_one(
     traceparent: Option<TraceParent>,
 ) -> InterpretationResult<QueryResult> {
     // We need to ensure that we have a record finder, else we delete everything (conversion to empty filter).
-    let filter = match q.record_filter {
-        Some(f) => Ok(f),
-        None => Err(InterpreterError::InterpretationError(
-            "No record filter specified for delete record operation. Aborting.".to_owned(),
-            None,
-        )),
-    }?;
+    let filter = q.record_filter;
 
     if let Some(selected_fields) = q.selected_fields {
         let record = tx

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

@@ -382,7 +382,7 @@ pub struct UpdateManyRecordsFields {
 pub struct DeleteRecord {
     pub name: String,
     pub model: Model,
-    pub record_filter: Option<RecordFilter>,
+    pub record_filter: RecordFilter,
     /// Fields of the deleted record that client has requested to return.
     /// `None` if the connector does not support returning the deleted row.
     pub selected_fields: Option<DeleteRecordFields>,
@@ -460,14 +460,11 @@ impl FilteredQuery for DeleteManyRecords {
 
 impl FilteredQuery for DeleteRecord {
     fn get_filter(&mut self) -> Option<&mut Filter> {
-        self.record_filter.as_mut().map(|f| &mut f.filter)
+        Some(&mut self.record_filter.filter)
     }
 
     fn set_filter(&mut self, filter: Filter) {
-        match self.record_filter {
-            Some(ref mut rf) => rf.filter = filter,
-            None => self.record_filter = Some(filter.into()),
-        }
+        self.record_filter.filter = filter
     }
 }
 
@@ -485,10 +482,6 @@ impl FilteredNestedMutation for UpdateManyRecords {
 
 impl FilteredNestedMutation for DeleteRecord {
     fn set_selectors(&mut self, selectors: Vec<SelectionResult>) {
-        if let Some(ref mut rf) = self.record_filter {
-            rf.selectors = Some(selectors);
-        } else {
-            self.record_filter = Some(selectors.into())
-        }
+        self.record_filter.selectors = Some(selectors);
     }
 }

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

@@ -29,7 +29,7 @@ pub(crate) fn delete_record(
         let delete_query = Query::Write(WriteQuery::DeleteRecord(DeleteRecord {
             name: field.name,
             model: model.clone(),
-            record_filter: Some(filter.into()),
+            record_filter: filter.into(),
             selected_fields: Some(DeleteRecordFields {
                 fields: selected_fields,
                 order: selection_order,
@@ -58,7 +58,7 @@ pub(crate) fn delete_record(
         let delete_query = Query::Write(WriteQuery::DeleteRecord(DeleteRecord {
             name: String::new(), // This node will not be serialized so we don't need a name.
             model: model.clone(),
-            record_filter: Some(filter.into()),
+            record_filter: filter.into(),
             selected_fields: None,
         }));
         let delete_node = graph.create_node(delete_query);

query-engine/core/src/query_graph_builder/write/nested/delete_nested.rs

@@ -99,7 +99,7 @@ pub fn nested_delete(
             let delete_record_node = graph.create_node(Query::Write(WriteQuery::DeleteRecord(DeleteRecord {
                 name: String::new(),
                 model: child_model.clone(),
-                record_filter: Some(filter.into()),
+                record_filter: filter.into(),
                 selected_fields: None,
             })));
 

query-engine/query-builders/query-builder/src/lib.rs

@@ -46,6 +46,20 @@ pub trait QueryBuilder {
         limit: Option<usize>,
     ) -> Result<Vec<DbQuery>, Box<dyn std::error::Error + Send + Sync>>;
 
+    fn build_delete(
+        &self,
+        model: &Model,
+        filter: RecordFilter,
+        selected_fields: Option<&FieldSelection>,
+    ) -> Result<DbQuery, Box<dyn std::error::Error + Send + Sync>>;
+
+    fn build_deletes(
+        &self,
+        model: &Model,
+        filter: RecordFilter,
+        limit: Option<usize>,
+    ) -> Result<Vec<DbQuery>, Box<dyn std::error::Error + Send + Sync>>;
+
     fn build_raw(
         &self,
         model: Option<&Model>,

query-engine/query-builders/sql-query-builder/src/lib.rs

@@ -110,9 +110,8 @@ impl<'a, V: Visitor<'a>> QueryBuilder for SqlQueryBuilder<'a, V> {
     ) -> Result<DbQuery, Box<dyn std::error::Error + Send + Sync>> {
         match selected_fields {
             Some(selected_fields) => {
-                let selected_fields = ModelProjection::from(selected_fields);
-                let query =
-                    update::update_one_with_selection(model, record_filter, args, &selected_fields, &self.context);
+                let projection = ModelProjection::from(selected_fields);
+                let query = update::update_one_with_selection(model, record_filter, args, &projection, &self.context);
                 self.convert_query(query)
             }
             None => {
@@ -137,6 +136,36 @@ impl<'a, V: Visitor<'a>> QueryBuilder for SqlQueryBuilder<'a, V> {
         Ok(vec![self.convert_query(query)?])
     }
 
+    fn build_delete(
+        &self,
+        model: &Model,
+        record_filter: RecordFilter,
+        selected_fields: Option<&FieldSelection>,
+    ) -> Result<DbQuery, Box<dyn std::error::Error + Send + Sync>> {
+        let query = if let Some(selected_fields) = selected_fields {
+            write::delete_returning(model, record_filter.filter, &selected_fields.into(), &self.context)
+        } else {
+            let mut queries = write::generate_delete_statements(model, record_filter, None, &self.context).into_iter();
+            let query = queries.next().expect("should generate at least one query");
+            assert_eq!(queries.next(), None, "should generat at most one query");
+            query
+        };
+        self.convert_query(query)
+    }
+
+    fn build_deletes(
+        &self,
+        model: &Model,
+        record_filter: RecordFilter,
+        limit: Option<usize>,
+    ) -> Result<Vec<DbQuery>, Box<dyn std::error::Error + Send + Sync>> {
+        let queries = write::generate_delete_statements(model, record_filter, limit, &self.context)
+            .into_iter()
+            .map(|q| self.convert_query(q))
+            .collect::<Result<Vec<_>, _>>()?;
+        Ok(queries)
+    }
+
     fn build_raw(
         &self,
         _model: Option<&Model>,

query-engine/query-builders/sql-query-builder/src/write.rs

@@ -1,4 +1,5 @@
 use crate::limit::wrap_with_limit_subquery_if_needed;
+use crate::FilterBuilder;
 use crate::{model_extensions::*, sql_trace::SqlTraceComment, Context};
 use itertools::Itertools;
 use quaint::ast::*;
@@ -212,12 +213,32 @@ fn projection_into_columns(
     selected_fields.as_columns(ctx).map(|c| c.set_is_selected(true))
 }
 
+/// Generates deletes for multiple records, defined in the `RecordFilter`.
+pub fn generate_delete_statements(
+    model: &Model,
+    record_filter: RecordFilter,
+    limit: Option<usize>,
+    ctx: &Context<'_>,
+) -> Vec<Query<'static>> {
+    let filter_condition = FilterBuilder::without_top_level_joins().visit_filter(record_filter.filter.clone(), ctx);
+
+    // If we have selectors, then we must chunk the mutation into multiple if necessary and add the ids to the filter.
+    if let Some(selectors) = record_filter.selectors.as_deref() {
+        let slice = &selectors[..limit.unwrap_or(selectors.len()).min(selectors.len())];
+        delete_many_from_ids_and_filter(model, slice, filter_condition, limit, ctx)
+    } else {
+        vec![delete_many_from_filter(model, filter_condition, limit, ctx)]
+    }
+}
+
 pub fn delete_returning(
     model: &Model,
-    filter: ConditionTree<'static>,
+    filter: Filter,
     selected_fields: &ModelProjection,
     ctx: &Context<'_>,
 ) -> Query<'static> {
+    let filter = FilterBuilder::without_top_level_joins().visit_filter(filter, ctx);
+
     Delete::from_table(model.as_table(ctx))
         .so_that(filter)
         .returning(projection_into_columns(selected_fields, ctx))