feat(qe): avoid extra select on delete queries (#4595)

.github/workflows/test-query-engine-driver-adapters.yml

@@ -53,6 +53,7 @@ jobs:
       CLOSED_TX_CLEANUP: "2"
       SIMPLE_TEST_MODE: "1"
       QUERY_BATCH_SIZE: "10"
+      WASM_BUILD_PROFILE: "profiling" # Include debug info for proper backtraces
       WORKSPACE_ROOT: ${{ github.workspace }}
 
     runs-on: ubuntu-latest

psl/psl-core/src/builtin_connectors/cockroach_datamodel_connector.rs

@@ -60,7 +60,9 @@ const CAPABILITIES: ConnectorCapabilities = enumflags2::make_bitflags!(Connector
     InsertReturning |
     UpdateReturning |
     RowIn |
-    LateralJoin
+    LateralJoin |
+    DeleteReturning |
+    SupportsFiltersOnRelationsWithoutJoins
 });
 
 const SCALAR_TYPE_DEFAULTS: &[(ScalarType, CockroachType)] = &[

psl/psl-core/src/builtin_connectors/mongodb.rs

@@ -30,7 +30,8 @@ const CAPABILITIES: ConnectorCapabilities = enumflags2::make_bitflags!(Connector
     MongoDbQueryRaw |
     DefaultValueAuto |
     TwoWayEmbeddedManyToManyRelation |
-    UndefinedType
+    UndefinedType |
+    DeleteReturning
 });
 
 pub(crate) struct MongoDbDatamodelConnector;

psl/psl-core/src/builtin_connectors/mssql_datamodel_connector.rs

@@ -50,7 +50,9 @@ const CAPABILITIES: ConnectorCapabilities = enumflags2::make_bitflags!(Connector
     SupportsTxIsolationReadCommitted |
     SupportsTxIsolationRepeatableRead |
     SupportsTxIsolationSerializable |
-    SupportsTxIsolationSnapshot
+    SupportsTxIsolationSnapshot |
+    SupportsFiltersOnRelationsWithoutJoins
+    // InsertReturning | DeleteReturning - unimplemented.
 });
 
 pub(crate) struct MsSqlDatamodelConnector;

psl/psl-core/src/builtin_connectors/mysql_datamodel_connector.rs

@@ -59,7 +59,8 @@ const CAPABILITIES: ConnectorCapabilities = enumflags2::make_bitflags!(Connector
     SupportsTxIsolationReadCommitted |
     SupportsTxIsolationRepeatableRead |
     SupportsTxIsolationSerializable |
-    RowIn
+    RowIn |
+    SupportsFiltersOnRelationsWithoutJoins
 });
 
 const CONSTRAINT_SCOPES: &[ConstraintScope] = &[ConstraintScope::GlobalForeignKey, ConstraintScope::ModelKeyIndex];

psl/psl-core/src/builtin_connectors/postgres_datamodel_connector.rs

@@ -68,7 +68,9 @@ const CAPABILITIES: ConnectorCapabilities = enumflags2::make_bitflags!(Connector
     UpdateReturning |
     RowIn |
     DistinctOn |
-    LateralJoin
+    LateralJoin |
+    DeleteReturning |
+    SupportsFiltersOnRelationsWithoutJoins
 });
 
 pub struct PostgresDatamodelConnector;

psl/psl-core/src/builtin_connectors/sqlite_datamodel_connector.rs

@@ -24,10 +24,12 @@ const CAPABILITIES: ConnectorCapabilities = enumflags2::make_bitflags!(Connector
     SupportsTxIsolationSerializable |
     NativeUpsert |
     FilteredInlineChildNestedToOneDisconnect |
-    RowIn
-    // InsertReturning - While SQLite does support RETURNING, it does not return column information on the way back from the database.
-    // This column type information is necessary in order to preserve consistency for some data types such as int, where values could overflow.
+    RowIn |
+    // InsertReturning, DeleteReturning, UpdateReturning - While SQLite does support RETURNING, it does not return column information on the
+    // way back from the database. This column type information is necessary in order to preserve consistency for some data types such as int,
+    // where values could overflow.
     // Since we care to stay consistent with reads, it is not enabled.
+    SupportsFiltersOnRelationsWithoutJoins
 });
 
 pub struct SqliteDatamodelConnector;

