Skip to content

Commit

Permalink
Merge pull request #11497 from henriknyman/3.4-support-values-in-comp…
Browse files Browse the repository at this point in the history
…iled-2

Support Values in compiled runtime - part 2
  • Loading branch information
henriknyman committed Apr 18, 2018
2 parents ef5fb9c + dd3a2f6 commit ddbdd39
Show file tree
Hide file tree
Showing 35 changed files with 677 additions and 440 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,9 @@ public void pop( Expression expression )
public void box( Expression expression )
{
//For source code we rely on autoboxing
append( "(/*box*/ " );
expression.accept( this );
append( ")" );
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
import java.util.stream.IntStream;
import java.util.stream.LongStream;

import org.neo4j.cypher.internal.compiler.v3_4.spi.NodeIdWrapper;
import org.neo4j.cypher.internal.compiler.v3_4.spi.RelationshipIdWrapper;
import org.neo4j.cypher.internal.util.v3_4.IncomparableValuesException;
import org.neo4j.cypher.internal.util.v3_4.UnorderableValueException;
import org.neo4j.graphdb.Path;
Expand All @@ -41,6 +39,8 @@
import org.neo4j.values.AnyValue;
import org.neo4j.values.AnyValues;
import org.neo4j.values.storable.Values;
import org.neo4j.values.virtual.VirtualNodeValue;
import org.neo4j.values.virtual.VirtualRelationshipValue;

import static java.lang.String.format;

Expand Down Expand Up @@ -189,17 +189,17 @@ else if ( value instanceof List<?> || value.getClass().isArray() )
{
return LIST;
}
else if ( value instanceof NodeIdWrapper )
else if ( value instanceof VirtualNodeValue )
{
if ( ((NodeIdWrapper) value).id() == -1 )
if ( ((VirtualNodeValue) value).id() == -1 )
{
return VOID;
}
return NODE;
}
else if ( value instanceof RelationshipIdWrapper )
else if ( value instanceof VirtualRelationshipValue )
{
if ( ((RelationshipIdWrapper) value).id() == -1 )
if ( ((VirtualRelationshipValue) value).id() == -1 )
{
return VOID;
}
Expand Down Expand Up @@ -284,10 +284,10 @@ else if ( lhs instanceof String && rhs instanceof Character )

private static Comparator<Boolean> BOOLEAN_COMPARATOR = Boolean::compareTo;

private static Comparator<NodeIdWrapper> NODE_COMPARATOR = Comparator.comparingLong( NodeIdWrapper::id );
private static Comparator<VirtualNodeValue> NODE_COMPARATOR = Comparator.comparingLong( VirtualNodeValue::id );

private static Comparator<RelationshipIdWrapper> RELATIONSHIP_COMPARATOR =
Comparator.comparingLong( RelationshipIdWrapper::id );
private static Comparator<VirtualRelationshipValue> RELATIONSHIP_COMPARATOR =
Comparator.comparingLong( VirtualRelationshipValue::id );

// TODO test
private static Comparator<Path> PATH_COMPARATOR = ( lhs, rhs ) ->
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
import java.util.Arrays;
import java.util.HashMap;

import org.neo4j.cypher.internal.compiler.v3_4.spi.NodeIdWrapper;
import org.neo4j.cypher.internal.compiler.v3_4.spi.RelationshipIdWrapper;
import org.neo4j.cypher.internal.util.v3_4.IncomparableValuesException;
import org.neo4j.kernel.impl.util.ValueUtils;
import org.neo4j.values.virtual.VirtualValues;

import static java.lang.String.format;

Expand All @@ -45,12 +45,12 @@ public class CypherOrderabilityTest
new HashMap<Long,Long>(),

// NODE
(NodeIdWrapper) () -> 1,
(NodeIdWrapper) () -> 2,
VirtualValues.node( 1 ),
VirtualValues.node( 2 ),

// RELATIONSHIP
(RelationshipIdWrapper) () -> 1,
(RelationshipIdWrapper) () -> 2,
VirtualValues.relationship( 1 ),
VirtualValues.relationship( 2 ),

// LIST
new String[]{"boo"},
Expand All @@ -67,15 +67,15 @@ public class CypherOrderabilityTest
new long[]{1, 2, 3, Long.MIN_VALUE},
new int[]{1, 2, 3, Integer.MIN_VALUE},
new Object[]{1L, 2, 3, Double.NaN},
new Object[]{1L, 2, 3, (NodeIdWrapper) () -> -1},
ValueUtils.of(new Object[]{1L, 2, 3, null}),
new Long[]{1L, 2L, 4L},
new int[]{2},
new Integer[]{3},
new double[]{4D},
new Double[]{5D},
new float[]{6},
new Float[]{7F},
new Object[]{(RelationshipIdWrapper) () -> -1},
ValueUtils.of(new Object[]{null}),

// TODO: PATH

Expand Down Expand Up @@ -130,7 +130,7 @@ public class CypherOrderabilityTest
Double.NaN,

// VOID
null,
null
};

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,14 @@
import java.util.stream.LongStream;
import java.util.stream.Stream;

import org.neo4j.cypher.internal.compiler.v3_4.spi.NodeIdWrapper;
import org.neo4j.cypher.internal.compiler.v3_4.spi.RelationshipIdWrapper;
import org.neo4j.cypher.internal.util.v3_4.CypherTypeException;
import org.neo4j.cypher.internal.util.v3_4.IncomparableValuesException;
import org.neo4j.graphdb.Node;
import org.neo4j.graphdb.PropertyContainer;
import org.neo4j.graphdb.Relationship;
import org.neo4j.kernel.impl.core.EmbeddedProxySPI;
import org.neo4j.kernel.impl.util.NodeProxyWrappingNodeValue;
import org.neo4j.kernel.impl.util.RelationshipProxyWrappingValue;
import org.neo4j.kernel.impl.util.ValueUtils;
import org.neo4j.values.AnyValue;
import org.neo4j.values.SequenceValue;
Expand Down Expand Up @@ -280,21 +283,9 @@ public static AnyValue materializeAnyResult( EmbeddedProxySPI proxySpi, Object a
{
return NO_VALUE;
}
else if ( anyValue instanceof VirtualNodeValue )
else if ( anyValue instanceof AnyValue )
{
if ( anyValue instanceof NodeValue )
{
return (AnyValue) anyValue;
}
return ValueUtils.fromNodeProxy( proxySpi.newNodeProxy( ((VirtualNodeValue) anyValue).id() ) );
}
else if ( anyValue instanceof VirtualRelationshipValue )
{
if ( anyValue instanceof RelationshipValue )
{
return (AnyValue) anyValue;
}
return ValueUtils.fromRelationshipProxy( proxySpi.newRelationshipProxy( ((VirtualRelationshipValue) anyValue).id() ) );
return materializeAnyValueResult( proxySpi, anyValue );
}
else if ( anyValue instanceof List )
{
Expand Down Expand Up @@ -364,23 +355,42 @@ else if ( anyValue instanceof String[] )
return VirtualValues.list( copy );
}
}
else if ( anyValue instanceof AnyValue )
else
{
// If it is a list or map, run it through a ValueMapper that will create proxy objects for entities (storable arrays should
// TODO: This is expensive and will copy all the data even if no conversion is actually performed.
// Investigate if it pays off to (1) first do a dry run and return as is if no conversion is needed,
// or (2) always create proxy objects directly whenever we create values so we can skip this,
// or (3) wrap with TransformedListValue (existing) or TransformedMapValue (non-existing) that does the conversion lazily
if ( (anyValue instanceof ListValue && !((ListValue) anyValue).storable()) || anyValue instanceof MapValue )
return ValueUtils.of( anyValue );
}
}

