Index Tree managing only unique values

[jschmieg]

This modification merges index key object with RecordId, creating unique pair which can be sorted in index.

---

This change is 

[KFilipek]

First to do:
buildscripts/cpplint.py --linelength=120 PATH/TO/FILE

[KFilipek]

Give explanation of what function does with tree/cursor, instead of what will be assigned.
Short explanation for more function is welcome.
Check modified files with lint before commit.

---

Reviewed 4 of 6 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

---

src/pmse_index_cursor.cpp, line 66 at r2 (raw file):

//Find entry in tree which is equal or bigger to input entry
//Locates input cursor on that entry
//Sets _locateFoundDataEnd when result is after last entry in tree
Block comment as previous.

---

src/pmse_index_cursor.cpp, line 113 at r2 (raw file):

    cmp = (_cursor.node->keys[_cursor.index]).getBSON().woCompare(_endState->query.key,_ordering, false);
    if(cmp==0){
        if((_cursor.node->keys[_cursor.index]).loc<_endState->query.loc.repr())

hard to read ".loc<_endState".

---

src/pmse_index_cursor.cpp, line 196 at r2 (raw file):

Quoted 7 lines of code…

> cmp = (endCursor.node->keys[endCursor.index]).getBSON().woCompare(_endState->query.key,_ordering, false); > if(cmp==0){ > if((endCursor.node->keys[endCursor.index]).loc<_endState->query.loc.repr()) > cmp = -1; > else if((endCursor.node->keys[endCursor.index]).loc > _endState->query.loc.repr()) > cmp = 1; > else cmp = 0;

Duplicated code

---

src/pmse_index_cursor.cpp, line 230 at r2 (raw file):

_endState = EndState(stripFieldNames(key),
_forward == inclusive ? RecordId::max() : RecordId::min());
Hard to read. At least one line with parentheses or pass temp variable to function EndState.

---

src/pmse_index_cursor.cpp, line 359 at r2 (raw file):

if(gt)
if(query.woCompare(_cursor.node->keys[_cursor.index].getBSON(), _ordering, false)==0)
next(parts);
Maybe:
gt && query.woCompare(_cursor.node->keys[_cursor.index].getBSON(), _ordering, false) == 0

---

src/pmse_tree.cpp, line 441 at r2 (raw file):

i = index;
num_pointers = node->num_keys + 1;
i++;

i = index + 1; ?

---

src/pmse_tree.cpp, line 443 at r2 (raw file):

    num_pointers = node->num_keys + 1;
    i++;
    for (++i; i < num_pointers; i++) {

i = index;
i++
++i and i++ in for.

---

src/pmse_tree.cpp, line 444 at r2 (raw file):

    i++;
    for (++i; i < num_pointers; i++) {
        node->children_array[i - 1] = node->children_array[i];

Now substraction from i

---

Comments from Reviewable

[KFilipek]

This code delete only half of records.

t = db.remove3;
for ( i = 1; i <= 8; i++ ) { t.save({_id: i, x: i}); };
t.remove({_id: {$lt: 8}});

[KFilipek]

Reviewed 4 of 5 files at r3.
Review status: 5 of 6 files reviewed at latest revision, 10 unresolved discussions.

---

src/pmse_tree.cpp, line 104 at r3 (raw file):

lockNode->_pmutex.unlock();
Multiple unlocking can cause undefined behavior.

---

src/pmse_tree.cpp, line 510 at r3 (raw file):

    persistent_ptr<PmseTreeNode> current = node;

    if (current == nullptr)

If current can be null, this statement node->_pmutex gives you segmentation fault.

---

Comments from Reviewable

[jschmieg]

Review status: 5 of 6 files reviewed at latest revision, 10 unresolved discussions.

---

src/pmse_tree.cpp, line 104 at r3 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

lockNode->_pmutex.unlock();
Multiple unlocking can cause undefined behavior.

I don't see multiple unlocking here. Please give more details.

---

src/pmse_tree.cpp, line 510 at r3 (raw file):

Previously, KFilipek (Krzysztof Filipek) wrote…

If current can be null, this statement node->_pmutex gives you segmentation fault.

Actuay, current cannot be null. it is guarantied by calling functions.

---

Comments from Reviewable

[KFilipek]

but I create issue (deletion).

---

Reviewed 1 of 5 files at r3.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

---

Comments from Reviewable