Convert to PostgresDeleteExpression

To maintain clean separation between translation and SQL generation

src/EFCore.PG/Query/Expressions/Internal/PostgresDeleteExpression.cs

@@ -0,0 +1,73 @@
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal;
+
+public sealed class PostgresDeleteExpression : Expression, IPrintableExpression
+{
+    /// <summary>
+    ///     The tables that rows are to be deleted from.
+    /// </summary>
+    public TableExpression Table { get; }
+
+    /// <summary>
+    ///     Additional tables which can be referenced in the predicate.
+    /// </summary>
+    public IReadOnlyList<TableExpression> FromItems { get; }
+
+    /// <summary>
+    ///     The WHERE predicate for the DELETE.
+    /// </summary>
+    public SqlExpression? Predicate { get; }
+
+    public PostgresDeleteExpression(TableExpression table, IReadOnlyList<TableExpression> fromItems, SqlExpression? predicate)
+        => (Table, FromItems, Predicate) = (table, fromItems, predicate);
+
+    /// <inheritdoc />
+    public override Type Type
+        => typeof(object);
+
+    /// <inheritdoc />
+    public override ExpressionType NodeType
+        => ExpressionType.Extension;
+
+    protected override Expression VisitChildren(ExpressionVisitor visitor)
+        => Predicate is null
+            ? this
+            : Update((SqlExpression?)visitor.Visit(Predicate));
+
+    public PostgresDeleteExpression Update(SqlExpression? predicate)
+        => predicate == Predicate
+            ? this
+            : new PostgresDeleteExpression(Table, FromItems, predicate);
+
+    public void Print(ExpressionPrinter expressionPrinter)
+    {
+        expressionPrinter.AppendLine($"DELETE FROM {Table.Name} AS {Table.Alias}");
+
+        if (FromItems.Count > 0)
+        {
+            expressionPrinter
+                .Append("USING ")
+                .Append(string.Join(", ", FromItems.Select(fi => $"{fi.Name} AS {fi.Alias}")));
+        }
+
+        if (Predicate is not null)
+        {
+            expressionPrinter.Append("WHERE ");
+            expressionPrinter.Visit(Predicate);
+        }
+    }
+
+    /// <inheritdoc />
+    public override bool Equals(object? obj)
+        => obj != null
+            && (ReferenceEquals(this, obj)
+                || obj is PostgresDeleteExpression pgDeleteExpression
+                && Equals(pgDeleteExpression));
+
+    private bool Equals(PostgresDeleteExpression pgDeleteExpression)
+        => Table == pgDeleteExpression.Table
+            && FromItems.SequenceEqual(pgDeleteExpression.FromItems)
+            && (Predicate is null ? pgDeleteExpression.Predicate is null : Predicate.Equals(pgDeleteExpression.Predicate));
+
+    /// <inheritdoc />
+    public override int GetHashCode() => Table.GetHashCode();
+}

src/EFCore.PG/Query/Internal/NonQueryConvertingExpressionVisitor.cs