// NOTE: This assumes anyValue is an instance of AnyValue
public static AnyValue materializeAnyValueResult( EmbeddedProxySPI proxySpi, Object anyValue )
{
if ( anyValue instanceof VirtualNodeValue )
{
if ( anyValue instanceof NodeValue )
{
return CompiledMaterializeValueMapper.mapAnyValue( proxySpi, (AnyValue) anyValue );
return (AnyValue) anyValue;
}
return (AnyValue) anyValue;
return ValueUtils.fromNodeProxy( proxySpi.newNodeProxy( ((VirtualNodeValue) anyValue).id() ) );
}
else
if ( anyValue instanceof VirtualRelationshipValue )
{
return ValueUtils.of( anyValue );
if ( anyValue instanceof RelationshipValue )
{
return (AnyValue) anyValue;
}
return ValueUtils.fromRelationshipProxy( proxySpi.newRelationshipProxy( ((VirtualRelationshipValue) anyValue).id() ) );
}
// If it is a list or map, run it through a ValueMapper that will create proxy objects for entities if needed.
// This will first do a dry run and return as it is if no conversion is needed.
// If in the future we will always create proxy objects directly whenever we create values we can skip this
// Doing this conversion lazily instead, by wrapping with TransformedListValue or TransformedMapValue is probably not a
// good idea because of the complexities involved (see TOMBSTONE in VirtualValues about why TransformedListValue was killed).
// NOTE: There is also a case where a ListValue can be storable (ArrayValueListValue) where no conversion is needed
if ( (anyValue instanceof ListValue && !((ListValue) anyValue).storable()) || anyValue instanceof MapValue )
{
return CompiledMaterializeValueMapper.mapAnyValue( proxySpi, (AnyValue) anyValue );
}
return (AnyValue) anyValue;
}

