Did ZQuery.GotoBookmark break in the near past...?

[aehimself]

I have an application where I'm using a custom 'locking' of a dataset:

Code: Select all

Function LockDataSet(inDataSet: TZAbstractRODataset; inResetFilter: Boolean = False): TDataSetLock;
Begin
 If inDataSet = nil Then Exit;
 If _lockcount.ContainsKey(inDataSet) Then _lockcount[inDataSet] := _lockcount[inDataSet] + 1
   Else _lockcount.Add(inDataSet, 1);
 Result.DataSet := inDataSet;
 Result.Bookmark := inDataSet.GetBookmark;
 Result.AfterScroll := inDataSet.AfterScroll;
 Result.Filter := inDataSet.Filter;
 Result.FilterSet := Result.Filter <> '';
 Result.Sort := inDataSet.SortedFields;
 Result.SortType := inDataSet.SortType;
 Result.SortSet := Result.Sort <> '';
 inDataSet.DisableControls;
 inDataSet.AfterScroll := nil;
 If inResetFilter And Result.FilterSet Then inDataSet.Filter := '';
 If Result.SortSet Then Begin
                        inDataSet.SortType := stIgnored;
                        inDataSet.SortedFields := '';
                        End;
 inDataSet.First;
End;

Procedure UnlockDataSet(inDataSetLock: TDataSetLock);
Begin
 If inDataSetLock.DataSet = nil Then Exit;
 If inDataSetLock.FilterSet Then inDataSetLock.DataSet.Filter := inDataSetLock.Filter;
 If inDataSetLock.SortSet Then Begin
                               inDataSetLock.DataSet.SortedFields := inDataSetLock.Sort;
                               inDataSetLock.DataSet.SortType := inDataSetLock.SortType;
                               End;
 inDataSetLock.DataSet.GotoBookmark(inDataSetLock.Bookmark);
 If _lockcount[inDataSetLock.DataSet] = 1 Then Begin
                                               _lockcount.Remove(inDataSetLock.DataSet);
                                               inDataSetLock.DataSet.EnableControls;
                                               inDataSetLock.DataSet.AfterScroll := inDataSetLock.AfterScroll;
                                               End
   Else _lockcount[inDataSetLock.DataSet] := _lockcount[inDataSetLock.DataSet] - 1;
End;

Nothing fancy, just make sure absolutely nothing is going to be triggered / visually updated until I manipulate the cursor. I often use this nested...
lock1 := LockDataSet(ZQuery1);
lock2 := LockDataSet(ZQuery1);
UnlockDataSet(lock2);
UnlockDataSet(lock1);

The previous build of my application worked flawlessly, but in the new release it started to throw "Bookmark was not found" errors in the unlock's .GotoBookmark. This part of the code was not changed, only that I refreshed the Zeos sources.
Now, the TDataSetLock.Bookmark is initialized by DataSet.GetBookMark, filter is not changed between (so the data is unchanged) but it fails to revert. Is it possible that something broke bookmars for Oracle in the past ~20 days?
Unfortunately - as usual - I can not say which commit worked and which one did not. Previous version of my application was released ~8 days ago, but I'm not sure I was up-to-date with Zeos in Git... especially since I was experimenting with the command line .DCU building...

[marsupilami]

If you have a test application and test data, we could try to create a test and see in which revision it fails when going through the different revisions of Zeos.

@locking da dataset: Usually it should be impossible to interfere for the user or other code as long as you don't call application.processmessages because all user interaction is done using the Windows message queue?

[miab3]

@aehimself,

Much of the change in Oracle was Z7.3 r6557.
Check r6556.

Michal

[aehimself]

I'll start to go back in the Git history to see when it will start to work again. The bookmark data ([3, 0, 0, 0]) is returned by TZAbstractRODataset.GetBookmarkData, which instructs GotoRow to seek to record 206,158,430,211 in a dataset with ~3000 records.

