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.