dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.59k stars 3.14k forks source link

Support ExecuteUpdate when the value selector is a collection #32494

Open roji opened 8 months ago

roji commented 8 months ago

The following query fails:

await db.Test
    .ExecuteUpdateAsync(c => c.SetProperty(x => x.TextParts, x => x.TextParts.Concat(arr)));

The 1st reason for the failure is #32493; this can be worked around by extracting the lambda to a preceding Select:

await ctx.Blogs
    .Select(b => new { Blog = b, NewTextParts = b.TextParts.Concat(arr) })
    .ExecuteUpdateAsync(c => c.SetProperty(x => x.Blog.TextParts, x => x.NewTextParts));

At this point we run against other failures:

  1. If the type of the property being set (Blog.TextParts) and the type coming out of the value lambda is different, we attempt to apply a Convert node (code) which can fail (e.g. array property, List coming out of the selector - at least in PG).
  2. More importantly, the expression translated for NewTextParts in the anonymous object above is a CollectionResultExpression - this is what Select() returns when the thing being selected is an enumerable. We then attempt to (re-)translate that and fail.
    • We should be able to make this work.
    • But more generally, this raises the question of why we attempt to translate twice - once when translating the Select, and another time when translating the ExecuteUpdate's value selector lambda, into which we remap the lambda parameter (inserting a CollectionResultExpression into the lambda).
    • This can be another argument against out general remapping approach. Instead of doing a pre-pass the remap the lambda parameter, and then having to skip re-translating the thing we remapped, if SQL translator simply encountered a parameter it could process it accordingly.

Originally raised by @srasch in https://github.com/npgsql/efcore.pg/issues/3001

roji commented 8 months ago

Note: for this to actually work, we'd need to support transforming a relational rowset back to a primitive collection (tracked by #33792). For example, on SQL Server, the result of translating the above selector is a CollectionResultCollectiom, whose ProjectionBindingExpression is a ShapedQueryExpression doing a UNION over the column and the parameter:

Projection Mapping:
    EmptyProjectionMember -> t0.value
SELECT 1
FROM (
    SELECT t.value
    FROM OPENJSON(b.TextParts) WITH (value nvarchar(max) '') AS t
    UNION ALL
    SELECT a.value
    FROM OPENJSON(@__arr_0) AS a
) AS t0

On PostgreSQL we get a nicer, specialized translation:

Projection Mapping:
    EmptyProjectionMember -> t.value
SELECT 1
FROM unnest(b.TextParts + @__arr_0) WITH ORDINALITY AS t(value)

But that's still a rowset, whereas ExecuteUpdate requires the (non-rowset) primitive collection to be assigned to the column.

For SQL Server this would mean using FOR JSON, though I'm not seeing an option to return a simple array of primitives, only an array of objects (confirm):

SELECT * FROM (SELECT 1 UNION SELECT 2 UNION SELECT 3) AS f(v) FOR JSON AUTO;
-- Returns: [{"v":1},{"v":2},{"v":3}], but we want [1, 2, 3]

For PG, for the very specific above example, we'd simply unwrap the unnest. But when other operators are used, we get a real subquery like with SQL Server, so we need to use array_agg to transform that back from a rowset to an array.