TZQuery.FieldByName().OldValue broken...?

[aehimself]

I was about to implement a "smart cancel" for my table wrappers; making .Save to call TZQuery.CancelUpdates if nothing was changed. For that, I cycled through all fields of the TZQuery object, comparing .OldValue to .Value.

Bad thing is, it always returns the same.

All settings on default, the following code

Code: Select all

Var
 v: Variant;
begin
 ZQuery1.Active := True;
 ZQuery1.First;
 ZQuery1.Edit;
 ZQuery1.FieldByName('f').AsLargeInt := 0;
 v := ZQuery1.FieldByName('f').OldValue;
 v := ZQuery1.FieldByName('f').Value;
 Zquery1.Cancel;

will set "v" to the new value (in this particular case, 0) in both lines.

Using Zeos 7.3.0-a1d45853 connecting to a MySQL server 8.0.18 with libmysql.dll 6.1.11

commit a1d45853e3cbe60d101232b356fb9724c605f7ea
Author: egonhugeist <egonhugeist@localhost>
Date: Fri Jul 24 07:14:50 2020 +0000

hopefully finalize the logs for mySQL

P.s.: Because of a ZDbcIntFs bug ZConnection.ClientVersionStr will stack overflow if there is no active connection. I'll post a pull request on GitHub with the fix soon.

[aehimself]

Even more strange behavior.

Code is:

Code: Select all

procedure TForm1.Button1Click(Sender: TObject);
Var
 nw: UInt64;
 old1, old2, new: Variant;
begin
 nw := Abs(Random(nw.MaxValue));
 ZQuery1.Active := True;
 ZQuery1.First;
// ZConnection1.StartTransaction;
 old1 := ZQuery1.FieldByName('f').Value;
 ZQuery1.Edit;
 ZQuery1.FieldByName('f').AsLargeInt := nw;
 ZQuery1.Post;
 old2 := ZQuery1.FieldByName('f').OldValue;
 new := ZQuery1.FieldByName('f').Value;
 ZQuery1.CancelUpdates;
 ZConnection1.Rollback;
end;

Results:

AutoCommit = True, CachedUpdates = False, No transaction
.OldValue, .Value both contains new value, changes written to database.

AutoCommit = True, CachedUpdates = False, manual transaction
.OldValue, .Value both contains new value, changes are discarded

AutoCommit = True, CachedUpdates = True, No transaction
Run 1: .OldValue, .Value both contains old value, changes are discarded
Run 2: .OldValue, .Value both contains new value, changes are discarded

AutoCommit = True, CachedUpdates = True, manual transaction
Run 1: .OldValue, .Value both contains old value, changes are discarded
Run 2: .OldValue, .Value both contains new value, changes are discarded

AutoCommit = False, CachedUpdates = False
.OldValue, .Value both contains new value, changes are discarded

AutoCommit = False, CachedUpdates = True
Run 1: .OldValue, .Value both contains old value, changes are discarded
Run 2: .OldValue, .Value both contains new value, changes are discarded

[aehimself]

@Egonhugeist,

I saw the commit r6745 in subversion with the title "I'm affraid -> there is no test available!".
How come?

Choose an already existing table and set an existing field to a known value. Commit everything (if applicable) and then change the field value without commiting.
I this state, OldValue and Value should differ:

Code: Select all

// Preparation
ZQuery.Edit;
ZQuery.FieldByName('MyStringField').AsString := 'Known for sure';
ZQuery.Post; // ZQuery.ApplyUpdates;
// ZConnection.Commit; // If applicable
// Now, change the field value to something different from the known value
ZQuery.Edit;
ZQuery.FieldByName('MyStringField').AsString := 'Something different';
ZQuery.Post;
Assert(ZQuery.FieldByName('MyStringField').OldValue <> ZQuery.FieldByName('MyStringField').Value);

Wouldn't this work...?

[aehimself]

However the checkin is not in Git, I made the same modifications as you did in ZAbstractRODataSet.pas. The issue still exists (at least on MySQL):

Code: Select all

procedure TForm1.Button1Click(Sender: TObject);
Var
 field: TField;
begin
 ZQuery1.Active := True; // Select * from mytable
 field := ZQuery1.FieldByName('f'); // F is BIGINT(20) unsigned
 ZQuery1.First; // Only 1 record exists; where F is 1470057288

 ZConnection1.StartTransaction;
 Try
  Memo1.Lines.Add('Original value: ' + String(field.Value));

  ZQuery1.Edit;
  ZQuery1.FieldByName('f').AsLargeInt := 0;
  ZQuery1.Post;

  Memo1.Lines.Add('Current value: ' + String(field.Value));
  Memo1.Lines.Add('Old value: ' + String(field.OldValue));

  If field.Value = field.OldValue Then Memo1.Lines.Add('No change was detected')
    Else Memo1.Lines.Add('Field value changed');

 Finally
  ZConnection1.Rollback;
 End;
end;

This returns:
Original value: 1470057288
Current value: 0
Old value: 0
No change was detected

Test was done with default configuration; AutoCommit = True, CachedUpdates = False.

[EgonHugeist]

Hi,