psl/psl-core/src/datamodel_connector/capabilities.rs

@@ -106,6 +106,8 @@ capabilities!(
     RowIn,      // Connector supports (a, b) IN (c, d) expression.
     DistinctOn, // Connector supports DB-level distinct (e.g. postgres)
     LateralJoin,
+    DeleteReturning, // Connector supports deleting records and returning them in one operation.
+    SupportsFiltersOnRelationsWithoutJoins, // Connector supports rendering filters on relation fields without joins.
 );
 
 /// Contains all capabilities that the connector is able to serve.

quaint/src/ast/delete.rs

@@ -6,6 +6,7 @@ use std::borrow::Cow;
 pub struct Delete<'a> {
     pub(crate) table: Table<'a>,
     pub(crate) conditions: Option<ConditionTree<'a>>,
+    pub(crate) returning: Option<Vec<Column<'a>>>,
     pub(crate) comment: Option<Cow<'a, str>>,
 }
 
@@ -35,6 +36,7 @@ impl<'a> Delete<'a> {
         Self {
             table: table.into(),
             conditions: None,
+            returning: None,
             comment: None,
         }
     }
@@ -77,4 +79,25 @@ impl<'a> Delete<'a> {
         self.conditions = Some(conditions.into());
         self
     }
+
+    /// Sets the returned columns.
+    ///
+    /// ```rust
+    /// # use quaint::{ast::*, visitor::{Visitor, Sqlite}};
+    /// # fn main() -> Result<(), quaint::error::Error> {
+    /// let query = Delete::from_table("users").returning(vec!["id"]);
+    /// let (sql, _) = Sqlite::build(query)?;
+    ///
+    /// assert_eq!("DELETE FROM `users` RETURNING \"id\"", sql);
+    /// # Ok(())
+    /// # }
+    #[cfg(any(feature = "postgresql", feature = "mssql", feature = "sqlite"))]
+    pub fn returning<K, I>(mut self, columns: I) -> Self
+    where
+        K: Into<Column<'a>>,
+        I: IntoIterator<Item = K>,
+    {
+        self.returning = Some(columns.into_iter().map(|k| k.into()).collect());
+        self
+    }
 }

quaint/src/connector/queryable.rs

@@ -75,7 +75,7 @@ pub trait Queryable: Send + Sync {
         self.execute(q.into()).await
     }
 
-    /// Execute a `DELETE` query, returning the number of affected rows.
+    /// Execute a `DELETE` query.
     async fn delete(&self, q: Delete<'_>) -> crate::Result<()> {
         self.query(q.into()).await?;
         Ok(())

quaint/src/error.rs

@@ -178,7 +178,7 @@ pub enum ErrorKind {
     #[error("Authentication failed for user {}", user)]
     AuthenticationFailed { user: Name },
 
-    #[error("Query returned no data")]
+    #[error("Query returned no data.")]
     NotFound,
 
     #[error("No such table: {}", table)]

quaint/src/visitor/postgres.rs

@@ -32,6 +32,17 @@ impl<'a> Postgres<'a> {
             _ => self.visit_expression(expr),
         }
     }
+
+    fn visit_returning(&mut self, returning: Option<Vec<Column<'a>>>) -> visitor::Result {
+        if let Some(returning) = returning {
+            if !returning.is_empty() {
+                let values = returning.into_iter().map(|r| r.into()).collect();
+                self.write(" RETURNING ")?;
+                self.visit_columns(values)?;
+            }
+        }
+        Ok(())
+    }
 }
 
 impl<'a> Visitor<'a> for Postgres<'a> {
@@ -328,13 +339,7 @@ impl<'a> Visitor<'a> for Postgres<'a> {
             None => (),
         }
 
