maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

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