Skip to content

Commit dd47f26

Browse files
CopilotPhenXCopilotfabien.menager
authored
Fix complex property update in ON CONFLICT clause (#89)
* Initial plan * Fix complex property update in ON CONFLICT clause and add tests Co-authored-by: PhenX <42170+PhenX@users.noreply.github.com> * Address code review feedback for documentation and comments Co-authored-by: PhenX <42170+PhenX@users.noreply.github.com> * Improve comment Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix nested MemberInitExpression for complex property updates in ON CONFLICT Co-authored-by: PhenX <42170+PhenX@users.noreply.github.com> * Make GetUpdatesFromMemberInit truly recursive for arbitrary nesting Co-authored-by: PhenX <42170+PhenX@users.noreply.github.com> * Fix compilation warning --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PhenX <42170+PhenX@users.noreply.github.com> Co-authored-by: Fabien Ménager <PhenX@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: fabien.menager <fabien.menager@am-creations.fr>
1 parent 844a080 commit dd47f26

4 files changed

Lines changed: 239 additions & 8 deletions

File tree

‎src/PhenX.EntityFrameworkCore.BulkInsert/Dialect/SqlDialectBuilder.cs‎

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,9 @@ protected IEnumerable<string> GetUpdates<T>(DbContext context, TableMetadata tab
253253
}
254254
case MemberInitExpression memberInit:
255255
{
256-
foreach (var binding in memberInit.Bindings.OfType<MemberAssignment>())
256+
foreach (var updateSql in GetUpdatesFromMemberInit<T>(context, table, memberInit, lambda))
257257
{
258-
yield return $"{table.GetQuotedColumnName(binding.Member.Name)} = {ToSqlExpression<T>(context, table, binding.Expression, lambda)}";
258+
yield return updateSql;
259259
}
260260

261261
break;
@@ -297,15 +297,20 @@ private string ToSqlExpression<TEntity>(DbContext context, TableMetadata table,
297297
case MemberExpression memberExpr:
298298
var columnName = table.GetColumnName(memberExpr.Member.Name);
299299

300+
// Traverse up the expression chain to find the root parameter
301+
// This handles both simple properties (e.g., excluded.Name) and
302+
// complex properties (e.g., excluded.ComplexObject.Property)
303+
var rootParam = GetRootParameter(memberExpr);
304+
300305
// If the member expression is a property of the current lambda
301-
if (lambda is { Parameters.Count: > 1 } && memberExpr.Expression is ParameterExpression paramExpr)
306+
if (lambda is { Parameters.Count: > 1 } && rootParam != null)
302307
{
303-
if (paramExpr.Name == lambda.Parameters[0].Name)
308+
if (rootParam.Name == lambda.Parameters[0].Name)
304309
{
305310
return GetInsertedColumnName(columnName);
306311
}
307312

308-
if (paramExpr.Name == lambda.Parameters[1].Name)
313+
if (rootParam.Name == lambda.Parameters[1].Name)
309314
{
310315
return GetExcludedColumnName(columnName);
311316
}
@@ -405,4 +410,64 @@ private string ToSqlExpression<TEntity>(DbContext context, TableMetadata table,
405410
throw new NotSupportedException($"Expression not supported: {expr.NodeType}");
406411
}
407412
}
413+
414+
/// <summary>
415+
/// Extracts update SQL statements from a MemberInitExpression, handling both simple properties
416+
/// and nested complex property initializations recursively.
417+
/// </summary>
418+
/// <param name="context">DB context</param>
419+
/// <param name="table">Table metadata</param>
420+
/// <param name="memberInit">The member initialization expression</param>
421+
/// <param name="lambda">Current lambda expression</param>
422+
/// <typeparam name="T">Entity type</typeparam>
423+
/// <returns>SQL update statements for each property assignment</returns>
424+
private IEnumerable<string> GetUpdatesFromMemberInit<T>(DbContext context, TableMetadata table, MemberInitExpression memberInit, LambdaExpression lambda)
425+
{
426+
foreach (var binding in memberInit.Bindings.OfType<MemberAssignment>())
427+
{
428+
// Check if the binding expression is a nested MemberInitExpression (complex property assignment)
429+
if (binding.Expression is MemberInitExpression nestedMemberInit)
430+
{
431+
// Recursively process nested complex property assignments to handle arbitrary nesting levels
432+
foreach (var update in GetUpdatesFromMemberInit<T>(context, table, nestedMemberInit, lambda))
433+
{
434+
yield return update;
435+
}
436+
}
437+
else
438+
{
439+
// Simple property assignment - the column name is the property name
440+
yield return $"{table.GetQuotedColumnName(binding.Member.Name)} = {ToSqlExpression<T>(context, table, binding.Expression, lambda)}";
441+
}
442+
}
443+
}
444+
445+
/// <summary>
446+
/// Traverses up a member expression chain to find the root parameter expression.
447+
/// This handles both simple properties (e.g., excluded.Name) and complex properties (e.g., excluded.ComplexObject.Property).
448+
/// </summary>
449+
/// <param name="memberExpr">The member expression to traverse.</param>
450+
/// <returns>The root parameter expression if found; otherwise, null if the expression chain doesn't contain a parameter.</returns>
451+
private static ParameterExpression? GetRootParameter(MemberExpression memberExpr)
452+
{
453+
Expression? current = memberExpr.Expression;
454+
while (current != null)
455+
{
456+
if (current is ParameterExpression param)
457+
{
458+
return param;
459+
}
460+
461+
if (current is MemberExpression nested)
462+
{
463+
current = nested.Expression;
464+
}
465+
else
466+
{
467+
break;
468+
}
469+
}
470+
471+
return null;
472+
}
408473
}

‎tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/DbContext/TestSmartEnum.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ private TestSmartEnum(string name, int value) : base(name, value)
88
{
99
}
1010

11-
public static readonly TestSmartEnum Value = new TestSmartEnum("test", 1);
11+
public static readonly TestSmartEnum Test = new TestSmartEnum("test", 1);
1212
}

‎tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/Tests/Merge/MergeTestsBase.cs‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,4 +348,170 @@ public async Task InsertEntities_WithConflict_MultipleColumns(InsertStrategy str
348348
Assert.Contains(insertedEntities, e => e.Name == $"{_run}_Entity2");
349349
Assert.Contains(insertedEntities, e => e.Name == $"{_run}_Entity1 - Conflict" && e.Price == 0);
350350
}
351+
352+
[SkippableTheory]
353+
[InlineData(InsertStrategy.InsertReturn)]
354+
[InlineData(InsertStrategy.InsertReturnAsync)]
355+
public async Task InsertEntities_WithComplexType_UpdateAll(InsertStrategy strategy)
356+
{
357+
Skip.If(_context.IsProvider(ProviderType.MySql));
358+
// Oracle MERGE does not support returning entities
359+
Skip.If(_context.IsProvider(ProviderType.Oracle));
360+
361+
// Arrange
362+
var entities = new List<TestEntityWithComplexType>
363+
{
364+
new TestEntityWithComplexType
365+
{
366+
TestRun = _run,
367+
OwnedComplexType = new OwnedObject { Code = 1, Name = "Name1" }
368+
},
369+
new TestEntityWithComplexType
370+
{
371+
TestRun = _run,
372+
OwnedComplexType = new OwnedObject { Code = 2, Name = "Name2" }
373+
}
374+
};
375+
376+
// Act - First insert (without CopyGeneratedColumns - returns generated IDs via RETURNING)
377+
var insertedEntities = await _context.InsertWithStrategyAsync(strategy, entities);
378+
379+
// Update the complex properties
380+
foreach (var entity in insertedEntities)
381+
{
382+
entity.OwnedComplexType = new OwnedObject
383+
{
384+
Code = entity.OwnedComplexType.Code + 100,
385+
Name = $"Updated_{entity.OwnedComplexType.Name}"
386+
};
387+
}
388+
389+
// Act - Second insert with update on conflict
390+
// The ParameterExpression case in GetUpdates generates UPDATE statements for all columns
391+
var updatedEntities = await _context.InsertWithStrategyAsync(strategy, insertedEntities, o => o.CopyGeneratedColumns = true,
392+
onConflict: new OnConflictOptions<TestEntityWithComplexType>
393+
{
394+
Update = (inserted, excluded) => inserted,
395+
});
396+
397+
// Assert - complex properties should be updated
398+
Assert.Equal(2, updatedEntities.Count);
399+
Assert.All(updatedEntities, e =>
400+
{
401+
Assert.StartsWith("Updated_", e.OwnedComplexType.Name);
402+
Assert.True(e.OwnedComplexType.Code > 100);
403+
});
404+
}
405+
406+
[SkippableTheory]
407+
[InlineData(InsertStrategy.InsertReturn)]
408+
[InlineData(InsertStrategy.InsertReturnAsync)]
409+
public async Task InsertEntities_WithComplexType_UpdateWithWhere(InsertStrategy strategy)
410+
{
411+
Skip.If(_context.IsProvider(ProviderType.MySql));
412+
// Oracle MERGE does not support returning entities
413+
Skip.If(_context.IsProvider(ProviderType.Oracle));
414+
415+
// Arrange - initial Code values are 10 and 20
416+
var entities = new List<TestEntityWithComplexType>
417+
{
418+
new TestEntityWithComplexType
419+
{
420+
TestRun = _run,
421+
OwnedComplexType = new OwnedObject { Code = 10, Name = "Original1" }
422+
},
423+
new TestEntityWithComplexType
424+
{
425+
TestRun = _run,
426+
OwnedComplexType = new OwnedObject { Code = 20, Name = "Original2" }
427+
}
428+
};
429+
430+
// Act - First insert (without CopyGeneratedColumns - returns generated IDs via RETURNING)
431+
var insertedEntities = await _context.InsertWithStrategyAsync(strategy, entities);
432+
433+
// Update the complex property - new Code values will be original + 100 (110 and 120)
434+
foreach (var entity in insertedEntities)
435+
{
436+
entity.OwnedComplexType.Name = $"Changed_{entity.OwnedComplexType.Name}";
437+
entity.OwnedComplexType.Code = entity.OwnedComplexType.Code + 100;
438+
}
439+
440+
// Act - Second insert updating complex properties with a WHERE condition
441+
// This tests that complex property access works correctly in the Where clause
442+
var updatedEntities = await _context.InsertWithStrategyAsync(strategy, insertedEntities, o => o.CopyGeneratedColumns = true,
443+
onConflict: new OnConflictOptions<TestEntityWithComplexType>
444+
{
445+
Update = (inserted, excluded) => inserted,
446+
Where = (inserted, excluded) => excluded.OwnedComplexType.Code > inserted.OwnedComplexType.Code
447+
});
448+
449+
// Assert - entities should be updated because the new Code values (110, 120)
450+
// are greater than the existing values in the database (10, 20)
451+
Assert.Equal(2, updatedEntities.Count);
452+
Assert.All(updatedEntities, e =>
453+
{
454+
Assert.StartsWith("Changed_", e.OwnedComplexType.Name);
455+
Assert.True(e.OwnedComplexType.Code > 100);
456+
});
457+
}
458+
459+
[SkippableTheory]
460+
[InlineData(InsertStrategy.InsertReturn)]
461+
[InlineData(InsertStrategy.InsertReturnAsync)]
462+
public async Task InsertEntities_WithComplexType_UpdateComplexPropertyConditionally(InsertStrategy strategy)
463+
{
464+
Skip.If(_context.IsProvider(ProviderType.MySql));
465+
// Oracle MERGE does not support returning entities
466+
Skip.If(_context.IsProvider(ProviderType.Oracle));
467+
468+
// Arrange - Create entities with different Code values
469+
var entities = new List<TestEntityWithComplexType>
470+
{
471+
new TestEntityWithComplexType
472+
{
473+
TestRun = _run,
474+
OwnedComplexType = new OwnedObject { Code = 50, Name = "LowCode" }
475+
},
476+
new TestEntityWithComplexType
477+
{
478+
TestRun = _run,
479+
OwnedComplexType = new OwnedObject { Code = 150, Name = "HighCode" }
480+
}
481+
};
482+
483+
// Act - First insert (returns entities with generated IDs)
484+
var insertedEntities = await _context.InsertWithStrategyAsync(strategy, entities);
485+
486+
// Update both entities with new values
487+
foreach (var entity in insertedEntities)
488+
{
489+
entity.OwnedComplexType.Code = entity.OwnedComplexType.Code + 10;
490+
entity.OwnedComplexType.Name = $"Modified_{entity.OwnedComplexType.Name}";
491+
}
492+
493+
// Act - Update using nested MemberInitExpression for complex property assignment
494+
// Note: entities with Code >= 100 (original value) will not be updated due to WHERE clause
495+
var updatedEntities = await _context.InsertWithStrategyAsync(strategy, insertedEntities,
496+
o => o.CopyGeneratedColumns = true,
497+
onConflict: new OnConflictOptions<TestEntityWithComplexType>
498+
{
499+
Update = (inserted, excluded) => new TestEntityWithComplexType
500+
{
501+
OwnedComplexType = new OwnedObject
502+
{
503+
Code = excluded.OwnedComplexType.Code,
504+
Name = excluded.OwnedComplexType.Name
505+
}
506+
},
507+
Where = (inserted, excluded) => inserted.OwnedComplexType.Code < 100
508+
});
509+
510+
// Assert - Only the entity with original Code < 100 should be updated (Code was 50, now 60)
511+
// The one with original Code >= 100 is not updated but is also not returned by RETURNING clause
512+
Assert.Single(updatedEntities);
513+
var updatedEntity = updatedEntities.Single();
514+
Assert.Equal(60, updatedEntity.OwnedComplexType.Code);
515+
Assert.Equal("Modified_LowCode", updatedEntity.OwnedComplexType.Name);
516+
}
351517
}

‎tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/Tests/Various/VariousTestsBase.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ public async Task InsertSmartEnumEntities(InsertStrategy strategy)
3434
// Arrange
3535
var entities = new List<TestEntityWithSmartEnum>
3636
{
37-
new TestEntityWithSmartEnum { TestRun = _run, Enum = TestSmartEnum.Value},
38-
new TestEntityWithSmartEnum { TestRun = _run, Enum = TestSmartEnum.Value}
37+
new TestEntityWithSmartEnum { TestRun = _run, Enum = TestSmartEnum.Test},
38+
new TestEntityWithSmartEnum { TestRun = _run, Enum = TestSmartEnum.Test}
3939
};
4040

4141
// Act

0 commit comments

Comments
 (0)