Deferred Lobs feature suggestion

[MJFShark]

Currently if CachedLobs is false, then lobs are read every time they are accessed.
If CachedLobs is true, lobs are read on record load and cached.

My suggestion is to add an additional "DeferredLobs" option which will cache lobs, but only after they've been read/accessed and only when CachedLobs is true.

So basically the setting would be:

CachedLobs: True DeferredLobs: False
Lobs would be read and cached at record load.

CachedLobs: True DeferredLobs: True
Lobs would read and cached only when accessed the first time.

I totally understand btw that this is a bit complex when we get to the very cool TryKeepDataOnDisconnect feature and/or cached/cloned datasets. Of course the main issue is what to do with lobs that were deferred but never read. I guess there would have to be an indicator (e.g. deferred, not loaded, 'N/A')or something along those lines.

Btw, my main use case is that I'm using an Oracle Cloud database and lob access is brutally slow, so I cache them myself outside the dataset only when accessed the first time and it would be extremely helpful if the dataset could do it.

-Mark

[aehimself]

I don't think it would be that hard to implement. But I suggest do drop "cachedlobs" and introduce a LobCacheMode enum instead. lcmNone, lcmOnLoad, lcmOnAccess.

Easier to make conditions on enums, plus these two booleans would control (almost) the same thing.

Yes, we'd break backwards compatibility, but 8.0 stable is not out yet, anyway.

[MJFShark]

Excellent points!

[MJFShark]

Just wanted to say that I have an initial implementation of this that looks pretty good. I'll post my code after some testing and cleanup.

-Mark

[aehimself]

Noice!

However if you did break backwards compatibility I suggest to wait for Michael to give an amen on it before submitting. He might see more things underlying what we do not.

Also the test suite has to be updated because Jenkins will be upset :)

[MJFShark]

I totally understand and agree! Right now I've made the changes to the dbc layer, such that you can set the LobCacheMode to a default value in code, but keep all existing behavior of CachedLobs. So the initial value of LobCacheMode would be lcmNone, causing the CachedLobs setting to be used instead.

I ran into some issues when adding the TLobCacheMode enum property to the component layer and having it available from the dbc layer. I'm guessing someone more savvy than I needs to decide how that will work.

I'll try to do a pull request for the non-breaking first step that will keep the current behavior, but allow changing in the future.

-Mark

[marsupilami]

I totally understand and agree! Right now I've made the changes to the dbc layer, such that you can set the LobCacheMode to a default value in code, but keep all existing behavior of CachedLobs. So the initial value of LobCacheMode would be lcmNone, causing the CachedLobs setting to be used instead.

Hmm - Michael and you should find an agreement wether this feature is to make it into 8.0 or not. I dislike the idea of having two properties that nearly do the same.

I ran into some issues when adding the TLobCacheMode enum property to the component layer and having it available from the dbc layer. I'm guessing someone more savvy than I needs to decide how that will work.

I would assume that just declaring the property on a TZ(ReadOnly)Dataset would make it available on the component layer and that it would have to be set when opening the query. But it is easy for me to come to wrong conclusions here.

I'll try to do a pull request for the non-breaking first step that will keep the current behavior, but allow changing in the future.

Sounds good

[MJFShark]

No worries. I was absolutely not suggesting there would be two properties in any kind of release version. I just meant that I can put in the ability to defer lob caching without breaking compatibility so that it can be tested and final decisions made.

-Mark

[EgonHugeist]

Mark, had some test regression (if the OCILobs are NULL). Fixed by https://sourceforge.net/p/zeoslib/code-0/7419/.

According the compatibility question:
1. I have no objections if the enumerator will be added to the TZAbstractRODateSet descendants (except TZMemTable -> late fetched lobs are not supported there). Except the final comment.
2. dropping the mentioned doUncachedLob option is ok for me. I'm fine with that. We have so many breaking changes inbetween. One more or less doesn't matter. But this should be documented (@Jan)

The only thing disturbing me is: only Oracle, IB/FB and Postgres OID lobs do support the "streamed" LOB's using a discriptor in any kind. Usually we use a Dataset.Properties string for a behavior which is not portable @all. That than means: as i added the "old" option i also did this mistake.
What you guys think about?

[MJFShark]

Aha! Thanks for that fix! This pointed out one other place where I wasn't checking the return value of GetLob for nil.

In procedure TZCachedResultSet.CacheAllLobs;

could you add the check as follows?

Current := FRowAccessor.GetBlob(ColumnIndex, LastNull);
if (Current <> nil) and not Current.IsCached then

I'm thinking you can do it much faster than I (and I'm not positive how to amend a pull request.)

Thanks!
-Mark

[MJFShark]

[Update - Ignore this - fixed it]

If people could test or just look at what I've done and see if they can see anything wrong I'd appreciate it. In my testing with lcmOnAccess I will occasionally get the error:

