FWD » Core Development » Database

• the BufferImpl s register the HandleChain, but at that moment the dynamic attribute is not yet available because the object is not yet fully constructed. However, not the handles should be added to BUFFER list but the RecordBuffer s created. We should add it to list when the buffer is set to dynamic;
• the sorting mechanism must be implemented: use a delimitation pointer, insert temp-table buffers before and permanent ones after. This requires an iteration of the list. Alternatively, we could use two list and after the last temp-table to jump automatically to first permanent table buffer. Since this algorithm must be used only on buffers (I don't know if other resources requires special sorting) the insertion method should be protected and specialized in the extended class.

One more thing: please make the BufferImpl constructor you added protected instead of public. As an implementation class, I want to expose as little of it to the outside world as necessary.

Ovidiu, about HandleChain changes: the buffer-specific code I think should reside in overridden methods in the BufferImpl class.

Constantin Asofiei wrote:

Ovidiu, about HandleChain changes: the buffer-specific code I think should reside in overridden methods in the BufferImpl class.

I know, that was my first thought, too (see my last phrase from note3 above). However, the data that they use is private to HandleChain. I am now thinking how to meet-at-the-midle implementation. If you have any idea that should work, please let me know.

Ovidiu Maxiniuc wrote:

Constantin Asofiei wrote:

Ovidiu, about HandleChain changes: the buffer-specific code I think should reside in overridden methods in the BufferImpl class.

I know, that was my first thought, too (see my last phrase from note3 above). However, the data that they use is private to HandleChain. I am now thinking how to meet-at-the-midle implementation. If you have any idea that should work, please let me know.

Why not add protected workers in HandleChain, to access the private fields? I.e. get/setLast/FirstResource for WorkArea.first/lastResource . You already have setters for the next/PrevSibling, what is missing is a getter which returns the reference directly (not wrapped in a handle instance).

Ovidiu, please merge the 0303c update up to the latest revision of P2J.

Code review 0313a:

• The change to RecordBuffer.getLDBName is OK to leave, but it will necessitate runtime regression testing.
• There is still buffer-specific code in HandleChain (e.g., PERMANENT_TABLE).
• You removed the call to release() from TemporaryBuffer, but you never addressed my question in note 5 above.
• Should BufferImpl.hasNextSibling return different results by instance, or is it meant to return a class-wide true or false? This is a genuine question, not a request to change the implementation.

The rest looks fine. Once you've addressed the above, please post a final update for Constantin's review.

Eric Faulhaber wrote:

Code review 0313a:

• The change to RecordBuffer.getLDBName is OK to leave, but it will necessitate runtime regression testing.

OK

• There is still buffer-specific code in HandleChain (e.g., PERMANENT_TABLE).

I understand. I will move the related code to BufferImpl.

• You removed the call to release() from TemporaryBuffer, but you never addressed my question in note 5 above.

The call to release() was an attempt to fix the issue from respective if block (there is a large comment). I keep it in my testing environment breakpointed, but it was never hit since I running the investigation. Since I don't have a proof of its utility I was not intending to promote the change to review, I manually remove it when creating the updates. Accidentally, it slipped into 0313a.

• Should BufferImpl.hasNextSibling return different results by instance, or is it meant to return a class-wide true or false? This is a genuine question, not a request to change the implementation.

Only dynamic buffers are stored in the HandleChain list. Normally, the buffers should know when they are dynamically created. However, the p2j implementation use the same constructors and only at a later moment, the builder will declare them as dynamic and call interlink() to put them in the list before returning the new resource. So buffers are not stored in chain when the HandleChain constructor is called (hasNextSibling should be false) only after they have been declared as dynamic (hasNextSibling should now return true).

The rest looks fine. Once you've addressed the above, please post a final update for Constantin's review.

OK, thanks.

Ovidiu Maxiniuc wrote:

Constantin, please review the attached update.

Only issue I see is that you have this System.out.println("\tInterlinking BUFFER " + permanentTable); on line 2873 - please comment/remove it.

As I understand from the code, only dynamic buffers are chained, correct?

Constantin Asofiei wrote:

Only issue I see is that you have this System.out.println("\tInterlinking BUFFER " + permanentTable); on line 2873 - please comment/remove it.

It was only for debugging purposes. I will remove it.

As I understand from the code, only dynamic buffers are chained, correct?

Exactly. See OE reference, page 1455:
Note: Only dynamic buffers created with the CREATE BUFFER statement are chained on the SESSION system handle.

Thanks, I will enqueue the update for regression testing.