Zorba

Code review comment for lp:~zorba-coders/zorba/structuralrelationships2

structuralrelationships2
Merge into trunk

Revision history for this message

Federico Cavalieri (fcavalieri) wrote on 2011-10-20:

> > Connection dropped...
> >
> > Do you know why document nodes have a component less than any other kind of
> > node when they are the root of their tree?
> >
>
> I finally found out what you meant (the code is in the OrdPathNode ctor, not
> the OrdPath ctor).
> I think the reason why element node as root has an extra component in its
> ordpath is to support fn:put on element nodes. The w3c tests require that
> whatever is fn:put-ed, is retrievable by fn:doc(), which means that I have to
> put a doc node as parent to the element node. But I think this can be fixed,
> because the element node is going to be copied anyway.

Sorry for the wrong name. I wrote while I was travelling and I made a mistake in
remembering the class name. Hope you didn't lose time figuring that out.
And thanks for making the change.

>
> > Another question, tree ids are expected to be unique among all tree
> currently
> > in the store, or not?
>
> Yes, this is the assumption, although it is not really enforced in the
> simplestore. Why do you ask?
>

At the time I wrote that I was experiencing some bugs in my main branch after trying to merge this one into.
Specifically i was failing to check if two nodes were in the same tree.
Apparently node in different trees had the same tree id.

Since then I investigated the issue and found out that treeids are unique only within their collection or within the collection-less nodes. The store seems to be designed with this in mind and either checks if the tree itself is the same or check the collectionid first.

This behaviour is because XmlTree::setCollection() in node_items.cpp does

theId = collection->createTreeId();

and not:

theId = GET_STORE().createTreeId();

and each collection in turn uses an internally incremented integer starting from 0.

This make sense for me, since it limit the risk of going out of range with the integers, but also means
that almost every single function I wrote should also check the collection id to determine if two nodes are in the same tree. I am doing this modification right now.

Federico
> >
> > Thanks,
> > Federico

Cheers
Federico

> > Connection dropped...
> >
> > Do you know why document nodes have a component less than any other kind of
> > node when they are the root of their tree?
> >
> 
> I finally found out what you meant (the code is in the OrdPathNode ctor, not
> the OrdPath ctor).
> I think the reason why element node as root has an extra component in its
> ordpath is to support fn:put on element nodes. The w3c tests require that
> whatever is fn:put-ed, is retrievable by fn:doc(), which means that I have to
> put a doc node as parent to the element node. But I think this can be fixed,
> because the element node is going to be copied anyway.

Sorry for the wrong name. I wrote while I was travelling and I made a mistake in
remembering the class name. Hope you didn't lose time figuring that out. 
And thanks for making the change.

> 
> > Another question, tree ids are expected to be unique among all tree
> currently
> > in the store, or not?
> 
> Yes, this is the assumption, although it is not really enforced in the
> simplestore. Why do you ask?
>

Since then I investigated the issue and found out that treeids are unique only within their collection or within the collection-less nodes. The store seems to be designed with this in mind and either checks if  the tree itself is the same or check the collectionid first.