Skip to content

Commit 8800738

Browse files
AndriySvyrydAndriySvyryd
committed
Derive the state of modifications to a shared row only from the main entry (the entry that isn't a dependent of any other entry in the row). If the main entry isn't present the row is assumed to be modified.
The principal entry is now not required to be in the same state as the dependent when saving changes to a shared row. Fixes #17422 Fixes #17935
1 parent 0350335 commit 8800738

File tree

12 files changed

+338
-242
lines changed

12 files changed

+338
-242
lines changed

‎src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,9 +2036,12 @@ protected virtual IEnumerable<MigrationOperation> GetDataOperations([NotNull] Di
20362036
private object GetValue(ColumnModification columnModification)
20372037
{
20382038
var converter = GetValueConverter(columnModification.Property);
2039+
var value = columnModification.UseCurrentValueParameter
2040+
? columnModification.Value
2041+
: columnModification.OriginalValue;
20392042
return converter != null
2040-
? converter.ConvertToProvider(columnModification.Value)
2041-
: columnModification.Value;
2043+
? converter.ConvertToProvider(value)
2044+
: value;
20422045
}
20432046

20442047
#endregion

‎src/EFCore.Relational/Update/ColumnModification.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,24 +149,24 @@ public ColumnModification(
149149
/// <summary>
150150
/// Indicates whether the original value of the property must be passed as a parameter to the SQL
151151
/// </summary>
152-
public virtual bool UseOriginalValueParameter => _useParameters && IsCondition && IsConcurrencyToken;
152+
public virtual bool UseOriginalValueParameter => _useParameters && IsCondition;
153153

154154
/// <summary>
155155
/// Indicates whether the current value of the property must be passed as a parameter to the SQL
156156
/// </summary>
157-
public virtual bool UseCurrentValueParameter => _useParameters && (IsWrite || IsCondition && !IsConcurrencyToken);
157+
public virtual bool UseCurrentValueParameter => _useParameters && IsWrite;
158158

159159
/// <summary>
160160
/// The parameter name to use for the current value parameter (<see cref="UseCurrentValueParameter" />), if needed.
161161
/// </summary>
162162
public virtual string ParameterName
163-
=> _parameterName ?? (_parameterName = _generateParameterName());
163+
=> _parameterName ?? (_parameterName = UseCurrentValueParameter ? _generateParameterName() : null);
164164

165165
/// <summary>
166166
/// The parameter name to use for the original value parameter (<see cref="UseOriginalValueParameter" />), if needed.
167167
/// </summary>
168168
public virtual string OriginalParameterName
169-
=> _originalParameterName ?? (_originalParameterName = _generateParameterName());
169+
=> _originalParameterName ?? (_originalParameterName = UseOriginalValueParameter ? _generateParameterName() : null);
170170

171171
/// <summary>
172172
/// The name of the column.
@@ -187,7 +187,9 @@ public virtual string OriginalParameterName
187187
/// </summary>
188188
public virtual object Value
189189
{
190-
get => Entry == null ? _value : Entry.GetCurrentValue(Property);
190+
get => Entry == null
191+
? _value
192+
: Entry.EntityState == EntityState.Deleted ? null : Entry.GetCurrentValue(Property);
191193
[param: CanBeNull]
192194
set
193195
{

‎src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs

Lines changed: 8 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ protected virtual IEnumerable<ModificationCommand> CreateModificationCommands(
184184
var tableKey = (schema, table);
185185

186186
ModificationCommand command;
187+
var isMainEntry = true;
187188
if (_sharedTableEntryMapFactories.TryGetValue(tableKey, out var commandIdentityMapFactory))
188189
{
189190
if (sharedTablesCommandsMap == null)
@@ -201,20 +202,20 @@ protected virtual IEnumerable<ModificationCommand> CreateModificationCommands(
201202
}
202203

203204
command = sharedCommandsMap.GetOrAddValue(entry);
205+
isMainEntry = sharedCommandsMap.GetPrincipals(entry.EntityType.GetRootType()).Count == 0;
204206
}
205207
else
206208
{
207209
command = new ModificationCommand(
208210
table, schema, generateParameterName, _sensitiveLoggingEnabled, comparer: null);
209211
}
210212

211-
command.AddEntry(entry);
213+
command.AddEntry(entry, isMainEntry);
212214
commands.Add(command);
213215
}
214216

215217
if (sharedTablesCommandsMap != null)
216218
{
217-
Validate(sharedTablesCommandsMap);
218219
AddUnchangedSharingEntries(sharedTablesCommandsMap, entries);
219220
}
220221

@@ -223,86 +224,20 @@ protected virtual IEnumerable<ModificationCommand> CreateModificationCommands(
223224
|| c.ColumnModifications.Any(m => m.IsWrite));
224225
}
225226

226-
private void Validate(
227-
Dictionary<(string Schema, string Name),
228-
SharedTableEntryMap<ModificationCommand>> sharedTablesCommandsMap)
229-
{
230-
foreach (var modificationCommandIdentityMap in sharedTablesCommandsMap.Values)
231-
{
232-
foreach (var command in modificationCommandIdentityMap.Values)
233-
{
234-
if (command.EntityState != EntityState.Added
235-
&& command.EntityState != EntityState.Deleted)
236-
{
237-
continue;
238-
}
239-
240-
// ReSharper disable once ForCanBeConvertedToForeach
241-
for (var entryIndex = 0; entryIndex < command.Entries.Count; entryIndex++)
242-
{
243-
var entry = command.Entries[entryIndex];
244-
var principals = modificationCommandIdentityMap.GetPrincipals(entry.EntityType);
245-
// ReSharper disable once ForCanBeConvertedToForeach
246-
for (var principalIndex = 0; principalIndex < principals.Count; principalIndex++)
247-
{
248-
var principalEntityType = principals[principalIndex];
249-
var principalFound = false;
250-
// ReSharper disable once ForCanBeConvertedToForeach
251-
for (var otherEntryIndex = 0; otherEntryIndex < command.Entries.Count; otherEntryIndex++)
252-
{
253-
var principalEntry = command.Entries[otherEntryIndex];
254-
if (principalEntry != entry
255-
&& principalEntityType.IsAssignableFrom(principalEntry.EntityType))
256-
{
257-
principalFound = true;
258-
break;
259-
}
260-
}
261-
262-
if (principalFound)
263-
{
264-
continue;
265-
}
266-
267-
var tableName = (string.IsNullOrEmpty(command.Schema) ? "" : command.Schema + ".") +
268-
command.TableName;
269-
if (_sensitiveLoggingEnabled)
270-
{
271-
throw new InvalidOperationException(
272-
RelationalStrings.SharedRowEntryCountMismatchSensitive(
273-
entry.EntityType.DisplayName(),
274-
tableName,
275-
principalEntityType.DisplayName(),
276-
entry.BuildCurrentValuesString(entry.EntityType.FindPrimaryKey().Properties),
277-
command.EntityState));
278-
}
279-
280-
throw new InvalidOperationException(
281-
RelationalStrings.SharedRowEntryCountMismatch(
282-
entry.EntityType.DisplayName(),
283-
tableName,
284-
principalEntityType.DisplayName(),
285-
command.EntityState));
286-
}
287-
}
288-
}
289-
}
290-
}
291-
292227
private void AddUnchangedSharingEntries(
293228
Dictionary<(string Schema, string Name), SharedTableEntryMap<ModificationCommand>> sharedTablesCommandsMap,
294229
IList<IUpdateEntry> entries)
295230
{
296-
foreach (var modificationCommandIdentityMap in sharedTablesCommandsMap.Values)
231+
foreach (var sharedCommandsMap in sharedTablesCommandsMap.Values)
297232
{
298-
foreach (var command in modificationCommandIdentityMap.Values)
233+
foreach (var command in sharedCommandsMap.Values)
299234
{
300235
if (command.EntityState != EntityState.Modified)
301236
{
302237
continue;
303238
}
304239

305-
foreach (var entry in modificationCommandIdentityMap.GetAllEntries(command.Entries[0]))
240+
foreach (var entry in sharedCommandsMap.GetAllEntries(command.Entries[0]))
306241
{
307242
if (entry.EntityState != EntityState.Unchanged)
308243
{
@@ -311,7 +246,8 @@ private void AddUnchangedSharingEntries(
311246

312247
entry.EntityState = EntityState.Modified;
313248

314-
command.AddEntry(entry);
249+
var isMainEntry = sharedCommandsMap.GetPrincipals(entry.EntityType.GetRootType()).Count == 0;
250+
command.AddEntry(entry, isMainEntry);
315251
entries.Add(entry);
316252
}
317253
}

‎src/EFCore.Relational/Update/Internal/SharedTableEntryMap.cs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -257,14 +257,11 @@ public EntryComparer(IReadOnlyDictionary<IEntityType, IReadOnlyList<IEntityType>
257257
}
258258

259259
public int Compare(IUpdateEntry x, IUpdateEntry y)
260-
{
261-
if (_principals[x.EntityType].Count == 0)
262-
{
263-
return -1;
264-
}
265-
266-
return _principals[y.EntityType].Count == 0 ? 1 : StringComparer.Ordinal.Compare(x.EntityType.Name, y.EntityType.Name);
267-
}
260+
=> _principals[x.EntityType].Count == 0
261+
? -1
262+
: _principals[y.EntityType].Count == 0
263+
? 1
264+
: StringComparer.Ordinal.Compare(x.EntityType.Name, y.EntityType.Name);
268265
}
269266
}
270267
}

‎src/EFCore.Relational/Update/ModificationCommand.cs

Lines changed: 84 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public class ModificationCommand
3030
private readonly List<IUpdateEntry> _entries = new List<IUpdateEntry>();
3131
private IReadOnlyList<ColumnModification> _columnModifications;
3232
private bool _requiresResultPropagation;
33+
private bool _mainEntryAdded;
3334

3435
/// <summary>
3536
/// Initializes a new <see cref="ModificationCommand" /> instance.
@@ -104,22 +105,10 @@ public virtual EntityState EntityState
104105
{
105106
get
106107
{
107-
if (_entries.Count > 0)
108+
if (_mainEntryAdded)
108109
{
109-
for (var i = 0; i < _entries.Count; i++)
110-
{
111-
var entry = _entries[0];
112-
if (entry.SharedIdentityEntry != null)
113-
{
114-
return EntityState.Modified;
115-
}
116-
117-
var state = entry.EntityState;
118-
if (state != EntityState.Unchanged)
119-
{
120-
return state;
121-
}
122-
}
110+
var entry = _entries[0];
111+
return entry.SharedIdentityEntry != null ? EntityState.Modified : entry.EntityState;
123112
}
124113

125114
return EntityState.Modified;
@@ -152,7 +141,16 @@ public virtual bool RequiresResultPropagation
152141
/// Adds an <see cref="IUpdateEntry" /> to this command representing an entity to be inserted, updated, or deleted.
153142
/// </summary>
154143
/// <param name="entry"> The entry representing the entity to add. </param>
144+
[Obsolete("Use AddEntry with most parameters")]
155145
public virtual void AddEntry([NotNull] IUpdateEntry entry)
146+
=> AddEntry(entry, mainEntry: false);
147+
148+
/// <summary>
149+
/// Adds an <see cref="IUpdateEntry" /> to this command representing an entity to be inserted, updated, or deleted.
150+
/// </summary>
151+
/// <param name="entry"> The entry representing the entity to add. </param>
152+
/// <param name="mainEntry"> A value indicating whether this is the main entry for the row. </param>
153+
public virtual void AddEntry([NotNull] IUpdateEntry entry, bool mainEntry)
156154
{
157155
Check.NotNull(entry, nameof(entry));
158156

@@ -166,38 +164,65 @@ public virtual void AddEntry([NotNull] IUpdateEntry entry)
166164
throw new ArgumentException(RelationalStrings.ModificationCommandInvalidEntityState(entry.EntityState));
167165
}
168166

169-
if (_entries.Count > 0)
167+
if (mainEntry)
170168
{
171-
var currentState = EntityState;
172-
var entryState = entry.SharedIdentityEntry == null
173-
? entry.EntityState
174-
: EntityState.Modified;
175-
if (currentState != entryState
176-
&& entryState != EntityState.Unchanged)
169+
Check.DebugAssert(!_mainEntryAdded, "Only expected a single main entry");
170+
171+
for (var i = 0; i < _entries.Count; i++)
177172
{
178-
if (_sensitiveLoggingEnabled)
179-
{
180-
throw new InvalidOperationException(
181-
RelationalStrings.ConflictingRowUpdateTypesSensitive(
182-
entry.EntityType.DisplayName(),
183-
entry.BuildCurrentValuesString(entry.EntityType.FindPrimaryKey().Properties),
184-
entryState,
185-
_entries[0].EntityType.DisplayName(),
186-
_entries[0].BuildCurrentValuesString(_entries[0].EntityType.FindPrimaryKey().Properties),
187-
currentState));
188-
}
173+
ValidateState(entry, _entries[i]);
174+
}
175+
176+
_mainEntryAdded = true;
177+
_entries.Insert(0, entry);
178+
}
179+
else
180+
{
181+
if (_mainEntryAdded)
182+
{
183+
ValidateState(_entries[0], entry);
184+
}
185+
186+
_entries.Add(entry);
187+
}
188+
189+
_columnModifications = null;
190+
}
189191

192+
private void ValidateState(IUpdateEntry mainEntry, IUpdateEntry entry)
193+
{
194+
var mainEntryState = mainEntry.SharedIdentityEntry == null
195+
? mainEntry.EntityState
196+
: EntityState.Modified;
197+
if (mainEntryState == EntityState.Modified)
198+
{
199+
return;
200+
}
201+
202+
var entryState = entry.SharedIdentityEntry == null
203+
? entry.EntityState
204+
: EntityState.Modified;
205+
if (mainEntryState != entryState)
206+
{
207+
if (_sensitiveLoggingEnabled)
208+
{
190209
throw new InvalidOperationException(
191-
RelationalStrings.ConflictingRowUpdateTypes(
210+
RelationalStrings.ConflictingRowUpdateTypesSensitive(
192211
entry.EntityType.DisplayName(),
212+
entry.BuildCurrentValuesString(entry.EntityType.FindPrimaryKey().Properties),
193213
entryState,
194-
_entries[0].EntityType.DisplayName(),
195-
currentState));
214+
mainEntry.EntityType.DisplayName(),
215+
mainEntry.BuildCurrentValuesString(mainEntry.EntityType.FindPrimaryKey().Properties),
216+
mainEntryState));
196217
}
197-
}
198218

199-
_entries.Add(entry);
200-
_columnModifications = null;
219+
throw new InvalidOperationException(
220+
RelationalStrings.ConflictingRowUpdateTypes(
221+
entry.EntityType.DisplayName(),
222+
entryState,
223+
mainEntry.EntityType.DisplayName(),
224+
mainEntryState));
225+
}
201226
}
202227

203228
private IReadOnlyList<ColumnModification> GenerateColumnModifications()
@@ -231,6 +256,10 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
231256

232257
foreach (var entry in _entries)
233258
{
259+
var nonMainEntry = updating
260+
&& (entry.EntityState == EntityState.Deleted
261+
|| entry.EntityState == EntityState.Added);
262+
234263
foreach (var property in entry.EntityType.GetProperties())
235264
{
236265
var isKey = property.IsPrimaryKey();
@@ -247,7 +276,8 @@ private IReadOnlyList<ColumnModification> GenerateColumnModifications()
247276
{
248277
writeValue = property.GetBeforeSaveBehavior() == PropertySaveBehavior.Save;
249278
}
250-
else if (updating && property.GetAfterSaveBehavior() == PropertySaveBehavior.Save)
279+
else if ((updating && property.GetAfterSaveBehavior() == PropertySaveBehavior.Save)
280+
|| (!isKey && nonMainEntry))
251281
{
252282
writeValue = columnPropagator?.TryPropagate(property, (InternalEntityEntry)entry)
253283
?? entry.IsModified(property);
@@ -353,15 +383,27 @@ public void RecordValue(IProperty property, IUpdateEntry entry)
353383

354384
break;
355385
case EntityState.Added:
356-
if (!_write)
386+
_currentValue = entry.GetCurrentValue(property);
387+
388+
var comparer = property.GetValueComparer() ?? property.FindTypeMapping()?.Comparer;
389+
if (comparer == null)
357390
{
358-
_currentValue = entry.GetCurrentValue(property);
359391
_write = !Equals(_originalValue, _currentValue);
360392
}
393+
else
394+
{
395+
_write = !comparer.Equals(_originalValue, _currentValue);
396+
}
361397

362398
break;
363399
case EntityState.Deleted:
364400
_originalValue = entry.GetOriginalValue(property);
401+
if (!_write
402+
&& !property.IsPrimaryKey())
403+
{
404+
_write = true;
405+
_currentValue = null;
406+
}
365407
break;
366408
}
367409
}

0 commit comments

Comments
 (0)