This project has moved. For the latest updates, please go here.

a bug in SparseIndex??

Sep 30, 2013 at 2:26 AM
Edited Sep 30, 2013 at 2:46 AM
Hello Steven:

when I reading the source codes of SparseIndex, in its FindNode method, I saw codes like below:
        uint nodeIndexAddress = RootNodeIndexAddress;
        byte nodeLevel = RootNodeLevel;
        while (true)
        {
            currentNode = GetNode(nodeLevel);
            currentNode.SetNodeIndex(nodeIndexAddress);
            if (nodeLevel == level)
                return currentNode;
            currentNode.TryGet(m_key, m_value);
            nodeIndexAddress = m_value.Value;
            nodeLevel--;
        }
I believe that is a bug. In my opinion, the correct codes should be like below:
        uint nodeIndexAddress = RootNodeIndexAddress;
        byte nodeLevel = RootNodeLevel;
        while (true)
        {
            currentNode = GetNode(nodeLevel);
            currentNode.SetNodeIndex(nodeIndexAddress);
            if (nodeLevel == level)
                return currentNode;
            currentNode.TryGet(key, m_value);
            nodeIndexAddress = m_value.Value;
            nodeLevel--;
        }
To put it simply, when calling the method "currentNode.TryGet", we should use the parameter "key" other than the member field "m_key". I guess that is a typo.

please let me know whether my finding is correct or not.
thanks a lot.
Developer
Mar 4, 2014 at 12:25 AM
At first, I was hesitant to apply this bug fix, as I was sure that it would have manifested itself by now since Sparse Indexes are a big component of the historian. Turns out I was finally able to produce code that showed this truly was a bug. Thanks for your fix.
Mar 4, 2014 at 1:28 PM
I am glad that my post at 2013 finally got a reply at 2014.
Developer
Mar 4, 2014 at 7:19 PM
When code happens to work, even though it's not supposed to, this usually means that there is a deeper bug somewhere in the code. I had to do a complete code review of SparseIndex to find the root of the problem. Something I chose not to do in 2013 because it was working for my general use case.