Skip to content

Commit e038a97

Browse files
rojiCopilot
roji
and
Copilot
committed
Fix SQLite ExecuteUpdate with navigation properties (#38010)
Fixes #38010 Fixes #31402 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 713aaf0 commit e038a97

7 files changed

Lines changed: 271 additions & 20 deletions

File tree

‎src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,97 @@ protected SqliteQueryableMethodTranslatingExpressionVisitor(
8484
protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVisitor()
8585
=> new SqliteQueryableMethodTranslatingExpressionVisitor(this);
8686

87+
/// <summary>
88+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
89+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
90+
/// any release. You should only use it directly in your code with extreme caution and knowing that
91+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
92+
/// </summary>
93+
protected override UpdateExpression TranslateExecuteUpdate(ShapedQueryExpression source, IReadOnlyList<ExecuteUpdateSetter> setters)
94+
{
95+
var updateExpression = base.TranslateExecuteUpdate(source, setters);
96+
97+
// SQLite doesn't support referencing the UPDATE target table alias in JOIN ON clauses.
98+
// Detect such joins and replace them with cross joins, lifting their predicates to WHERE.
99+
var selectExpression = updateExpression.SelectExpression;
100+
if (selectExpression.Tables.Count <= 1)
101+
{
102+
return updateExpression;
103+
}
104+
105+
var targetAlias = updateExpression.Table.Alias;
106+
var tables = selectExpression.Tables.ToList();
107+
var predicate = selectExpression.Predicate;
108+
var changed = false;
109+
110+
for (var i = 0; i < tables.Count; i++)
111+
{
112+
if (tables[i] is InnerJoinExpression innerJoin
113+
&& ContainsTargetReference(innerJoin.JoinPredicate, targetAlias))
114+
{
115+
predicate = predicate == null
116+
? innerJoin.JoinPredicate
117+
: _sqlExpressionFactory.AndAlso(predicate, innerJoin.JoinPredicate);
118+
119+
tables[i] = new CrossJoinExpression(innerJoin.Table);
120+
changed = true;
121+
}
122+
}
123+
124+
if (!changed)
125+
{
126+
return updateExpression;
127+
}
128+
129+
#pragma warning disable EF1001 // Internal EF Core API usage.
130+
var newSelect = new SelectExpression(
131+
selectExpression.Alias,
132+
tables,
133+
predicate,
134+
selectExpression.GroupBy,
135+
selectExpression.Having,
136+
selectExpression.Projection,
137+
selectExpression.IsDistinct,
138+
selectExpression.Orderings,
139+
selectExpression.Offset,
140+
selectExpression.Limit,
141+
_sqlAliasManager,
142+
(IReadOnlySet<string>)selectExpression.Tags,
143+
selectExpression.GetAnnotations().ToDictionary(a => a.Name));
144+
#pragma warning restore EF1001 // Internal EF Core API usage.
145+
146+
return updateExpression.Update(newSelect, updateExpression.ColumnValueSetters);
147+
148+
static bool ContainsTargetReference(SqlExpression expression, string targetAlias)
149+
{
150+
var visitor = new TargetReferenceCheckingVisitor(targetAlias);
151+
visitor.Visit(expression);
152+
return visitor.Found;
153+
}
154+
}
155+
156+
private sealed class TargetReferenceCheckingVisitor(string targetAlias) : ExpressionVisitor
157+
{
158+
public bool Found { get; private set; }
159+
160+
[return: NotNullIfNotNull(nameof(node))]
161+
public override Expression? Visit(Expression? node)
162+
{
163+
if (Found)
164+
{
165+
return node;
166+
}
167+
168+
if (node is ColumnExpression col && col.TableAlias == targetAlias)
169+
{
170+
Found = true;
171+
return node;
172+
}
173+
174+
return base.Visit(node);
175+
}
176+
}
177+
87178
/// <summary>
88179
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
89180
/// the same compatibility standards as public APIs. It may be changed or removed without notice in

‎test/EFCore.Relational.Specification.Tests/Query/Associations/Navigations/NavigationsBulkUpdateRelationalTestBase.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.EntityFrameworkCore.Query.Associations.Navigations;
5+
6+
public abstract class NavigationsBulkUpdateRelationalTestBase<TFixture> : AssociationsBulkUpdateTestBase<TFixture>
7+
where TFixture : NavigationsRelationalFixtureBase, new()
8+
{
9+
public NavigationsBulkUpdateRelationalTestBase(TFixture fixture, ITestOutputHelper testOutputHelper)
10+
: base(fixture)
11+
{
12+
fixture.TestSqlLoggerFactory.Clear();
13+
fixture.TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
14+
}
15+
16+
protected void AssertSql(params string[] expected)
17+
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
18+
19+
protected void AssertExecuteUpdateSql(params string[] expected)
20+
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected, forUpdate: true);
21+
}

‎test/EFCore.Sqlite.FunctionalTests/BulkUpdates/Inheritance/TPTFiltersInheritanceBulkUpdatesSqliteTest.cs‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using Microsoft.Data.Sqlite;
5-
64
namespace Microsoft.EntityFrameworkCore.BulkUpdates.Inheritance;
75

86
#nullable disable
@@ -113,9 +111,27 @@ public override async Task Update_base_type(bool async)
113111
""");
114112
}
115113

116-
// #31402
117-
public override Task Update_base_type_with_OfType(bool async)
118-
=> Assert.ThrowsAsync<SqliteException>(() => base.Update_base_property_on_derived_type(async));
114+
public override async Task Update_base_type_with_OfType(bool async)
115+
{
116+
await base.Update_base_type_with_OfType(async);
117+
118+
AssertExecuteUpdateSql(
119+
"""
120+
@p='NewBird' (Size = 7)
121+
122+
UPDATE "Animals" AS "a0"
123+
SET "Name" = @p
124+
FROM "Birds" AS "b"
125+
CROSS JOIN "Kiwi" AS "k0"
126+
CROSS JOIN (
127+
SELECT "a"."Id"
128+
FROM "Animals" AS "a"
129+
LEFT JOIN "Kiwi" AS "k" ON "a"."Id" = "k"."Id"
130+
WHERE "a"."CountryId" = 1 AND "k"."Id" IS NOT NULL
131+
) AS "s"
132+
WHERE "a0"."Id" = "b"."Id" AND "a0"."Id" = "k0"."Id" AND "a0"."Id" = "s"."Id"
133+
""");
134+
}
119135

120136
public override async Task Update_where_hierarchy_subquery(bool async)
121137
{
@@ -124,9 +140,21 @@ public override async Task Update_where_hierarchy_subquery(bool async)
124140
AssertExecuteUpdateSql();
125141
}
126142

127-
// #31402
128-
public override Task Update_base_property_on_derived_type(bool async)
129-
=> Assert.ThrowsAsync<SqliteException>(() => base.Update_base_property_on_derived_type(async));
143+
public override async Task Update_base_property_on_derived_type(bool async)
144+
{
145+
await base.Update_base_property_on_derived_type(async);
146+
147+
AssertExecuteUpdateSql(
148+
"""
149+
@p='SomeOtherKiwi' (Size = 13)
150+
151+
UPDATE "Animals" AS "a"
152+
SET "Name" = @p
153+
FROM "Birds" AS "b"
154+
CROSS JOIN "Kiwi" AS "k"
155+
WHERE "a"."CountryId" = 1 AND "a"."Id" = "b"."Id" AND "a"."Id" = "k"."Id"
156+
""");
157+
}
130158

131159
public override async Task Update_derived_property_on_derived_type(bool async)
132160
{
@@ -140,7 +168,7 @@ public override async Task Update_derived_property_on_derived_type(bool async)
140168
SET "FoundOn" = @p
141169
FROM "Animals" AS "a"
142170
INNER JOIN "Birds" AS "b" ON "a"."Id" = "b"."Id"
143-
WHERE "a"."Id" = "k"."Id" AND "a"."CountryId" = 1
171+
WHERE "a"."CountryId" = 1 AND "a"."Id" = "k"."Id"
144172
""");
145173
}
146174