public static NodeValue materializeNodeValue( EmbeddedProxySPI proxySpi, Object anyValue )
Expand All @@ -394,6 +404,10 @@ else if ( anyValue instanceof VirtualNodeValue )
{
return ValueUtils.fromNodeProxy( proxySpi.newNodeProxy( ((VirtualNodeValue) anyValue).id() ) );
}
else if ( anyValue instanceof Node )
{
return ValueUtils.fromNodeProxy( (Node) anyValue );
}
throw new IllegalArgumentException( "Do not know how to materialize node value from type " + anyValue.getClass().getName() );
}

Expand All @@ -408,6 +422,10 @@ else if ( anyValue instanceof VirtualRelationshipValue )
{
return ValueUtils.fromRelationshipProxy( proxySpi.newRelationshipProxy( ((VirtualRelationshipValue) anyValue).id() ) );
}
else if ( anyValue instanceof Relationship )
{
return ValueUtils.fromRelationshipProxy( (Relationship) anyValue );
}
throw new IllegalArgumentException( "Do not know how to materialize relationship value from type " + anyValue.getClass().getName() );
}

Expand Down Expand Up @@ -546,7 +564,7 @@ else if ( list instanceof boolean[] )
}

@SuppressWarnings( "unused" ) // called from compiled code
public static long unboxNodeOrNull( NodeIdWrapper value )
public static long unboxNodeOrNull( VirtualNodeValue value )
{
if ( value == null )
{
Expand All @@ -556,7 +574,7 @@ public static long unboxNodeOrNull( NodeIdWrapper value )
}

@SuppressWarnings( "unused" ) // called from compiled code
public static long unboxRelationshipOrNull( RelationshipIdWrapper value )
public static long unboxRelationshipOrNull( VirtualRelationshipValue value )
{
if ( value == null )
{
Expand All @@ -572,14 +590,6 @@ public static long extractLong( Object obj )
{
return -1L;
}
else if ( obj instanceof NodeIdWrapper )
{
return ((NodeIdWrapper) obj).id();
}
else if ( obj instanceof RelationshipIdWrapper )
{
return ((RelationshipIdWrapper) obj).id();
}
else if ( obj instanceof VirtualNodeValue )
{
return ((VirtualNodeValue) obj).id();
Expand Down Expand Up @@ -611,15 +621,39 @@ public static Object makeValueNeoSafe( Object object )
@SuppressWarnings( "unchecked" )
public static Object mapGetProperty( Object object, String key )
{
try
if ( object instanceof MapValue )
{
MapValue map = (MapValue) object;
return map.get( key );
}
catch ( ClassCastException e )
if ( object instanceof NodeProxyWrappingNodeValue )
{
return Values.of( ((NodeProxyWrappingNodeValue) object).nodeProxy().getProperty( key ) );
}
if ( object instanceof RelationshipProxyWrappingValue )
{
throw new CypherTypeException( "Type mismatch: expected a map but was " + object, e );
return Values.of( ((RelationshipProxyWrappingValue) object).relationshipProxy().getProperty( key ) );
}
if ( object instanceof PropertyContainer ) // Entity that is not wrapped by an AnyValue
{
return Values.of( ((PropertyContainer) object).getProperty( key ) );
}
if ( object instanceof NodeValue )
{
return ((NodeValue) object).properties().get( key );
}
if ( object instanceof RelationshipValue )
{
return ((RelationshipValue) object).properties().get( key );
}
if ( object instanceof Map<?,?> )
{
Map<String,Object> map = (Map<String,Object>) object;
return map.get( key );
}
// NOTE: VirtualNodeValue and VirtualRelationshipValue will fall through to here
// To handle these we would need specialized cursor code
throw new CypherTypeException( String.format( "Type mismatch: expected a map but was %s", object ), null );
}

static class ArrayIterator implements Iterator
Expand Down

0 comments on commit ddbdd39

Please sign in to comment.