Use same relation type between different relationship classes

[a-takahashi223]

class RelA(StructuredRel):
    pass


class RelB(StructuredRel):
    pass


class NodeA(StructuredNode):
    b = RelationshipTo('NodeB', 'REL_X', model=RelA)


class NodeB(StructuredNode):
    pass


class NodeC(StructuredNode):
    d = RelationshipTo('NodeD', 'REL_X', model=RelB)


class NodeD(StructuredNode):
    pass

This works with neomodel<4.0.8, but with 4.0.8, install_labels raises RelationshipClassRedefined.

[OlehChyhyryn]

The current 4.0.8 release has a bug already fixed in the master branch.

#604

You can use our .whl package to work around it until the following release (whenever it finally comes):

https://github.com/cchangelabs/neomodel/releases/tag/3.3.4

[a-takahashi223]

With master branch ( 13445c4 ), this bug occurs.

[mariusconjeaud]

Looking at the code, this looks like the expected behaviour to me, in the sense that we prevent a given relationship type (in the Neo4j sense) to be used for two completely unrelated relationship objects. Because then, you might have the same type for two relationships but completely different property sets.

This is technically possible in Neo4j, and is also acceptable if you're in control of your data model ; so even though the issue you describe is expected behaviour, I am not sure it is desired behaviour. I see many use cases where we want to use the same relationship type even though it's a different domain object. For example :

• (:NodeA)-[:HAS_VERSION]->(:NodeAVersion)
• (:NodeB)-[:HAS_VERSION]->(:NodeBVersion)

The above won't be accepted by neomodel if you make HAS_VERSION a direct child of StructuredRel. And... this is where I see a real bug : if your neomodel classes defining these two HAS_VERSION relationships are not direct children of StructuredRel (but still unrelated to each other), then it's accepted by neomodel.
So here this would work :

class VersionBaseRel(StructuredRel):
    pass

class VersionARel(VersionBaseRel):
    pass

class VersionBRel(VersionBaseRel):
    pass

class NodeA(StructuredNode):
    version = RelationshipTo("NodeAVersion", "HAS_VERSION", VersionARel)

class NodeAVersion(StructuredNode):
    pass

class NodeB(StructuredNode):
    version = RelationshipTo("NodeBVersion", "HAS_VERSION", VersionBRel)

class NodeBVersion(StructuredNode):
    pass

So, to summarize... I think there is some thinking to be done, and some fixing regardless of what we decide.

The question being : do we want to be opinionated and refuse users to give different relationships the same type (relationships being defined by a type + the labels of the two nodes it connects) ; or we want to allow freedom on that.

What do you think ?

For reference, the part of the code which is at stake here :

# If the relationship mapping exists then it is attempted
# to be redefined so that it applies to the same label
# In this case, it has to be ensured that the class
# that is overriding the relationship is a descendant
# of the already existing class.
model_from_registry = db._NODE_CLASS_REGISTRY[label_set]
if not issubclass(model, model_from_registry):
    is_parent = issubclass(model_from_registry, model)
            if is_direct_subclass(model, StructuredRel) and not is_parent:
                    raise RelationshipClassRedefined(
                        relation_type, db._NODE_CLASS_REGISTRY, model
                    )
``
`

[OlehChyhyryn]

From the perspective of how we are using the code in the project, relationship labels do not always mean the same properties in the relationship. It's more like a general name, what this label means.

If we have two sets of unrelated nodes, let's name them A, B, C, D:

(A) -[:INCLUDES{prop1, prop2}] -> (B)
(C) -[:INCLUDES(prop3, prop4}] -> (D)

While they are not intersecting with each other, there is no data conflict. Errors exist when you have conflicting labels, like:

(:A:C)-[:INCLUDES]->(:B:D)

In this case, we cannot understand the exact "Includes" relationship we assume.

Neomodel should check only this intersection case when it is unclear how to resolve it.