Version Column causes extra update and if no fields are updated gets updated anyway

[Todd Flora]

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.

[Todd Flora]

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

[Todd Flora]

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;

[Todd Flora]

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);

[Stefan Glienke]

fixed issue #192

→ <<cset 1b757f11f81d>>