procedure TZAbstractRODataset.GetBookmarkData(
Buffer: TRecordBuffer;
Data: {$IFDEF WITH_BOOKMARKDATA_TBOOKMARK}TBookMark{$ELSE}Pointer{$ENDIF});
begin
PInteger(Data)^ := PZRowBuffer(Buffer)^.Index;
end;

{$IFDEF WITH_InternalGotoBookmark_TBookmark}
procedure TZAbstractRODataset.InternalGotoBookmark(Bookmark: TBookmark);
{$ELSE}
procedure TZAbstractRODataset.InternalGotoBookmark(Bookmark: Pointer);
{$ENDIF}
begin
if not GotoRow(PNativeInt(Bookmark)^) then
raise EZDatabaseError.Create(SBookmarkWasNotFound);
end;

"On 32-bit platforms, NativeInt is equivalent to the Integer type. On 64-bit platforms, NativeInt is equivalent to the Int64 type."
PNativeInt(Bookmark)^ is 206,158,430,211.

Shouldn't these two be the same...?

Edit: I can remember some PNativeInt changes lately....

[aehimself]

Yep, that's the bug.

I'll prepare a pull request in a minute.

[aehimself]

@locking da dataset: Usually it should be impossible to interfere for the user or other code as long as you don't call application.processmessages because all user interaction is done using the Windows message queue?

Ah, I did not see this one. There are two issues. One - yes, I am calling Application.ProcessMessages to be able to receive the notification that the query did load the data. Second - because of this - in the moment anything changes in the master dataset, child datasets are attempting to be refreshed, onScroll events are firing, causing massive AVs as the data is not present / not fully loaded yet.
I am very well aware that this is a bad practice and I already wanted to re-write the whole thing to an event-driven thing instead of an Application.ProcessMessages loop. It would just need so much change to the existing code that I really did not want to even start with it yet.

As for your comment on GitHib, I made further changes to handle bookmarks as PIntegers instead of PNativeInts internally through all TZAbstractRODataset as there were inconsistencies. New commit is visible in the same pull request.

[marsupilami]

As for your comment on GitHib, I made further changes to handle bookmarks as PIntegers instead of PNativeInts internally through all TZAbstractRODataset as there were inconsistencies. New commit is visible in the same pull request.

I appliled the patch and synced it to github. Thanks

[aehimself]

Sure thing.
Soon I'll have to get used to test / debug before asking questions first

[aehimself]

Had enough of not knowing what build I'm using in my applications. When I am refreshing the Zeos repository, my script (re)creates a small unit named ZGitVersion.pas with only one constant, the latest Git commit hash in in. And I added that one constant to the ZEOS_VERSION.

Now all my applications will report:

Successfully connected to database.host. MySQL 8.0.18, client version: 6.1.11, database component version: 7.3.0-a8d26a47

This will come handy if I need to know when something broke

[Fr0sT]

Pretty good if you need that info! I guess you have set it as Git after-pull hook?
In fact, this could be made on per-app basis without touching any component's repo thanks to RAD Studio having pre-compile scripts.

[aehimself]

I guess you have set it as Git after-pull hook?

Yes and no. I have a batch script which is responsible for everything refreshing all local Git branches - it checks out one, stashes changes, pulls from Jan's repository, pushes it back to my GitHub and reapplies the stash (if any). And it does this with all local branches. In addition, it pushes all changes made to 7.3-testing to 7.3-testing-aehimself, and reapplies my changes on top of those.
I just added one more step before syncing 7.3-testing-aehimself back to GitHub; to check the last commit, update the file and reapply my latest checkin with the new file.

In fact, this could be made on per-app basis without touching any component's repo thanks to RAD Studio having pre-compile scripts.

I'll be honest, I never experimented with these until now. I started to use D5 as an IDE and compiler only, handled everything else with batch scripts. And since I have a large collection of those already, it's quicker to just change one to fulfill a new task than learning something completely new