Skip to content

Commit

Permalink
Virtual node hack to support db.schema procedure
Browse files Browse the repository at this point in the history 
The db.schema procedure has been implemented using a hack where virtual
nodes and relationships were created with negative ids. This was done
in order to display the results as a graph in the browser, and has been
working well as long as we have been using the Core API types in Cypher.

Upon introducing the Value types, this did not work anymore, as the hacked
virtual node information was lost during conversions.

This commit introduces a hack to pass on the hacked Core API Node/Relationship
implementations to the surface if they have negative id numbers. It should/can
be removed when we introduce support for full stack virtual nodes.

We are not proud...
  • Loading branch information
@fickludd
fickludd committed Sep 6, 2017
1 parent 8a0a83e commit ba4898e
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 36 deletions.
22 changes: 20 additions & 2 deletions ...er/cypher/src/main/java/org/neo4j/cypher/internal/javacompat/BaseToObjectValueWriter.java
View file
Expand Up @@ -94,7 +94,16 @@ public void writeNodeReference( long nodeId ) throws RuntimeException
@Override @Override
public void writeNode( long nodeId, TextArray ignore, MapValue properties ) throws RuntimeException public void writeNode( long nodeId, TextArray ignore, MapValue properties ) throws RuntimeException
{ {
writeValue( newNodeProxyById( nodeId ) ); if ( nodeId >= 0 )
{
writeValue( newNodeProxyById( nodeId ) );
}
}

@Override
public void writeVirtualNodeHack( Object node )
{
writeValue( node );
} }


@Override @Override
Expand All @@ -107,7 +116,16 @@ public void writeEdgeReference( long edgeId ) throws RuntimeException
public void writeEdge( long edgeId, long startNodeId, long endNodeId, TextValue type, MapValue properties ) public void writeEdge( long edgeId, long startNodeId, long endNodeId, TextValue type, MapValue properties )
throws RuntimeException throws RuntimeException
{ {
writeValue( newRelationshipProxyById( edgeId ) ); if ( edgeId >= 0 )
{
writeValue( newRelationshipProxyById( edgeId ) );
}
}

@Override
public void writeVirtualEdgeHack( Object relationship )
{
writeValue( relationship );
} }


@Override @Override
Expand Down
6 changes: 6 additions & 0 deletions ...cypher/src/main/java/org/neo4j/cypher/internal/javacompat/NodeProxyWrappingNodeValue.java
View file
Expand Up @@ -64,6 +64,12 @@ public <E extends Exception> void writeTo( AnyValueWriter<E> writer ) throws E
p = VirtualValues.EMPTY_MAP; p = VirtualValues.EMPTY_MAP;


} }

if ( id() < 0 )
{
writer.writeVirtualNodeHack( node );
}

writer.writeNode( node.getId(), l, p ); writer.writeNode( node.getId(), l, p );
} }


Expand Down
6 changes: 6 additions & 0 deletions ...rc/main/java/org/neo4j/cypher/internal/javacompat/RelationshipProxyWrappingEdgeValue.java
View file
Expand Up @@ -61,6 +61,12 @@ public <E extends Exception> void writeTo( AnyValueWriter<E> writer ) throws E
p = VirtualValues.EMPTY_MAP; p = VirtualValues.EMPTY_MAP;


} }

if ( id() < 0 )
{
writer.writeVirtualEdgeHack( relationship );
}

writer.writeEdge( id(), startNode().id(), endNode().id(), type(), p ); writer.writeEdge( id(), startNode().id(), endNode().id(), type(), p );
} }


Expand Down
10 changes: 6 additions & 4 deletions .../neo4j/cypher/internal/compatibility/v3_3/runtime/helpers/RuntimeTextValueConverter.scala
View file
Expand Up @@ -21,7 +21,7 @@ package org.neo4j.cypher.internal.compatibility.v3_3.runtime.helpers