‎test/EFCore.Sqlite.FunctionalTests/BulkUpdates/Inheritance/TPTInheritanceBulkUpdatesSqliteTest.cs‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using Microsoft.Data.Sqlite;
5-
64
namespace Microsoft.EntityFrameworkCore.BulkUpdates.Inheritance;
75

86
#nullable disable
@@ -98,9 +96,27 @@ public override async Task Update_base_type(bool async)
9896
""");
9997
}
10098

101-
// #31402
102-
public override Task Update_base_type_with_OfType(bool async)
103-
=> Assert.ThrowsAsync<SqliteException>(() => base.Update_base_property_on_derived_type(async));
99+
public override async Task Update_base_type_with_OfType(bool async)
100+
{
101+
await base.Update_base_type_with_OfType(async);
102+
103+
AssertExecuteUpdateSql(
104+
"""
105+
@p='NewBird' (Size = 7)
106+
107+
UPDATE "Animals" AS "a0"
108+
SET "Name" = @p
109+
FROM "Birds" AS "b"
110+
CROSS JOIN "Kiwi" AS "k0"
111+
CROSS JOIN (
112+
SELECT "a"."Id"
113+
FROM "Animals" AS "a"
114+
LEFT JOIN "Kiwi" AS "k" ON "a"."Id" = "k"."Id"
115+
WHERE "k"."Id" IS NOT NULL
116+
) AS "s"
117+
WHERE "a0"."Id" = "b"."Id" AND "a0"."Id" = "k0"."Id" AND "a0"."Id" = "s"."Id"
118+
""");
119+
}
104120

105121
public override async Task Update_where_hierarchy_subquery(bool async)
106122
{
@@ -109,9 +125,21 @@ public override async Task Update_where_hierarchy_subquery(bool async)
109125
AssertExecuteUpdateSql();
110126
}
111127

112-
// #31402
113-
public override Task Update_base_property_on_derived_type(bool async)
114-
=> Assert.ThrowsAsync<SqliteException>(() => base.Update_base_property_on_derived_type(async));
128+
public override async Task Update_base_property_on_derived_type(bool async)
129+
{
130+
await base.Update_base_property_on_derived_type(async);
131+
132+
AssertExecuteUpdateSql(
133+
"""
134+
@p='SomeOtherKiwi' (Size = 13)
135+
136+
UPDATE "Animals" AS "a"
137+
SET "Name" = @p
138+
FROM "Birds" AS "b"
139+
CROSS JOIN "Kiwi" AS "k"
140+
WHERE "a"."Id" = "b"."Id" AND "a"."Id" = "k"."Id"
141+
""");
142+
}
115143

116144
public override async Task Update_derived_property_on_derived_type(bool async)
117145
{

‎test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqliteTest.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ public override async Task Update_main_table_in_entity_with_entity_splitting(boo
133133
"""
134134
UPDATE "Blogs" AS "b"
135135
SET "CreationTimestamp" = '2020-01-01 00:00:00'
136+
FROM "BlogsPart1" AS "b0"
137+
WHERE "b"."Id" = "b0"."Id"
136138
""");
137139
}
138140

‎test/EFCore.Sqlite.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqliteTest.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,7 @@ public override async Task Update_Where_using_navigation_2_set_constant(bool asy
10721072
SET "Quantity" = CAST(@p AS INTEGER)
10731073
FROM "Orders" AS "o0"
10741074
LEFT JOIN "Customers" AS "c" ON "o0"."CustomerID" = "c"."CustomerID"
1075-
WHERE "o"."OrderID" = "o0"."OrderID" AND "c"."City" = 'Seattle'
1075+
WHERE "c"."City" = 'Seattle' AND "o"."OrderID" = "o0"."OrderID"
10761076
""");
10771077
}
10781078

