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.78k stars 3.19k forks source link

Sqlite: "target object/alias may not appear in FROM clause" in ExecuteUpdate #33947

Open roji opened 5 months ago

roji commented 5 months ago

For example, test Replace_ColumnExpression_in_column_setter:

await ss.Set<Owner>()
    .SelectMany(e => e.OwnedCollections)
    .ExecuteUpdateAsync(s => s.SetProperty(o => o.Value, "SomeValue"))

... produces the following SQL:

UPDATE "OwnedCollection" AS "o0"
SET "Value" = 'SomeValue'
FROM "Owner" AS "o"
INNER JOIN "OwnedCollection" AS "o0" ON "o"."Id" = "o0"."OwnerId"

... which fails on SQLite with "target object/alias may not appear in FROM clause: o0" (SQL Server produces a slightly different SQL that does work).

Note that this SQL may be optimizable, removing the Owner altogether (see #33946), at which point the simplified SQL would work; but this doesn't necessarily mean the problem wouldn't exist in some other query form.

Originally flagged by @ajcvickers in #33937

roji commented 2 months ago

Same problem in PG: https://github.com/npgsql/efcore.pg/issues/3253

ChrisJollyAU commented 2 months ago

From my comment over at npgsl/efcore.pg#3253 this can be fixed by a simple change in one line if (ReferenceEquals(updateExpression.Table, joinExpression?.Table ?? table)) becomes if (updateExpression.Table.Alias == (joinExpression?.Table.Alias ?? table.Alias))

This test ends up with updateExpression.Table not being the same instance as joinExpression?.Table even though they may be pointing at the same table with the same alias

I will do a PR for this shortly

ChrisJollyAU commented 2 months ago

@roji Was just checking things before I do the PR and found another ReferenceEquals in the same function.

 if (selectExpression is
     {
         Offset: null,
         Limit: null,
         Having: null,
         Orderings: [],
         GroupBy: [],
         Projection: [],
     }
     && (selectExpression.Tables.Count == 1
         || !ReferenceEquals(selectExpression.Tables[0], updateExpression.Table)
         || selectExpression.Tables[1] is InnerJoinExpression
         || selectExpression.Tables[1] is CrossJoinExpression))
 {

Is this meant to be checking on the exact same instance, or is it the case of as long as it refers to the same table irrespective or whether or not it is the same instance

Actually I'm not sure if that line is fully needed. You can comment it out and all current tests pass - both for sqlite and on efcore.pg (SQL Server uses its own override that doesnt have those last couple of lines). Given that all tests pass means that it isnt the sole statement that is causing that section to be true. There is always another expression (table count, inner join, cross join) being true. Otherwise it would drop down to the InvalidOperationException

Any thoughts on that part?