i must admit i didn't know the TField.OldValue property. And as i wrote in the commit: There is !NO! test available to test it.

looking to the docs of Delphi http://docs.embarcadero.com/products/ra ... Value.html we can access the Value only if the row is not applyed to the server...

So it should work if CachedUpdate is set to true. That point i don't se in your code. OTH i don't think it's working properly yet. Or does it?

Did you ever setup our test-suites? It would help if someone just adds tests, if done once usually the fixes are easy for me. That would save my rare free time. Writing tests usually takes me same time like debugging and fixing. If you are interested in writing tests, plz look at viewtopic.php?f=4&t=3671

I'm a bit puzzled about: How does it work in explicit transactions, the more if they are nested? I can't find something in the emba docs. Anybody else?

[marsupilami]

looking to the docs of Delphi http://docs.embarcadero.com/products/ra ... Value.html we can access the Value only if the row is not applyed to the server...

So it should work if CachedUpdate is set to true. That point i don't se in your code. OTH i don't think it's working properly yet. Or does it?

[...]

I'm a bit puzzled about: How does it work in explicit transactions, the more if they are nested? I can't find something in the emba docs. Anybody else?

In my opinion the two features "Cached Updates" and "(Nested) Transactions" are two distinct features that don't have anything to do with each other. Or at least they don't need to.

CachedUpdates: Cache updates to one dataset in the dataset itself and don't submit them to the database (yet). Apply these updates to the database upon calling ApplyUpdates. Cahced updates on two or more datasets don't interfere with each other.
Minimum required interaction with transactions: None. We simply use the transaction settings that the user currently has set. If the database supports (nested) transactions, we could start a (nested) transaction to get an all or nothing behavior. But that isn't a requirement.

(nested) Transactions: If started, collect updates to the database on the database side. It doesn't matter if the change was sent by a dataset or by an explicit statement or whatever. Upon calling Commit the database will make the changes permanent on an all or nothing basis. Datasets don't have any chance to see what happens there. They assume that any change gets applied and permanent immediately.
Minimum required interaction with cached updates: None - we simply don't care if the user has something cached. Maybe there are good reasons to cache some changes until a transaction was committed or rolled back.

So - the outcome of OldValue should only depend on CachedUpdates and when ApplyUpdates or CancelUpdates were called last. Maybe I get around to writing some (small) test today evening.

[aehimself]

Datasets don't have any chance to see what happens there. They assume that any change gets applied and permanent immediately.

As usual, Jan gets me thinking. I completely agree with this; especially since .OldValue is a property of a field of a dataset. It belongs to the dataset; and the dataset has nothing to do with it's connection or transactions.

So - the outcome of OldValue should only depend on CachedUpdates and when ApplyUpdates or CancelUpdates were called last.

Considering my first quote, this is true as well. But it bugs me a little.
.OldValue is used to compare if a field changed it's value from the original, which is in the database. If we are using transactions, all logic built on modified field values will break - at least we can't rely on them.
To be honest we should test this, how default Delphi components are behaving.

Anyone has FireDAC available?

OTH i don't think it's working properly yet. Or does it?

Edited because I lied. See my latest post.

[aehimself]

I must have done a typo or something, because after pulling the Git repository it seems it is (so-so) working, according to Jan's considerations:

Code: Select all

procedure TForm1.Button1Click(Sender: TObject);
begin
 ZQuery1.Open;
 Try
  ZQuery1.CachedUpdates := True;
//  ZConnection1.StartTransaction;
  Try
   ZQuery1.Edit;
   ZQuery1.FieldByName('f').AsLargeInt := ZQuery1.FieldByName('f').AsLargeInt + 1;
   ZQuery1.Post;
   If ZQuery1.FieldByName('f').Value = ZQuery1.FieldByName('f').OldValue Then ShowMessage('Not good!');
//   If ZQuery1.FieldByName('f').Value = ZQuery1.FieldByName('f').OldValue Then ShowMessage('Not good for the second time!');
  Finally
   ZQuery1.CancelUpdates; // ZQuery1.ApplyUpdates;
//   ZConnection1.Rollback;
   ZQuery1.CachedUpdates := False;
  End;
 Finally
  ZConnection1.Disconnect;
 End;
end;

Works as it should, .Value is NOT equal to .OldValue and therefore no message pops up. This is correct behavior.
Change .CachedUpdates to .StartTransaction and .Rollback; both .OldValue and .Value will show the new value. While this still bugs me a lot I can understand if this is working as designed (since Jan's logic seems to be correct) and I'll have to write my own .DidChange method to consider transactions too.

Now, for the funny thing; .OldValue works and it does not. Remove the comment from the second If and you'll see that the message "Not good for the second time" will pop up. Both .Value and .OldValue will return the original value.
Strangely, though; in this state if you do a ZQuery1.ApplyUpdates the new value will be written to the database correctly.

[EgonHugeist]

Hi aehimself,
hope i got it inbetween. So please test and report your findings.

[aehimself]

Based on the amount of commits I saw that we just opened a can of worms. Sorry about that :)
My attention is diverted to MSSQL and the DBLib issue (a potentional customer is using MSSQL) but I'll make sure to test this thing, too.

Thank you for your hard work!