JanusGraph / janusgraph

JanusGraph: an open-source, distributed graph database
https://janusgraph.org
Other
5.32k stars 1.17k forks source link

neq behaviour differs from TinkerGraph #2205

Closed porunov closed 3 years ago

porunov commented 4 years ago

TinkerPop added a test which now fails with JanusGraph. In our case we assume that if there is no property with the specified name in has step with neq predicate than the result is true but TinkerGraph assumes that if there is no such property the result should always be false.

The related discussion with @spmallette: https://github.com/apache/tinkerpop/commit/76fc5c23e72b7ad9faafb1faab836ce1ac1259d2#r42228711

As we need to alight with TinkerPop, I believe we need to change the behaviour on neq step to preserve compatibility.

For confirmed bugs, please report:

gremlin> graph=JanusGraphFactory.open("inmemory")
==>standardjanusgraph[inmemory:[127.0.0.1]]
gremlin> graph.addVertex()
==>v[4344]
gremlin> graph.traversal().V().has("p", neq("v"))
18:51:57 WARN  org.janusgraph.graphdb.transaction.StandardJanusGraphTx  - Query requires iterating over all vertices [()]. For better performance, use indexes
==>v[4344]
gremlin>
gremlin> graph=TinkerGraph.open()
==>tinkergraph[vertices:0 edges:0]
gremlin> graph.addVertex()
==>v[0]
gremlin> graph.traversal().V().has("p", neq("v"))
gremlin> 
rngcntr commented 4 years ago

Responsible for that part of the logic should be this piece of code: https://github.com/JanusGraph/janusgraph/blob/aceff0a39ea70553ce71dad8128b18f79105bd3d/janusgraph-core/src/main/java/org/janusgraph/graphdb/query/QueryUtil.java#L161-L167 Just as a hint for everyone who wants to start investigating.

rngcntr commented 4 years ago

The definition of has('someProp', null) should probably also be changed in order to stay consistent with TinkerPop. Example:

gremlin> graph=JanusGraphFactory.open("inmemory")
==>standardjanusgraph[inmemory:[127.0.0.1]]
gremlin> graph.addVertex()
==>v[4344]
gremlin> graph.traversal().V().has("p", null)
18:51:57 WARN  org.janusgraph.graphdb.transaction.StandardJanusGraphTx  - Query requires iterating over all vertices [()]. For better performance, use indexes
==>v[4344]
gremlin>
gremlin> graph=TinkerGraph.open()
==>tinkergraph[vertices:0 edges:0]
gremlin> graph.addVertex()
==>v[0]
gremlin> graph.traversal().V().has("p", null)
gremlin> 
spmallette commented 4 years ago

Note that null will have meaning for Gremlin in 3.5.0 - https://tinkerpop.apache.org/docs/3.5.0-SNAPSHOT/upgrade/#_null_semantics

li-boxuan commented 4 years ago

In fact, JanusGraph's neq behavior when using index contradicts to its own behavior when using full scan, as shown in https://github.com/JanusGraph/janusgraph/issues/1868#issuecomment-694933702