-        if let Some(returning) = insert.returning {
-            if !returning.is_empty() {
-                let values = returning.into_iter().map(|r| r.into()).collect();
-                self.write(" RETURNING ")?;
-                self.visit_columns(values)?;
-            }
-        };
+        self.visit_returning(insert.returning)?;
 
         if let Some(comment) = insert.comment {
             self.write(" ")?;
@@ -724,6 +729,25 @@ impl<'a> Visitor<'a> for Postgres<'a> {
 
         Ok(())
     }
+
+    fn visit_delete(&mut self, delete: Delete<'a>) -> visitor::Result {
+        self.write("DELETE FROM ")?;
+        self.visit_table(delete.table, true)?;
+
+        if let Some(conditions) = delete.conditions {
+            self.write(" WHERE ")?;
+            self.visit_conditions(conditions)?;
+        }
+
+        self.visit_returning(delete.returning)?;
+
+        if let Some(comment) = delete.comment {
+            self.write(" ")?;
+            self.visit_comment(comment)?;
+        }
+
+        Ok(())
+    }
 }
 
 #[cfg(test)]

quaint/src/visitor/sqlite.rs

@@ -15,6 +15,29 @@ pub struct Sqlite<'a> {
     parameters: Vec<Value<'a>>,
 }
 
+impl<'a> Sqlite<'a> {
+    fn returning(&mut self, returning: Option<Vec<Column<'a>>>) -> visitor::Result {
+        if let Some(returning) = returning {
+            if !returning.is_empty() {
+                let values_len = returning.len();
+                self.write(" RETURNING ")?;
+
+                for (i, column) in returning.into_iter().enumerate() {
+                    // Workaround for SQLite parsing bug
+                    // https://sqlite.org/forum/info/6c141f151fa5c444db257eb4d95c302b70bfe5515901cf987e83ed8ebd434c49?t=h
+                    self.surround_with_backticks(&column.name)?;
+                    self.write(" AS ")?;
+                    self.surround_with_backticks(&column.name)?;
+                    if i < (values_len - 1) {
+                        self.write(", ")?;
+                    }
+                }
+            }
+        }
+        Ok(())
+    }
+}
+
 impl<'a> Sqlite<'a> {
     fn visit_order_by(&mut self, direction: &str, value: Expression<'a>) -> visitor::Result {
         self.visit_expression(value)?;
@@ -198,22 +221,7 @@ impl<'a> Visitor<'a> for Sqlite<'a> {
             self.visit_upsert(update)?;
         }
 
-        if let Some(returning) = insert.returning {
-            if !returning.is_empty() {
-                let values_len = returning.len();
-                self.write(" RETURNING ")?;
-
-                for (i, column) in returning.into_iter().enumerate() {
-                    //yay https://sqlite.org/forum/info/6c141f151fa5c444db257eb4d95c302b70bfe5515901cf987e83ed8ebd434c49?t=h
-                    self.surround_with_backticks(&column.name)?;
-                    self.write(" AS ")?;
-                    self.surround_with_backticks(&column.name)?;
-                    if i < (values_len - 1) {
-                        self.write(", ")?;
-                    }
-                }
-            }
-        }
+        self.returning(insert.returning)?;
 
         if let Some(comment) = insert.comment {
             self.write("; ")?;
@@ -392,6 +400,25 @@ impl<'a> Visitor<'a> for Sqlite<'a> {
 
         Ok(())
     }
+
+    fn visit_delete(&mut self, delete: Delete<'a>) -> visitor::Result {
+        self.write("DELETE FROM ")?;
+        self.visit_table(delete.table, true)?;
+
+        if let Some(conditions) = delete.conditions {
+            self.write(" WHERE ")?;
+            self.visit_conditions(conditions)?;
+        }
+
+        self.returning(delete.returning)?;
+
+        if let Some(comment) = delete.comment {
+            self.write(" ")?;
+            self.visit_comment(comment)?;
+        }
+
+        Ok(())
+    }
 }
 
 #[cfg(test)]

query-engine/connector-test-kit-rs/query-engine-tests/tests/new/ref_actions/on_delete/set_null.rs

@@ -50,7 +50,7 @@ mod one2one_opt {
             uniq    Int? @unique
             child   Child?
           }
