FWD » Core Development » Database

After the following change:

=== modified file 'src/com/goldencode/p2j/persist/RecordBuffer.java'
--- old/src/com/goldencode/p2j/persist/RecordBuffer.java    2024-11-20 08:24:11 +0000
+++ new/src/com/goldencode/p2j/persist/RecordBuffer.java    2024-11-22 08:46:09 +0000
@@ -1377,6 +1377,7 @@
 **                           nursery remove will do the job.
 **     AL2 20241112          Adapt to new nursery remove signature.
 ** 378 AP  20241009          Added new method to reset the sortIndex.
+** 379 AOG 20241121          Updated condition of currentRecord.setActiveBuffer only when invoking setter.
 */

 /*
@@ -12678,18 +12679,17 @@
                }
             }

-            // for any setter method and some getters (e.g., _getClob), the DMO needs access to this buffer
-            // the active buffer is set for each buffer is a single assign and just once for a buffer
-            // used in a batch assign.
-            boolean activeBufferSet = bufferManager.isBufferDirtyBatch(RecordBuffer.this);
-            if (!bufferManager.isBatchAssignMode() || !activeBufferSet)
-            {
-               restoreActiveState = currentRecord.setActiveBuffer(RecordBuffer.this);
-            }
-            
             boolean denormalized = false;
             if (isSetter)
             {
+               // for any setter method and some getters (e.g., _getClob), the DMO needs access to this buffer
+               // the active buffer is set for each buffer is a single assign and just once for a buffer
+               // used in a batch assign.
+               if (!bufferManager.isBatchAssignMode() || !bufferManager.isBufferDirtyBatch(RecordBuffer.this))
+               {
+                  restoreActiveState = currentRecord.setActiveBuffer(RecordBuffer.this);
+               }
+
                pm = dmoInfo.getPropsBySetterMap().get(method);
                if (pm == null)
                {

That reached trunk as rev. 15573. The activeBuffer is set only for setters. Thus _getClob won't work anymore. This should be fixed asap. Maybe we can work-around this specific case somehow - for instance, check if it is about _getClob in Handler.invoke. If so, so a temporary set of the active buffer and restore - just for the scope of the getter.

Also, can you test what happens in the following scenario:

function some-function-call returns char():
   tt2.f1 = 'a'. // getter
   return '';
end.

assign tt.f1 = some-function-call(). // setter

Will tt2 be marked as "dirty batch buffer" because it was used nested in a batch assign? If so, isn't this a problem as the restore won't properly kick in?

isBatchAssignMode looks in the current scope, so in the case from note 1, tt2 won't be added to the dirty batch buffers. If in the some-function-call batch assign mode is used, restore will be performed by the logic of that batch. Overall I don't think this could be an issue.
I looked through the getters in Record and could not find another one (besides _GetCLOB) that needs the activeBuffer to be set, but we need to consider the possibility that such methods will be added or the existing one will be augmented.
I implemented setting the active buffer and restoring it's state for getters used as a standalone call or in a batch assign context.
Also I found some other issues regarding the active buffer and fixed them.
The code is in 9365a/15575.

My understanding of the problem and proposed solution: The problem is that CLOB field getters don't have an active buffer. The implementation shows a new collection in BufferManager to keep the state on what buffers were set as active buffers to their records and use that to manage the restore timing.

Review of 9365a/15575

• The collection of getterActiveBuffers leaks as there is no clear from this collection.
  • Also, I don't fully understand the motivation behind getterActiveBuffers. It seems to me that this stores the buffers that called getClob on assign (batch or not). The main goal of it seems to be the restore on batch end. However, I don't think delaying the restore till the batch end is actually needed. It is also tricky in the context of:

assign tt.f1 = buf1.clobField 
     tt.f2 = buf2.clobField

• The implementation rules out setting the active buffer of the record when resolving buf2.clobField, because it is part of getterActiveBuffers already and its restore was delayed. However, if the buf2 and buf1 buffers imply different collations, this implementation won't work properly.

• Artur, please attempt a less resource demanding solution (like managing getterActiveBuffers) with a more conditional imperative approach: if the getter is for a CLOB, simply set the activeBuffer and restore at the end in the finally block. Unless there is a strong lead on the need of restoring at the end of the batch for such case, then lets keep it simple.

getterActiveBuffers has a similar functionality to dirtyBathBuffers In a simple assign case, we can just check if a getter is getClob, and set the active buffer, the existing logic will restore it's state in the finally, but if is is used in a batch assign, then it's state can't be restored in the invoke of the getter, it has to be at the end of the batch, like the dirtyBathBuffers. I think tracking just the buffers used by getters that need an active buffer is more lightweight that tracking all the buffers that were set as active buffers.
When it comes to multiple buffers being used for the same table:

define temp-table tt no-undo field f1 as clob field f2 as clob.
define temp-table tt1 no-undo field f1 as clob field f2 as clob.
define buffer buf1 for tt1.
create tt1.
find buf1.

create tt.
assign tt.f1 = tt1.f1 tt.f2 = buf1.f2. // tt is in dirtyBatchBuffers, it's state will be restored at the end of the batch.
                                       // tt1 is in getterActiveBuffers, it's state will be restored at the end of the batch.
                                       // buf1 is in getterActiveBuffers, it's state will be restored at the end of the batch.

I made the necessary changes and committed 15576, meanwhile I started a large app unit test module for testing.

Review of 15577:

• I think there is a confusion. I was looking into a branch older than rev. 15573 in which the activeBuffer was set only for setters. Before that, there was an activeBufferSet flag which was a local variable:
  

  boolean activeBufferSet = bufferManager.isBufferDirtyBatch(RecordBuffer.this);

  • I was thinking of resurrecting it and use it in the if conditional. Having it as a class member causes harder management of the flag (e.g. bound buffers that should inherit that flag, nested logic, etc.)

I am OK with the changes in 9365a. Please merge to trunk now.

9365a is merged to trunk as rev 15579.