@@ -0,0 +1,82 @@
+using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal;
+
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.Internal;
+
+/// <summary>
+///     Converts the relational <see cref="NonQueryExpression" /> into a PG-specific <see cref="PostgresDeleteExpression" />, which
+///     precisely models a DELETE statement in PostgreSQL. This is done to handle the PG-specific USING syntax for table joining.
+/// </summary>
+public class NonQueryConvertingExpressionVisitor : ExpressionVisitor
+{
+    protected override Expression VisitExtension(Expression node)
+        => node switch
+        {
+            DeleteExpression deleteExpression => VisitDelete(deleteExpression),
+
+            _ => base.VisitExtension(node)
+        };
+
+    protected virtual Expression VisitDelete(DeleteExpression deleteExpression)
+    {
+        var selectExpression = deleteExpression.SelectExpression;
+
+        if (selectExpression.Offset != null
+            || selectExpression.Limit != null
+            || selectExpression.Having != null
+            || selectExpression.Orderings.Count > 0
+            || selectExpression.GroupBy.Count > 0
+            || selectExpression.Projection.Count > 0)
+        {
+            throw new InvalidOperationException(RelationalStrings.BulkOperationWithUnsupportedOperatorInSqlGeneration);
+        }
+
+        var fromItems = new List<TableExpression>();
+        SqlExpression? joinPredicates = null;
+
+        // The SelectExpression also contains the target table being modified (same as deleteExpression.Table).
+        // If it has additional inner joins, use the PostgreSQL-specific USING syntax to express the join.
+        // Note that the non-join TableExpression isn't necessary the target table - through projection the last table being
+        // joined may be the one being modified.
+        foreach (var tableBase in selectExpression.Tables)
+        {
+            switch (tableBase)
+            {
+                case TableExpression tableExpression:
+                    if (tableExpression != deleteExpression.Table)
+                    {
+                        fromItems.Add(tableExpression);
+                    }
+
+                    break;
+
+                case InnerJoinExpression { Table: TableExpression tableExpression } innerJoinExpression:
+                    if (tableExpression != deleteExpression.Table)
+                    {
+                        fromItems.Add(tableExpression);
+                    }
+
+                    joinPredicates = joinPredicates is null
+                        ? innerJoinExpression.JoinPredicate
+                        : new SqlBinaryExpression(
+                            ExpressionType.AndAlso, joinPredicates, innerJoinExpression.JoinPredicate, typeof(bool),
+                            innerJoinExpression.JoinPredicate.TypeMapping);
+                    break;
+
+                default:
+                    throw new InvalidOperationException(RelationalStrings.BulkOperationWithUnsupportedOperatorInSqlGeneration);
+            }
+        }
+
+        // Combine the join predicates (if any) before the user-provided predicate
+        var predicate = (joinPredicates, selectExpression.Predicate) switch
+        {
+            (null, not null) => selectExpression.Predicate,
+            (not null, null) => joinPredicates,
+            (null, null) => null,
+            (not null, not null) => new SqlBinaryExpression(
+                ExpressionType.AndAlso, joinPredicates, selectExpression.Predicate, typeof(bool), joinPredicates.TypeMapping)
+        };
+
+        return new PostgresDeleteExpression(deleteExpression.Table, fromItems, predicate);
+    }
+}

src/EFCore.PG/Query/Internal/NpgsqlParameterBasedSqlProcessor.cs

@@ -9,6 +9,18 @@ public NpgsqlParameterBasedSqlProcessor(
     {
     }
 
+    public override Expression Optimize(
+        Expression queryExpression,
+        IReadOnlyDictionary<string, object?> parametersValues,
+        out bool canCache)
+    {
+        queryExpression = base.Optimize(queryExpression, parametersValues, out canCache);
+
+        queryExpression = new NonQueryConvertingExpressionVisitor().Visit(queryExpression);
+
+        return queryExpression;
+    }
+
     /// <inheritdoc />
     protected override Expression ProcessSqlNullability(
         Expression selectExpression, IReadOnlyDictionary<string, object?> parametersValues, out bool canCache)

src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs

@@ -44,6 +44,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
             PostgresAnyExpression anyExpression                     => VisitArrayAny(anyExpression),
             PostgresArrayIndexExpression arrayIndexExpression       => VisitArrayIndex(arrayIndexExpression),
             PostgresBinaryExpression binaryExpression               => VisitPostgresBinary(binaryExpression),
+            PostgresDeleteExpression deleteExpression               => VisitPostgresDelete(deleteExpression),
             PostgresFunctionExpression functionExpression           => VisitPostgresFunction(functionExpression),
             PostgresILikeExpression iLikeExpression                 => VisitILike(iLikeExpression),
             PostgresJsonTraversalExpression jsonTraversalExpression => VisitJsonPathTraversal(jsonTraversalExpression),
@@ -206,91 +207,37 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression binary)
         }
     }
 
