Query.CachedUpdates := False is attempting to double-apply

[aehimself]

Imagine the following scenario:

Code: Select all

CREATE TABLE `temp` (
  `ID` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
  `f` bigint(20) unsigned DEFAULT NULL,
  PRIMARY KEY (`ID`),
  UNIQUE KEY `ID_UNIQUE` (`ID`)
) ENGINE=InnoDB AUTO_INCREMENT=5 DEFAULT CHARSET=utf16;

Now, put one record in it... INSERT INTO temp (f) VALUES (0)

And then:

Code: Select all

 ZQuery1.SQL.Text('SELECT * FROM TEMP');
 ZQuery1.Open;
 ZQuery1.CachedUpdates := True;
 Try
  ZConnection1.StartTransaction;
  ZQuery1.Edit;
  ZQuery1.FieldByName('f').AsInteger := Random(100000);
  ZQuery1.Post;
  If ZQUery1.UpdatesPending Then ZQuery1.ApplyUpdates;
  ZConnection1.Commit;
 Finally
  ZQuery1.CachedUpdates := False;
 End;

This will end up in a:
"Project Project1.exe raised exception class EZSQLException with message '0 record(s) updated. Only one record should have been updated.'."
during setting cachedupdates to false.

SQLMonitor results:

'2020-06-05 16:06:06' cat: Execute, proto: mysql, msg: Native SetAutoCommit False call <- ZConnection1.StartTransaction
'2020-06-05 16:06:06' cat: Prepare, proto: mysql, msg: Statement 6 : SHOW KEYS FROM personal.temp
'2020-06-05 16:06:06' cat: Execute, proto: mysql, msg: Statement 6 : SHOW KEYS FROM personal.temp
'2020-06-05 16:06:06' cat: Prepare, proto: mysql, msg: Statement 7 : UPDATE personal.temp SET f=? WHERE `ID`=?
'2020-06-05 16:06:06' cat: Bind prepared, proto: mysql, msg: Statement 7 : 455,4
'2020-06-05 16:06:06' cat: Execute, proto: mysql, msg: Statement 7 : UPDATE personal.temp SET f=? WHERE `ID`=? <- ZQuery1.ApplyUpdates
'2020-06-05 16:06:06' cat: Execute, proto: mysql, msg: Native Commit call <- ZConnection1.Commit
'2020-06-05 16:06:06' cat: Bind prepared, proto: mysql, msg: Statement 7 : 455,4
'2020-06-05 16:06:06' cat: Execute, proto: mysql, msg: Statement 7 : UPDATE personal.temp SET f=? WHERE `ID`=? <- ZQuery1.CachedUpdates := False;

Stack trace is:
ZAbstractDataSet.TZAbstractDataSet.SetCachedUpdates(False);
ZDbcCachedResultSet.TZAbstractCachedResultSet.SetCachedUpdates(False);
ZDbcCachedResultSet.TZAbstractCachedResultSet.PostUpdates;
ZDbcCachedResultSet.TZAbstractCachedResultSet.PostRowUpdates;
ZDbcMySqlResultSet.TZMySQLCachedResolver.PostUpdates;
ZDbcGenericResolver.TZGenerateSQLCachedResolver.PostUpdates;

I attempted to modify the following:

Code: Select all

procedure TZAbstractCachedResultSet.SetCachedUpdates(Value: Boolean);
begin
  if FCachedUpdates <> Value then
  begin
    FCachedUpdates := Value;
    if not FCachedUpdates And Self.IsPendingUpdates then
      PostUpdates;
  end;
end;

but .IsPendingUpdates still returns True and the same error is thrown.

[aehimself]

.ApplyUpdates chooses like this:

Code: Select all

    if CachedResultSet <> nil then
      if Connection.AutoCommit and
        not ( Connection.TransactIsolationLevel in [tiReadCommitted, tiSerializable] ) then
        CachedResultSet.PostUpdates
      else
        CachedResultSet.PostUpdatesCached;

So the issue only raises if we are using transactions AND cached updates too. The main difference between PostUpdates and PostUpdatesCached is that the non-cached version actually does remove the modified record from FInitialRowsList; cached does not. It also states in a comment:

{**
Posts all saved updates to the server but keeps them cached.
}

When I set the .CachedUpdates property to false, it automatically calls .PostUpdates, which is going through all records in FInitialRowsList and attempts to apply the changes to the database. The issue is, it was already done in ApplyUpdates (and therefore PostUpdatesCached) it was just not removed from FInitialRowsList. Naturally, as there's nothing to update the exception is raised.

Is this by design? Why .PostUpdatedCached should keep the changes, if they are already in the database? Shouldn't ZConnection.Commit notify it's connected datasets to now empty their cached updates, as they were all written out successfully?

At this point I don't even know which is by design, I can't even decide which part to apply a patch to

[aehimself]

...turns out .Commit is supposed to do that. Created a pull request with the fix

[marsupilami]

The pull request got applied
Thanks

[aehimself]

Np, glad I could help

[EgonHugeist]

Create job, thx!

[aehimself]

Story does not end here!!!

Code: Select all

 ZQuery1.CachedUpdates := True;
 ZConnection1.StartTransaction;
 ZQuery1.Edit;
 ZQuery1.FieldByName('f').AsInteger := Random(100000);
 ZQuery1.Post;
 ZConnection1.Commit;
 ZQuery1.CachedUpdates := False;
 ZQuery1.Refresh;
 ShowMessage(ZQuery1.FieldByName('f').AsString);

ZSQLMonitor log:
'2020-06-18 15:06:41' cat: Execute, proto: mysql, msg: Native SetAutoCommit False call
'2020-06-18 15:06:41' cat: Execute, proto: mysql, msg: Native Commit call
'2020-06-18 15:06:41' cat: Execute, proto: mysql, msg: Statement 2 : SELECT * FROM Temp

Notice something funky? Yeah, changes only exist in the dataset, appearing to be committed, but not actually written to the database. Upon the refresh, data is re-read from the database therefore discarding the changes made. ShowMessage will pop up the previous state.

I'll start debugging, just wanted you to know that it's still not good. And it took me 8 days do recognize that my app wrote absolutely NOTHING in the database...!!!!!

[aehimself]

Found the issue, fixed the issue.
Pull requst waiting to be accepted.

[marsupilami]

Applied and synced the changes.
Thank you

[aehimself]

The pleasure is mine. Too bad Egonhugeist wishes to rewrite this part so it’s not going to stay for long I suppose Not like it’s a beautiful piece of code - I feel it a bit hacky. It would be better to force the cached dataset to apply its changes during .Commit, but this is how far my knowledge goes.

In the mean time; it will be working correctly for me and for everyone in the 7.3 branch.

[aehimself]

Beautyful. Now data is flowing in to the database, even quicker than before! An average data processing cycle had been reduced from 800-4000 ms to about... 100-150?

So transactions and cached updates do worth to be used together!