-          
+
           model Child {
             #id(childId, Int, @id)
             childUniq       Int? @unique
@@ -90,15 +90,15 @@ mod one2one_opt {
             b_id Int? @unique
             b B?
           }
-          
+
           model B {
             #id(id, Int, @id)
             a_id Int? @unique
             a A? @relation(fields: [a_id], references: [b_id], onDelete: SetNull)
 
             c C?
           }
-          
+
           model C {
             #id(id, Int, @id)
             b_id Int? @unique
@@ -160,15 +160,15 @@ mod one2one_opt {
                 b_id Int? @unique
                 b B?
               }
-              
+
               model B {
                 #id(id, Int, @id)
                 a_id Int? @unique
                 a A? @relation(fields: [a_id], references: [b_id], onDelete: SetNull)
 
                 c C?
               }
-              
+
               model C {
                 #id(id, Int, @id)
                 b_id Int? @unique
@@ -225,15 +225,15 @@ mod one2one_opt {
               b_id Int? @unique
               b B?
             }
-            
+
             model B {
               #id(id, Int, @id)
               a_id Int? @unique
               a A? @relation(fields: [a_id], references: [b_id], onDelete: SetNull)
 
               c C?
             }
-            
+
             model C {
               #id(id, Int, @id)
               b_id Int? @unique
@@ -246,7 +246,7 @@ mod one2one_opt {
     }
 
     // SET_NULL should also apply to child relations sharing a common fk
-    #[connector_test(schema(one2one2one_opt_set_null_cascade))]
+    #[connector_test(schema(one2one2one_opt_set_null_cascade), exclude_features("relationJoins"))]
     async fn delete_parent_set_null_cascade(runner: Runner) -> TestResult<()> {
         insta::assert_snapshot!(
           run_query!(runner, r#"mutation {
@@ -340,21 +340,21 @@ mod one2many_opt {
         let schema = indoc! {
             r#"model Main {
             #id(id, Int, @id)
-          
+
             alice   Alice?  @relation(fields: [aliceId], references: [id], onDelete: SetNull, onUpdate: Cascade)
             aliceId Int?
-          
+
             bob Bob?
           }
-          
+
           model Alice {
             #id(id, Int, @id)
             manyMains Main[]
           }
-          
+
           model Bob {
             #id(id, Int, @id)
-          
+
             main   Main   @relation(fields: [mainId], references: [id], onDelete: Cascade, onUpdate: Cascade)
             mainId Int @unique
           }"#

query-engine/connector-test-kit-rs/query-engine-tests/tests/queries/batch/select_one_singular.rs

@@ -410,7 +410,7 @@ mod singular_batch {
         .await?;
         insta::assert_snapshot!(
           res.to_string(),
-          @r###"{"batchResult":[{"data":{"findUniqueTestModel":{"id":2}}},{"errors":[{"error":"Error occurred during query execution:\nConnectorError(ConnectorError { user_facing_error: Some(KnownError { message: \"An operation failed because it depends on one or more records that were required but not found. Expected a record, found none.\", meta: Object {\"cause\": String(\"Expected a record, found none.\")}, error_code: \"P2025\" }), kind: RecordDoesNotExist, transient: false })","user_facing_error":{"is_panic":false,"message":"An operation failed because it depends on one or more records that were required but not found. Expected a record, found none.","meta":{"cause":"Expected a record, found none."},"error_code":"P2025"}}]}]}"###
+          @r###"{"batchResult":[{"data":{"findUniqueTestModel":{"id":2}}},{"errors":[{"error":"KnownError { message: \"An operation failed because it depends on one or more records that were required but not found. Expected a record, found none.\", meta: Object {\"cause\": String(\"Expected a record, found none.\")}, error_code: \"P2025\" }","user_facing_error":{"is_panic":false,"message":"An operation failed because it depends on one or more records that were required but not found. Expected a record, found none.","meta":{"cause":"Expected a record, found none."},"error_code":"P2025"}}]}]}"###
         );
         assert!(!compact_doc.is_compact());