@@ -1085,7 +1085,7 @@ public override async Task Update_Where_SelectMany_set_null(bool async)
10851085
UPDATE "Orders" AS "o"
10861086
SET "OrderDate" = NULL
10871087
FROM "Customers" AS "c"
1088-
WHERE "c"."CustomerID" = "o"."CustomerID" AND "c"."CustomerID" LIKE 'F%'
1088+
WHERE "c"."CustomerID" LIKE 'F%' AND "c"."CustomerID" = "o"."CustomerID"
10891089
""");
10901090
}
10911091

@@ -1304,7 +1304,7 @@ public override async Task Update_with_join_set_constant(bool async)
13041304
FROM "Orders" AS "o"
13051305
WHERE "o"."OrderID" < 10300
13061306
) AS "o0"
1307-
WHERE "c"."CustomerID" = "o0"."CustomerID" AND "c"."CustomerID" LIKE 'F%'
1307+
WHERE "c"."CustomerID" LIKE 'F%' AND "c"."CustomerID" = "o0"."CustomerID"
13081308
""");
13091309
}
13101310

‎test/EFCore.Sqlite.FunctionalTests/Query/Associations/Navigations/NavigationsBulkUpdateSqliteTest.cs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Microsoft.Data.Sqlite;
5+
6+
namespace Microsoft.EntityFrameworkCore.Query.Associations.Navigations;
7+
8+
public class NavigationsBulkUpdateSqliteTest(NavigationsSqliteFixture fixture, ITestOutputHelper testOutputHelper)
9+
: NavigationsBulkUpdateRelationalTestBase<NavigationsSqliteFixture>(fixture, testOutputHelper)
10+
{
11+
// FK constraint failures (SQLite enforces FK constraints on DELETE, blocking cascade)
12+
public override Task Delete_entity_with_associations()
13+
=> Assert.ThrowsAsync<SqliteException>(base.Delete_entity_with_associations);
14+
15+
public override Task Delete_required_associate()
16+
=> Assert.ThrowsAsync<SqliteException>(base.Delete_required_associate);
17+
18+
public override Task Delete_optional_associate()
19+
=> Assert.ThrowsAsync<SqliteException>(base.Delete_optional_associate);
20+
21+
// Translation not yet supported for navigation-mapped associations
22+
public override Task Update_associate_to_parameter()
23+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_associate_to_parameter);
24+
25+
public override Task Update_associate_to_inline()
26+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_associate_to_inline);
27+
28+
public override Task Update_associate_to_inline_with_lambda()
29+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_associate_to_inline_with_lambda);
30+
31+
public override Task Update_associate_to_another_associate()
32+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_associate_to_another_associate);
33+
34+
public override Task Update_associate_to_null()
35+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_associate_to_null);
36+
37+
public override Task Update_associate_to_null_with_lambda()
38+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_associate_to_null_with_lambda);
39+
40+
public override Task Update_associate_to_null_parameter()
41+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_associate_to_null_parameter);
42+
43+
public override Task Update_nested_associate_to_parameter()
44+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_nested_associate_to_parameter);
45+
46+
public override Task Update_nested_associate_to_inline_with_lambda()
47+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_nested_associate_to_inline_with_lambda);
48+
49+
public override Task Update_nested_associate_to_another_nested_associate()
50+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_nested_associate_to_another_nested_associate);
51+
52+
public override Task Update_nested_collection_to_parameter()
53+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_nested_collection_to_parameter);
54+
55+
public override Task Update_nested_collection_to_inline_with_lambda()
56+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_nested_collection_to_inline_with_lambda);
57+
58+
public override Task Update_nested_collection_to_another_nested_collection()
59+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_nested_collection_to_another_nested_collection);
60+
61+
public override Task Update_collection_to_parameter()
62+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_collection_to_parameter);
63+
64+
public override Task Update_collection_referencing_the_original_collection()
65+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_collection_referencing_the_original_collection);
66+
67+
public override Task Update_primitive_collection_to_another_collection()
68+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_primitive_collection_to_another_collection);
69+
70+
public override Task Update_inside_structural_collection()
71+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_inside_structural_collection);
72+
73+
public override Task Update_multiple_properties_inside_associates_and_on_entity_type()
74+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_multiple_properties_inside_associates_and_on_entity_type);
75+
76+
public override Task Update_multiple_projected_associates_via_anonymous_type()
77+
=> Assert.ThrowsAsync<InvalidOperationException>(base.Update_multiple_projected_associates_via_anonymous_type);
78+
79+
public override async Task Update_property_on_projected_associate_with_OrderBy_Skip()
80+
=> await Assert.ThrowsAnyAsync<Exception>(base.Update_property_on_projected_associate_with_OrderBy_Skip);
81+
}

0 commit comments

Comments
 (0)