raised exception class EZSQLException with message 'SQL Error: OCI_INVALID_HANDLE
Code: -2 Message: OCILobOpen'.

And it's happening as an uncached Lob is being cached, during the CreateFromClob call shown below. I feel like maybe it has something to do with refcounts of the lob object and perhaps someone with more experience can see what I've done wrong. It's frustrating because when it works, it seems to work very well. I'm definitely missing some nuance.

Code: Select all

  Result := FRowAccessor.GetBlob(ColumnIndex, LastWasNull);
  if ((FRowAccessor.LobCacheMode = lcmOnAccess) and (Result <> nil) and not Result.IsCached) then
  begin
    Current := Result;
    if Current.QueryInterface(IZCLob, Clob) = S_OK
    then Newlob := TZLocalMemCLob.CreateFromClob(Clob, FRowAccessor.GetColumnCodePage(ColumnIndex), ConSettings, FOpenLobStreams)
    else Newlob := TZLocalMemBLob.CreateFromBlob(Current, FOpenLobStreams);
    Result := Newlob;
    FRowAccessor.SetBlob(ColumnIndex, Result);
  end;

Thanks!

-Mark

[MJFShark]

Back to the drawing board! Don't bother testing this for a bit, it's not working at all correctly. I was testing with randomly generated clob text and didn't notice that the cached clobs are not showing up on the correct record. Changing the CLOBs to a sequential number shows the problem. This explains why it seemed to me it was working when it was not. I'll report back when I've got it sorted. [Although if someone sees my mistake or has insight, post here, thanks!]

[Update, ignore this, see below!]

-Mark

[MJFShark]

Found it! It turned out not to a problem with the code I put in the previous post, but had to do with using TryKeepDataOnDisconnect combined with testing lcmOnAccess. Setting TryKeepDataOnDisconnect set FCachedLobs to true, which then causes problems with lcmOnAccess. Once we replace the existing CachedLobs setting with the LobCacheMode setting this issue will go away.

The quick fix (if you use lcmOnAccess) is to comment out the line:

// FCachedLobs := FTryKeepDataOnDisconnect or (doCachedLobs in FOptions);

in TZAbstractRODataset.SetTryKeepDataOnDisconnect and to also not use CachedLobs = True while testing lcmOnAccess.

If it's decided how to set LobCacheMode I can do the leg work and also document it. I think the options are:

#1 Property on DataSet objects similar to TryKeepDataOnDisconnect. As pointed out this feature is pretty Oracle specific (I've only been testing on Oracle right now since it's where I need it) and that might be an argument against this as a DataSet property.

#2 An additional option in DataSet.Options (doCacheLobsOnAccess?) and either keep or rename doCachedLobs)

#3 another DSProps_CachedLobs type setting. Note that the answer could be both #2 and #3 similar to the existing CachedLobs feature.

Thanks!

-Mark

[marsupilami]

Hello Mark,

I think, when setting TryKeepDataOnDisconnect, the lobCacheMode should be set to always fetch and cache LOB contents. I think this is what Egonhugeist wanted to do with the line you mentioned. Also I suggest to completely remove doCachedLobs from Zeos 8.0 in favour of your new code. But I assume we would need some new tests for the test suite?
When it comes to implementig this feature, I am not sure. So I follow EgonHugeists advice: Implement it as an option to be set using DataSet.Properties. Maybe it also makes sense to be able to set it on TZConnection and have the datasets inherit it from there if it is not set there?

Best regards,

Jan

[MJFShark]

I think, when setting TryKeepDataOnDisconnect, the lobCacheMode should be set to always fetch and cache LOB contents. I think this is what Egonhugeist wanted to do with the line you mentioned.

My personal need for this is TryKeepDataOnDisconnect = True combined with lcmOnAccess. This is mostly due to my use of Oracle's cloud database with lobs (it's a bit of a worst case scenario, so good for testing.) My preference therefore would be to not force lcmOnLoad when setting TryKeep. I think this gives more flexibility.

Also I suggest to completely remove doCachedLobs from Zeos 8.0 in favour of your new code. But I assume we would need some new tests for the test suite?

I agree.

When it comes to implementig this feature, I am not sure. So I follow EgonHugeists advice: Implement it as an option to be set using DataSet.Properties. Maybe it also makes sense to be able to set it on TZConnection and have the datasets inherit it from there if it is not set there?

So change the existing DSProps_CachedLobs to something like:

Code: Select all

  /// <Alias>LobCacheMode</Alias>
  /// <type>Enumerator</type>
  /// <Values>None|OnLoad|OnAccess</Values>
  /// <usage>Connection,DataSet</usage>
  /// <syntax>Properties.Values[DSProps_LobCacheMode]={value}</syntax>
  /// <summary>How to cache lob types. OnLoad caches lobs on record fetch. OnAccess caches lobs only when accessed.</summary>
  DSProps_LobCacheMode = 'LobCacheMode';

-Mark