Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support basic deletes #5147

Merged
merged 4 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions query-compiler/query-compiler/src/translate/query/write.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:?}"),
})
}
12 changes: 12 additions & 0 deletions query-compiler/query-compiler/tests/data/delete-many-limit.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"modelName": "Post",
"action": "deleteMany",
"query": {
"arguments": {
"where": { "title": "whatever" }
},
"selection": {
"$scalars": true
}
}
}
13 changes: 13 additions & 0 deletions query-compiler/query-compiler/tests/data/delete-many.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"modelName": "Post",
"action": "deleteMany",
"query": {
"arguments": {
"where": { "title": "whatever" },
"limit": 2
},
"selection": {
"$scalars": true
}
}
}
12 changes: 12 additions & 0 deletions query-compiler/query-compiler/tests/data/delete-one.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"modelName": "Post",
"action": "deleteOne",
"query": {
"arguments": {
"where": { "id": 1 }
},
"selection": {
"$scalars": true
}
}
}
Original file line number Diff line number Diff line change
@@ -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"))])
Original file line number Diff line number Diff line change
@@ -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))])
Original file line number Diff line number Diff line change
@@ -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))])
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 4 additions & 11 deletions query-engine/core/src/query_ast/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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
}
}

Expand All @@ -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);
}
}
4 changes: 2 additions & 2 deletions query-engine/core/src/query_graph_builder/write/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})));

Expand Down
14 changes: 14 additions & 0 deletions query-engine/query-builders/query-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
35 changes: 32 additions & 3 deletions query-engine/query-builders/sql-query-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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>,
Expand Down
23 changes: 22 additions & 1 deletion query-engine/query-builders/sql-query-builder/src/write.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand Down Expand Up @@ -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))
Expand Down
Loading