Sergey, 1. I failed to find LLD for this patch anywhere. That's why I'll try to put together the description of the abstract data structures used by DsMrr_impl. 2. The main abstract data structure here could be called Flexible_LIFO_Buffer. I call it LIFO because it's filled in one direction while it's read always in the opposite direction starting from the end of the buffer. I call it Flexible because it can be extended when we write into it, and it can be shrunk when we read from it. 3. It makes sense to introduce two sub-classes of this data structure: Flexible_LIFO_Buffer_LR, that writes from left to right, and Flexible_LIFO_Buffer_RL, that writes from right to left. 4. You need two buffers: a buffer for keys [+ association] a buffer for rowids [+ association] If one is of the type Flexible_LIFO_Buffer_LR then the other must be of the type Flexible_LIFO_Buffer_RL. 5. It looks like the buffer for rowids should be of the type Flexible_FIFO_Buffer_LR/RL. Yet because we read from the buffer always after sorting we always can employ Flexible_LIFO_Buffer_LR/RL instead with a proper choice of the comparison function used by the sort procedure. 6. I don't mind if you use the name DsMrr_Buffer[_RL/RL] instead of Flexible_FIFO_Buffer_LR/RL. Maybe it will be even better. 7. The class DsMrr_Buffer must contain a method that sorts the elements in the buffer. Some other remarks: 1. Always use mnemonic constants instead of the constant like 1, -1 you used for direction values, or the constant from a enum type. 2. Each member of the class you use must me specified somehow. 3. Do not overload the semantics of the field members of you classes. E.g. key_tuple_length is a constant for any key buffer and should not everbe changed. 4. Your buffers are actually containers of elements. So it's more natural to have read/write methods for these elements. 5. It looks like it's enough to have only one cursor (let's call it 'pos') for your read/write operations. 6. I think that class DsMrr_impl does not have to know anything about the structure/representation of the elements in a buffer. 7. The companion iterator class should iterate over interesting association info. Maybe it make sense to have a counter of the remaining elements that still have to be iterated over. The init method of the iterator class should reset this counter. 8. DsMrr_impl should contain one method that shifts the boundary between the key buffer and the rowid buffer. It should employ the methods of the buffer classes to do this. 9. As you have only two buffers I don't mind if you have instead of two similar classes Flexible_LIFO_Buffer_LR and Flexible_LIFO_Buffer_RL two quite different classes DsMrr_Key_Buffer and DsMrr_Rowid_Buffer. The first one will be of the type Flexible_LIFO_Buffer with the iterator companion class in it. The second class may be of the Flexible_FIFO_Buffer type. However the generic functionality/interface still must be factored out into the base class DsMrr_Buffer. 10. I think it makes sense to skip unnecessary calls of handler functions. 11. Test cases for the new functionality are scarce. 12. Please take care of the bug I discovered earlier and report of which I resent you again (see [Fwd: [Fwd: [Fwd: Another serious problem with 5.3-dsmrr-cpk]]]) Regards, Igor. -------- Original Message -------- Subject: DS-MRR improvements patch ready for review Date: Fri, 20 Aug 2010 09:12:19 +0200 From: Sergey Petrunya <psergey@askmonty.org> To: igor@askmonty.org CC: maria-developers@lists.launchpad.net Hello Igor, Please find attached the combined patch of DS-MRR for clustered PKs and key sorting. The tree is in launchpad and buildbot also: https://code.launchpad.net/~maria-captains/maria/5.3-dsmrr-cpk and all observed buildbot failures in the tree are known to occur without the new code as well. BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog