Hi Varun, I was looking at the code for MDEV-21829, trying to understand whether there are any issues with how Unique object was extended to handle variable-size keys.
== TREE object (the RB tree) == TREE (the structure and functions that operate on it) support variable-sized keys, however the keys "are expected to know" their size. tree_insert() has "uint key_size" argument. This way, tree_insert() knows how many bytes to allocate and copy when putting the key into the tree. The rest of the TREE code doesn't seem to care about the size of the keys. When TREE code needs to compare a couple of keys, it will call TREE::compare callack. The callback only receives key pointers as arguments, that is, it is expected to infer key size from the contents of the key. The same happens when when walking the tree. Only "uchar *key" is passed to tree_walk_action callback. The walk function has to figure out they key size on its own. == Unique == Unique class cannot do the same what TREE functions function do. It needs to be aware of the sizes of the values it is storing: - It will need to pass value size to tree_insert() - It will need to know value size to write it into tmp file. - It will need to read it back when merging. How does it know it? Unique::merge() and Unique::walk() with merge_walk() functions do make certain assumptions about the data format being used. == The review points == Is this bad? I think tight coupling between the Unique object and the specifics of the data format it is storing is bad. What is worse is that currently this coupling is implicit, it is not immediately apparent to somebody just looking at the code. Possible ways out: 1. Explicitly declare in Unique class that variable-sized records must use certain data format. 2. Isolate the Unique from the specifics of the data format used. 2A. Let Unique get the size of the key as argument of unique_add() and then keep it together with the key. This creates some RAM/storage overhead. 2B. Pass a read_size() callback function which would get the size from the data? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp