diff --git a/query-compiler/query-compiler/src/translate/query/write.rs b/query-compiler/query-compiler/src/translate/query/write.rs index 68ab59893ac1..df26e8db7d51 100644 --- a/query-compiler/query-compiler/src/translate/query/write.rs +++ b/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::>(), + ), + other => todo!("{other:?}"), }) } diff --git a/query-compiler/query-compiler/tests/data/delete-many-limit.json b/query-compiler/query-compiler/tests/data/delete-many-limit.json new file mode 100644 index 000000000000..bd68a5b92360 --- /dev/null +++ b/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 + } + } +} diff --git a/query-compiler/query-compiler/tests/data/delete-many.json b/query-compiler/query-compiler/tests/data/delete-many.json new file mode 100644 index 000000000000..2d29d8fad338 --- /dev/null +++ b/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 + } + } +} diff --git a/query-compiler/query-compiler/tests/data/delete-one.json b/query-compiler/query-compiler/tests/data/delete-one.json new file mode 100644 index 000000000000..522736e75e6f --- /dev/null +++ b/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 + } + } +} diff --git a/query-compiler/query-compiler/tests/snapshots/queries__queries@delete-many-limit.json.snap b/query-compiler/query-compiler/tests/snapshots/queries__queries@delete-many-limit.json.snap new file mode 100644 index 000000000000..86b31273cad0 --- /dev/null +++ b/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"))]) diff --git a/query-compiler/query-compiler/tests/snapshots/queries__queries@delete-many.json.snap b/query-compiler/query-compiler/tests/snapshots/queries__queries@delete-many.json.snap new file mode 100644 index 000000000000..93c3035ad1cf --- /dev/null +++ b/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))]) diff --git a/query-compiler/query-compiler/tests/snapshots/queries__queries@delete-one.json.snap b/query-compiler/query-compiler/tests/snapshots/queries__queries@delete-one.json.snap new file mode 100644 index 000000000000..b2840c06bc37 --- /dev/null +++ b/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))]) diff --git a/query-engine/connectors/sql-query-connector/src/database/operations/write.rs b/query-engine/connectors/sql-query-connector/src/database/operations/write.rs index c218b8683fc9..b6233a0b2e4b 100644 --- a/query-engine/connectors/sql-query-connector/src/database/operations/write.rs +++ b/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, ctx: &Context<'_>, ) -> crate::Result { - 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(); diff --git a/query-engine/core/src/interpreter/query_interpreters/write.rs b/query-engine/core/src/interpreter/query_interpreters/write.rs index f6267bf0488c..11257a0b8047 100644 --- a/query-engine/core/src/interpreter/query_interpreters/write.rs +++ b/query-engine/core/src/interpreter/query_interpreters/write.rs @@ -272,13 +272,7 @@ async fn delete_one( traceparent: Option, ) -> InterpretationResult { // 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 diff --git a/query-engine/core/src/query_ast/write.rs b/query-engine/core/src/query_ast/write.rs index e2a76b5ebcc0..64d5463201b2 100644 --- a/query-engine/core/src/query_ast/write.rs +++ b/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, + 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, @@ -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) { - 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); } } diff --git a/query-engine/core/src/query_graph_builder/write/delete.rs b/query-engine/core/src/query_graph_builder/write/delete.rs index 9f2071440514..b522e706a797 100644 --- a/query-engine/core/src/query_graph_builder/write/delete.rs +++ b/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); diff --git a/query-engine/core/src/query_graph_builder/write/nested/delete_nested.rs b/query-engine/core/src/query_graph_builder/write/nested/delete_nested.rs index c3de8071d804..ceffb6abed28 100644 --- a/query-engine/core/src/query_graph_builder/write/nested/delete_nested.rs +++ b/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, }))); diff --git a/query-engine/query-builders/query-builder/src/lib.rs b/query-engine/query-builders/query-builder/src/lib.rs index c74552e798b6..8f9151f637dc 100644 --- a/query-engine/query-builders/query-builder/src/lib.rs +++ b/query-engine/query-builders/query-builder/src/lib.rs @@ -46,6 +46,20 @@ pub trait QueryBuilder { limit: Option, ) -> Result, Box>; + fn build_delete( + &self, + model: &Model, + filter: RecordFilter, + selected_fields: Option<&FieldSelection>, + ) -> Result>; + + fn build_deletes( + &self, + model: &Model, + filter: RecordFilter, + limit: Option, + ) -> Result, Box>; + fn build_raw( &self, model: Option<&Model>, diff --git a/query-engine/query-builders/sql-query-builder/src/lib.rs b/query-engine/query-builders/sql-query-builder/src/lib.rs index a46eadcce521..2a143b967d68 100644 --- a/query-engine/query-builders/sql-query-builder/src/lib.rs +++ b/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> { 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> { + 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, + ) -> Result, Box> { + let queries = write::generate_delete_statements(model, record_filter, limit, &self.context) + .into_iter() + .map(|q| self.convert_query(q)) + .collect::, _>>()?; + Ok(queries) + } + fn build_raw( &self, _model: Option<&Model>, diff --git a/query-engine/query-builders/sql-query-builder/src/write.rs b/query-engine/query-builders/sql-query-builder/src/write.rs index b0166e8ba14d..4b93557d9016 100644 --- a/query-engine/query-builders/sql-query-builder/src/write.rs +++ b/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, + ctx: &Context<'_>, +) -> Vec> { + 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))