import org.neo4j.cypher.internal.compatibility.v3_3.runtime.commands.values.KeyToken import org.neo4j.cypher.internal.compatibility.v3_3.runtime.commands.values.KeyToken
import org.neo4j.cypher.internal.spi.v3_3.QueryContext import org.neo4j.cypher.internal.spi.v3_3.QueryContext
import org.neo4j.graphdb.{Node, Path, Relationship} import org.neo4j.graphdb.{Entity, Node, Path, Relationship}


import scala.collection.Map import scala.collection.Map


Expand All @@ -34,7 +34,7 @@ class RuntimeTextValueConverter(scalaValues: RuntimeScalaValueConverter)(implici
def asTextValue(a: Any): String = { def asTextValue(a: Any): String = {
val scalaValue = scalaValues.asShallowScalaValue(a) val scalaValue = scalaValues.asShallowScalaValue(a)
scalaValue match { scalaValue match {
case node: Node => s"$node${props(node)}" case node: Node => s"Node[${node.getId}]${props(node)}"
case relationship: Relationship => s":${relationship.getType.name()}[${relationship.getId}]${props(relationship)}" case relationship: Relationship => s":${relationship.getType.name()}[${relationship.getId}]${props(relationship)}"
case path: Path => path.toString case path: Path => path.toString
case map: Map[_, _] => makeString(map) case map: Map[_, _] => makeString(map)
Expand All @@ -52,15 +52,17 @@ class RuntimeTextValueConverter(scalaValues: RuntimeScalaValueConverter)(implici


private def props(n: Node): String = { private def props(n: Node): String = {
val ops = context.nodeOps val ops = context.nodeOps
val properties = ops.propertyKeyIds(n.getId) val properties = if (isVirtualEntityHack(n)) Iterator.empty else ops.propertyKeyIds(n.getId)
val keyValStrings = properties.map(pkId => s"${context.getPropertyKeyName(pkId)}:${asTextValue(ops.getProperty(n.getId, pkId).asObject())}") val keyValStrings = properties.map(pkId => s"${context.getPropertyKeyName(pkId)}:${asTextValue(ops.getProperty(n.getId, pkId).asObject())}")
keyValStrings.mkString("{", ",", "}") keyValStrings.mkString("{", ",", "}")
} }


private def props(r: Relationship): String = { private def props(r: Relationship): String = {
val ops = context.relationshipOps val ops = context.relationshipOps
val properties = ops.propertyKeyIds(r.getId) val properties = if (isVirtualEntityHack(r)) Iterator.empty else ops.propertyKeyIds(r.getId)
val keyValStrings = properties.map(pkId => s"${context.getPropertyKeyName(pkId)}:${asTextValue(ops.getProperty(r.getId, pkId).asObject())}") val keyValStrings = properties.map(pkId => s"${context.getPropertyKeyName(pkId)}:${asTextValue(ops.getProperty(r.getId, pkId).asObject())}")
keyValStrings.mkString("{", ",", "}") keyValStrings.mkString("{", ",", "}")
} }

private def isVirtualEntityHack(entity:Entity): Boolean = entity.getId < 0
} }
63 changes: 38 additions & 25 deletions community/kernel/src/main/java/org/neo4j/kernel/builtinprocs/SchemaProcedure.java
View file
Expand Up @@ -21,6 +21,7 @@


import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
Expand Down Expand Up @@ -63,8 +64,8 @@ public SchemaProcedure( final GraphDatabaseAPI graphDatabaseAPI, final KernelTra


public GraphResult buildSchemaGraph() public GraphResult buildSchemaGraph()
{ {
final Map<String,NodeImpl> nodes = new HashMap<>(); final Map<String,VirtualNodeHack> nodes = new HashMap<>();
final Map<String,Set<RelationshipImpl>> relationships = new HashMap<>(); final Map<String,Set<VirtualRelationshipHack>> relationships = new HashMap<>();


try ( Statement statement = kernelTransaction.acquireStatement() ) try ( Statement statement = kernelTransaction.acquireStatement() )
{ {
Expand Down Expand Up @@ -122,15 +123,15 @@ public GraphResult buildSchemaGraph()
int relId = readOperations.relationshipTypeGetForName( relationshipTypeGetName ); int relId = readOperations.relationshipTypeGetForName( relationshipTypeGetName );
try ( ResourceIterator<Label> labelsInUse = graphDatabaseAPI.getAllLabelsInUse().iterator() ) try ( ResourceIterator<Label> labelsInUse = graphDatabaseAPI.getAllLabelsInUse().iterator() )
{ {
List<NodeImpl> startNodes = new LinkedList<>(); List<VirtualNodeHack> startNodes = new LinkedList<>();
List<NodeImpl> endNodes = new LinkedList<>(); List<VirtualNodeHack> endNodes = new LinkedList<>();


while ( labelsInUse.hasNext() ) while ( labelsInUse.hasNext() )
{ {
Label labelToken = labelsInUse.next(); Label labelToken = labelsInUse.next();
String labelName = labelToken.name(); String labelName = labelToken.name();
Map<String,Object> properties = new HashMap<>(); Map<String,Object> properties = new HashMap<>();
NodeImpl node = getOrCreateLabel( labelName, properties, nodes ); VirtualNodeHack node = getOrCreateLabel( labelName, properties, nodes );
int labelId = readOperations.labelGetForName( labelName ); int labelId = readOperations.labelGetForName( labelName );


if ( readOperations.countsForRelationship( labelId, relId, ReadOperations.ANY_LABEL ) > 0 ) if ( readOperations.countsForRelationship( labelId, relId, ReadOperations.ANY_LABEL ) > 0 )
Expand All @@ -143,11 +144,11 @@ public GraphResult buildSchemaGraph()
} }
} }


for ( NodeImpl startNode : startNodes ) for ( VirtualNodeHack startNode : startNodes )
{ {
for ( NodeImpl endNode : endNodes ) for ( VirtualNodeHack endNode : endNodes )
{ {
RelationshipImpl relationship = VirtualRelationshipHack relationship =
addRelationship( startNode, endNode, relationshipTypeGetName, relationships ); addRelationship( startNode, endNode, relationshipTypeGetName, relationships );
} }
} }
Expand All @@ -172,22 +173,22 @@ public GraphResult( List<Node> nodes, List<Relationship> relationships )
} }
} }


private NodeImpl getOrCreateLabel( String label, Map<String,Object> properties, private VirtualNodeHack getOrCreateLabel( String label, Map<String,Object> properties,
final Map<String,NodeImpl> nodeMap ) final Map<String,VirtualNodeHack> nodeMap )
{ {
if ( nodeMap.containsKey( label ) ) if ( nodeMap.containsKey( label ) )
{ {
return nodeMap.get( label ); return nodeMap.get( label );
} }
NodeImpl node = new NodeImpl( label, properties ); VirtualNodeHack node = new VirtualNodeHack( label, properties );
nodeMap.put( label, node ); nodeMap.put( label, node );
return node; return node;
} }


private RelationshipImpl addRelationship( NodeImpl startNode, NodeImpl endNode, String relType, private VirtualRelationshipHack addRelationship( VirtualNodeHack startNode, VirtualNodeHack endNode, String relType,
final Map<String,Set<RelationshipImpl>> relationshipMap ) final Map<String,Set<VirtualRelationshipHack>> relationshipMap )
{ {
Set<RelationshipImpl> relationshipsForType; Set<VirtualRelationshipHack> relationshipsForType;
if ( !relationshipMap.containsKey( relType ) ) if ( !relationshipMap.containsKey( relType ) )
{ {
relationshipsForType = new HashSet<>(); relationshipsForType = new HashSet<>();
Expand All @@ -197,19 +198,19 @@ private RelationshipImpl addRelationship( NodeImpl startNode, NodeImpl endNode,
{ {
relationshipsForType = relationshipMap.get( relType ); relationshipsForType = relationshipMap.get( relType );
} }
RelationshipImpl relationship = new RelationshipImpl( startNode, endNode, relType ); VirtualRelationshipHack relationship = new VirtualRelationshipHack( startNode, endNode, relType );
if ( !relationshipsForType.contains( relationship ) ) if ( !relationshipsForType.contains( relationship ) )
{ {
relationshipsForType.add( relationship ); relationshipsForType.add( relationship );
} }
return relationship; return relationship;
} }


private GraphResult getGraphResult( final Map<String,NodeImpl> nodeMap, private GraphResult getGraphResult( final Map<String,VirtualNodeHack> nodeMap,
final Map<String,Set<RelationshipImpl>> relationshipMap ) final Map<String,Set<VirtualRelationshipHack>> relationshipMap )
{ {
List<Relationship> relationships = new LinkedList<>(); List<Relationship> relationships = new LinkedList<>();
for ( Set<RelationshipImpl> relationship : relationshipMap.values() ) for ( Set<VirtualRelationshipHack> relationship : relationshipMap.values() )
{ {
relationships.addAll( relationship ); relationships.addAll( relationship );
} }
Expand All @@ -220,7 +221,7 @@ private GraphResult getGraphResult( final Map<String,NodeImpl> nodeMap,
return graphResult; return graphResult;
} }


private static class RelationshipImpl implements Relationship private static class VirtualRelationshipHack implements Relationship
{ {


private static AtomicLong MIN_ID = new AtomicLong( -1 ); private static AtomicLong MIN_ID = new AtomicLong( -1 );
Expand All @@ -230,7 +231,7 @@ private static class RelationshipImpl implements Relationship
private final Node endNode; private final Node endNode;
private final RelationshipType relationshipType; private final RelationshipType relationshipType;


RelationshipImpl( final NodeImpl startNode, final NodeImpl endNode, final String type ) VirtualRelationshipHack( final VirtualNodeHack startNode, final VirtualNodeHack endNode, final String type )
{ {
this.id = MIN_ID.getAndDecrement(); this.id = MIN_ID.getAndDecrement();
this.startNode = startNode; this.startNode = startNode;
Expand Down Expand Up @@ -265,7 +266,7 @@ public RelationshipType getType()
@Override @Override
public Map<String,Object> getAllProperties() public Map<String,Object> getAllProperties()
{ {
return new HashMap<String,Object>(); return new HashMap<>();
} }


@Override @Override
Expand Down Expand Up @@ -339,18 +340,24 @@ public Map<String,Object> getProperties( String... keys )
{ {
return null; return null;
} }

@Override
public String toString()
{
return String.format( "VirtualRelationshipHack[%s]", id );
}
} }


private static class NodeImpl implements Node private static class VirtualNodeHack implements Node
{ {


private final HashMap<String,Object> propertyMap = new HashMap<String,Object>(); private final HashMap<String,Object> propertyMap = new HashMap<>();


private static AtomicLong MIN_ID = new AtomicLong( -1 ); private static AtomicLong MIN_ID = new AtomicLong( -1 );
private final long id; private final long id;
private final Label label; private final Label label;


NodeImpl( final String label, Map<String,Object> properties ) VirtualNodeHack( final String label, Map<String,Object> properties )
{ {
this.id = MIN_ID.getAndDecrement(); this.id = MIN_ID.getAndDecrement();
this.label = Label.label( label ); this.label = Label.label( label );
Expand All @@ -373,7 +380,7 @@ public Map<String,Object> getAllProperties()
@Override @Override
public Iterable<Label> getLabels() public Iterable<Label> getLabels()
{ {
return Arrays.asList( label ); return Collections.singletonList( label );
} }


@Override @Override
Expand Down Expand Up @@ -549,5 +556,11 @@ public Map<String,Object> getProperties( String... keys )
{ {
return null; return null;
} }

@Override
public String toString()
{
return String.format( "VirtualNodeHack[%s]", id );
}
} }
} }
11 changes: 10 additions & 1 deletion community/values/src/main/java/org/neo4j/values/AnyValueWriter.java
View file
Expand Up @@ -32,7 +32,6 @@
*/ */
public interface AnyValueWriter<E extends Exception> extends ValueWriter<E> public interface AnyValueWriter<E extends Exception> extends ValueWriter<E>
{ {

void writeNodeReference( long nodeId ) throws E; void writeNodeReference( long nodeId ) throws E;


void writeNode( long nodeId, TextArray labels, MapValue properties ) throws E; void writeNode( long nodeId, TextArray labels, MapValue properties ) throws E;
Expand All @@ -54,4 +53,14 @@ public interface AnyValueWriter<E extends Exception> extends ValueWriter<E>
void beginPoint( CoordinateReferenceSystem coordinateReferenceSystem ) throws E; void beginPoint( CoordinateReferenceSystem coordinateReferenceSystem ) throws E;


void endPoint() throws E; void endPoint() throws E;

default void writeVirtualNodeHack( Object node )
{
// do nothing, this is an ugly hack.
}

default void writeVirtualEdgeHack( Object relationship )
{
// do nothing, this is an ugly hack.
}
} }
34 changes: 30 additions & 4 deletions .../src/test/scala/org/neo4j/internal/cypher/acceptance/BuiltInProcedureAcceptanceTest.scala
View file
Expand Up @@ -20,6 +20,9 @@
package org.neo4j.internal.cypher.acceptance package org.neo4j.internal.cypher.acceptance


import org.neo4j.cypher.SyntaxException import org.neo4j.cypher.SyntaxException
import org.neo4j.graphdb.{Label, Node, Relationship}

import scala.collection.JavaConversions._


class BuiltInProcedureAcceptanceTest extends ProcedureCallAcceptanceTest with CypherComparisonSupport { class BuiltInProcedureAcceptanceTest extends ProcedureCallAcceptanceTest with CypherComparisonSupport {


Expand All @@ -39,18 +42,41 @@ class BuiltInProcedureAcceptanceTest extends ProcedureCallAcceptanceTest with Cy
Map("label" -> "C"))) Map("label" -> "C")))
} }


test("db.schema should not throw an exception") { test("should be able to use db.schema") {

// Given
val neo = createLabeledNode("Neo") val neo = createLabeledNode("Neo")
val d1 = createLabeledNode("Department") val d1 = createLabeledNode("Department")
val e1 = createLabeledNode("Employee") val e1 = createLabeledNode("Employee")
relate(e1, d1, "WORKS_AT", "Hallo") relate(e1, d1, "WORKS_AT", "Hallo")
relate(d1, neo, "PART_OF", "Hallo") relate(d1, neo, "PART_OF", "Hallo")


// When
val query = "CALL db.schema()" val query = "CALL db.schema()"
val result = succeedWith(Configs.Procs, query) val result = succeedWith(Configs.Procs, query).toList
println(result.dumpToString())
// Then
result.size should equal(1)

// And then nodes
val nodes = result.head("nodes").asInstanceOf[Seq[Node]]

val nodeState: Set[(List[Label], Map[String,AnyRef])] =
nodes.map(n => (n.getLabels.toList, n.getAllProperties.toMap)).toSet

val empty = new java.util.ArrayList()
nodeState should equal(
Set(
(List(Label.label("Neo")), Map("indexes" -> empty, "constraints" -> empty, "name" -> "Neo")),
(List(Label.label("Department")), Map("indexes" -> empty, "constraints" -> empty, "name" -> "Department")),
(List(Label.label("Employee")), Map("indexes" -> empty, "constraints" -> empty, "name" -> "Employee"))
))

// And then relationships
val relationships = result.head("relationships").asInstanceOf[Seq[Relationship]]


// TODO this is not the proper acceptance test yet val relationshipState: Set[String] = relationships.map(_.getType.name()).toSet
relationshipState should equal(Set("WORKS_AT", "PART_OF"))
} }


test("should not be able to filter as part of standalone call") { test("should not be able to filter as part of standalone call") {
Expand Down

0 comments on commit ba4898e

Please sign in to comment.