Version Column causes extra update and if no fields are updated gets updated anyway
Two Questions / Issues here:
TUpdateExecutor uses two queries to update when version column is defined instead of one
This might not seem like a big deal but when dealing with a large number of records this extra update wastes valuable processing time.
Please consider the following code and comments from Spring.Persistence.SQL.Commands.Update
procedure TUpdateExecutor.Execute(const entity: TObject);
var
statement: IDBStatement;
sqlStatement: string;
begin
Assert(Assigned(entity));
if EntityData.HasVersionColumn and not TryIncrementVersionFor(entity) then
raise EORMOptimisticLockException.Create(entity);
The TryIncrementVersionFor function actually performs an update against the database and attempts to increment the version.
statement := Connection.CreateStatement;
fColumns.Clear;
if fEntityMap.IsMapped(entity) then
begin
fColumns := fEntityMap.GetChangedMembers(entity, EntityData);
if HasChangedVersionColumnOnly then
Exit;
end
else
fColumns.AddRange(EntityData.Columns);
Next it seems that it checks to see if any other fields have been modified and exits if none have, but since the Version is already incremented it is not rolled back
fCommand.SetCommandFieldsFromColumns(fColumns);
if not fCommand.UpdateFields.Any then
Exit;
fCommand.Entity := entity;
sqlStatement := Generator.GenerateUpdate(fCommand);
if sqlStatement = '' then
raise EORMCannotGenerateQueryStatement.Create(entity);
statement.SetSQLCommand(sqlStatement);
BuildParams(entity);
statement.SetParams(SQLParameters);
statement.Execute;
end;
Finally the actual update is performed.
The Optimistic lock functionality could be accomplished with just one statement if the version attribute were checked and incremented in the actual update.
For instance: If the Underlying update query looked as follows:
Update TableName
Set VersionColumn = VersionColumn + 1,
Other Columns,
where VersionColumn = :CurrentVersionNumber,
Other Where Columns;
Now all that needs to be done is to check that rows affected is equal to zero, and if so throw the Optimistic Lock Exception.
When there is nothing to udpate, instead of exiting without updating the row version is updated anyway
This is not critical but as seen in the center code block code checks to see if only the version column was updated and just exits and does not perform the update if that is the only column updated. This is awesome but because the previous TryIncrementVersionFor already ran it seems that the version value gets updated anyway in the database. It would be really nice if the version value was not updated if no fields changed.
This would also be accomplished if the entire update was done in one statement as described in number 1 above.
Comments (8)
-
reporter -
repo owner - changed status to open
-
repo owner - changed milestone to 1.3
- changed component to Persistence
-
assigned issue to
- changed version to 1.2 (develop)
-
reporter One thing I missed on this is to increment the version value on the Entity.
So the last line in the code snipet below is not in the pas unit, but is necessary.
procedure TUpdateExecutor.Execute(const entity: TObject); var statement: IDBStatement; sqlStatement: string; RowsAffected : Integer; begin Assert(Assigned(entity)); fColumns.Clear; if fEntityMap.IsMapped(entity) then begin fColumns := fEntityMap.GetChangedMembers(entity, EntityData); if (FColumns.Count = 0) then // No Columns to update Exit; end else fColumns.AddRange(EntityData.Columns); fCommand.VersionColumn := entityData.VersionColumn; fCommand.SetCommandFieldsFromColumns(fColumns); if not fCommand.UpdateFields.Any then Exit; statement := Connection.CreateStatement; fCommand.Entity := entity; sqlStatement := Generator.GenerateUpdate(fCommand); if sqlStatement = '' then raise EORMCannotGenerateQueryStatement.Create(entity); statement.SetSQLCommand(sqlStatement); BuildParams(entity); statement.SetParams(SQLParameters); RowsAffected := statement.Execute; if EntityData.HasVersionColumn and (RowsAffected = 0) then raise EORMOptimisticLockException.Create(entity); EntityData.VersionColumn.Member.SetValue(entity, EntityData.VersionColumn.Member.GetValue(entity).AsInteger + 1); < -- Need to update the Version Column value here. end;
-
reporter OOPS actually the last line needs to be
if Assigned(EntityData.VersionColumn) then EntityData.VersionColumn.Member.SetValue(entity, EntityData.VersionColumn.Member.GetValue(entity).AsInteger + 1);
-
repo owner - changed status to resolved
fixed issue
#192→ <<cset 1b757f11f81d>>
-
repo owner - changed version to 1.2
-
repo owner - changed milestone to 2.0
- Log in to comment
Attached is a possible way to implement this. I am testing it now on our system. Keep in mind that we always have Version Columns on our entities so I am not testing on entities that do not have a Version Column defined