-    protected override Expression VisitDelete(DeleteExpression deleteExpression)
-    {
-        var selectExpression = deleteExpression.SelectExpression;
-
-        if (selectExpression.Offset == null
-            && selectExpression.Limit == null
-            && selectExpression.Having == null
-            && selectExpression.Orderings.Count == 0
-            && selectExpression.GroupBy.Count == 0
-            && selectExpression.Projection.Count == 0)
+    protected override Expression VisitCommandRootExpression(Expression rootExpression)
+        => rootExpression switch
         {
-            Sql.Append("DELETE FROM ");
-            Visit(deleteExpression.Table);
-
-            SqlExpression? joinPredicates = null;
+            PostgresDeleteExpression pgDeleteExpression => VisitPostgresDelete(pgDeleteExpression),
 
-            // The SelectExpression also contains the target table being modified (same as deleteExpression.Table).
-            // If it has additional inner joins, use the PostgreSQL-specific USING syntax to express the join.
-            // Note that the non-join TableExpression isn't necessary the target table - through projection the last table being
-            // joined may be the one being modified.
-            var first = true;
-
-            foreach (var tableBase in selectExpression.Tables)
-            {
-                TableExpression table;
-
-                switch (tableBase)
-                {
-                    case TableExpression tableExpression:
-                        table = tableExpression;
-                        break;
-
-                    case InnerJoinExpression { Table: TableExpression tableExpression } innerJoinExpression:
-                        table = tableExpression;
-
-                        joinPredicates = joinPredicates is null
-                            ? innerJoinExpression.JoinPredicate
-                            : new SqlBinaryExpression(
-                                ExpressionType.AndAlso, joinPredicates, innerJoinExpression.JoinPredicate, typeof(bool),
-                                innerJoinExpression.JoinPredicate.TypeMapping);
-                        break;
-
-                    default:
-                        throw new InvalidOperationException(RelationalStrings.BulkOperationWithUnsupportedOperatorInSqlGeneration);
-                }
+            _ => base.VisitCommandRootExpression(rootExpression)
+        };
 
-                // Add the table name and alias to the USING list, unless it's the target table (already generated above after DELETE FROM)
-                if (table != deleteExpression.Table)
-                {
-                    if (first)
-                    {
-                        Sql.AppendLine().Append("USING ");
-
-                        first = false;
-                    }
-                    else
-                    {
-                        Sql.Append(", ");
-                    }
-
-                    Visit(table);
-                }
-            }
+    // NonQueryConvertingExpressionVisitor converts the relational DeleteExpression to PostgresDeleteExpression, so we should never
+    // get here
+    protected override Expression VisitDelete(DeleteExpression deleteExpression)
+        => throw new InvalidOperationException("Inconceivable!");
 
-            // Combine the join predicates (if any) before the user-provided predicate
-            var predicate = (joinPredicates, selectExpression.Predicate) switch
-            {
-                (null, not null) => selectExpression.Predicate,
-                (not null, null) => joinPredicates,
-                (null, null) => null,
-                (not null, not null) => new SqlBinaryExpression(
-                    ExpressionType.AndAlso, joinPredicates, selectExpression.Predicate, typeof(bool), joinPredicates.TypeMapping)
-            };
-
-            if (predicate is not null)
-            {
-                Sql.AppendLine().Append("WHERE ");
+    protected virtual Expression VisitPostgresDelete(PostgresDeleteExpression pgDeleteExpression)
+    {
+        Sql.Append("DELETE FROM ");
+        Visit(pgDeleteExpression.Table);
 
-                Visit(predicate);
-            }
+        if (pgDeleteExpression.FromItems.Count > 0)
+        {
+            Sql.AppendLine().Append("USING ");
+            GenerateList(pgDeleteExpression.FromItems, t => Visit(t), sql => sql.Append(", "));
+        }
 
-            return deleteExpression;
+        if (pgDeleteExpression.Predicate != null)
+        {
+            Sql.AppendLine().Append("WHERE ");
+            Visit(pgDeleteExpression.Predicate);
         }
 
-        throw new InvalidOperationException(RelationalStrings.BulkOperationWithUnsupportedOperatorInSqlGeneration);
+        return pgDeleteExpression;
     }
 
     protected virtual Expression VisitPostgresNewArray(PostgresNewArrayExpression postgresNewArrayExpression)
@@ -843,4 +790,22 @@ public virtual Expression VisitPostgresFunction(PostgresFunctionExpression e)
 
     private static bool RequiresBrackets(SqlExpression expression)
         => expression is SqlBinaryExpression || expression is LikeExpression || expression is PostgresBinaryExpression;
+
+    private void GenerateList<T>(
+        IReadOnlyList<T> items,
+        Action<T> generationAction,
+        Action<IRelationalCommandBuilder>? joinAction = null)
+    {
+        joinAction ??= (isb => isb.Append(", "));
+
+        for (var i = 0; i < items.Count; i++)
+        {
+            if (i > 0)
+            {
+                joinAction(Sql);
+            }
+
+            generationAction(items[i]);
+        }
+    }
 }