Skip to content

Commit

Permalink
Final part of feedback
Browse files Browse the repository at this point in the history 
Changed to IntSets
Added "Integral/Floating Point" step between VALUE and VALUE_GROUP
  • Loading branch information
@SaschaPeukert
SaschaPeukert committed Jul 26, 2018
1 parent e897df4 commit 02be46b
Show file tree
Hide file tree
Showing 4 changed files with 240 additions and 55 deletions.
4 changes: 4 additions & 0 deletions ...ernel-it/src/test/java/org/neo4j/kernel/impl/api/integrationtest/BuiltInProceduresIT.java
View file
Expand Up @@ -149,6 +149,10 @@ public void listProcedures() throws Throwable
equalTo( new Object[]{"db.schema", "db.schema() :: (nodes :: LIST? OF NODE?, relationships :: LIST? " + equalTo( new Object[]{"db.schema", "db.schema() :: (nodes :: LIST? OF NODE?, relationships :: LIST? " +
"OF " + "OF " +
"RELATIONSHIP?)", "Show the schema of the data.", "READ"} ), "RELATIONSHIP?)", "Show the schema of the data.", "READ"} ),
equalTo( new Object[]{"db.propertySchema",
"db.propertySchema() :: (type :: STRING?, nodeLabelsOrRelType :: LIST? OF STRING?, property :: STRING?, cypherType :: STRING?)",
"Show the derived property schema of the data in tabular form.",
"READ"} ),
equalTo( new Object[]{"db.relationshipTypes", "db.relationshipTypes() :: (relationshipType :: " + equalTo( new Object[]{"db.relationshipTypes", "db.relationshipTypes() :: (relationshipType :: " +
"STRING?)", "STRING?)",
"List all relationship types in the database.", "READ"} ), "List all relationship types in the database.", "READ"} ),
Expand Down
160 changes: 125 additions & 35 deletions community/kernel/src/main/java/org/neo4j/kernel/builtinprocs/SchemaCalculator.java
View file
Expand Up @@ -19,16 +19,15 @@
*/ */
package org.neo4j.kernel.builtinprocs; package org.neo4j.kernel.builtinprocs;


import sun.reflect.generics.reflectiveObjects.NotImplementedException; import org.eclipse.collections.api.set.primitive.MutableIntSet;
import org.eclipse.collections.impl.factory.primitive.IntSets;


import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set;
import java.util.stream.Stream; import java.util.stream.Stream;


import org.neo4j.helpers.collection.Pair; import org.neo4j.helpers.collection.Pair;
Expand All @@ -39,32 +38,42 @@
import org.neo4j.internal.kernel.api.PropertyCursor; import org.neo4j.internal.kernel.api.PropertyCursor;
import org.neo4j.internal.kernel.api.Read; import org.neo4j.internal.kernel.api.Read;
import org.neo4j.internal.kernel.api.RelationshipScanCursor; import org.neo4j.internal.kernel.api.RelationshipScanCursor;
import org.neo4j.internal.kernel.api.SchemaRead;
import org.neo4j.internal.kernel.api.TokenRead; import org.neo4j.internal.kernel.api.TokenRead;
import org.neo4j.internal.kernel.api.Transaction; import org.neo4j.internal.kernel.api.Transaction;
import org.neo4j.values.storable.IntegralArray;
import org.neo4j.values.storable.IntegralValue;
import org.neo4j.values.storable.Value; import org.neo4j.values.storable.Value;
import org.neo4j.values.storable.ValueGroup; import org.neo4j.values.storable.ValueGroup;


import static org.neo4j.kernel.builtinprocs.SchemaCalculator.ValueStatus.ANY; import static org.neo4j.kernel.builtinprocs.SchemaCalculator.ValueStatus.ANY;
import static org.neo4j.kernel.builtinprocs.SchemaCalculator.ValueStatus.CONSISTENT_NUMBER_VALUE;
import static org.neo4j.kernel.builtinprocs.SchemaCalculator.ValueStatus.VALUE; import static org.neo4j.kernel.builtinprocs.SchemaCalculator.ValueStatus.VALUE;
import static org.neo4j.kernel.builtinprocs.SchemaCalculator.ValueStatus.VALUE_GROUP; import static org.neo4j.kernel.builtinprocs.SchemaCalculator.ValueStatus.VALUE_GROUP;


public class SchemaCalculator public class SchemaCalculator
{ {
private org.neo4j.internal.kernel.api.Transaction ktx; private org.neo4j.internal.kernel.api.Transaction ktx;


private Map<LabelSet,Set<Integer>> labelSetToPropertyKeysMapping; private Map<LabelSet,MutableIntSet> labelSetToPropertyKeysMapping;
private Map<Pair<LabelSet,Integer>,ValueTypeDecider> labelSetANDNodePropertyKeyIdToValueTypeMapping; private Map<Pair<LabelSet,Integer>,ValueTypeDecider> labelSetANDNodePropertyKeyIdToValueTypeMapping;
private Map<Integer,String> labelIdToLabelNameMapping; private Map<Integer,String> labelIdToLabelNameMapping;
private Map<Integer,String> propertyIdToPropertylNameMapping; private Map<Integer,String> propertyIdToPropertylNameMapping;
private Map<Integer,String> relationshipTypIdToRelationshipNameMapping; private Map<Integer,String> relationshipTypIdToRelationshipNameMapping;
private Map<Integer,Set<Integer>> relationshipTypeIdToPropertyKeysMapping; private Map<Integer,MutableIntSet> relationshipTypeIdToPropertyKeysMapping;
private Map<Pair<Integer,Integer>,ValueTypeDecider> relationshipTypeIdANDPropertyTypeIdToValueTypeMapping; private Map<Pair<Integer,Integer>,ValueTypeDecider> relationshipTypeIdANDPropertyTypeIdToValueTypeMapping;


private final Set<Integer> emptyPropertyIdSet = Collections.unmodifiableSet( Collections.emptySet() ); private final MutableIntSet emptyPropertyIdSet = IntSets.mutable.empty();
private final String ANYVALUE = "ANY"; private final String ANYVALUE = "ANY";
private final String INTEGRAL = "INTEGRAL";
private final String INTEGRAL_ARRAY = "INTEGRALARRAY";
private final String FLOATING_POINT = "FLOATINGPOINT";
private final String FLOATING_POINT_ARRAY = "FLOATINGPOINTARRAY";
private final String NULLABLE = "?"; private final String NULLABLE = "?";
private final String NULLABLE_ANYVALUE = ANYVALUE + NULLABLE; private final String NULLABLE_ANYVALUE = ANYVALUE + NULLABLE;
private final String NULLABLE_INTEGRAL = INTEGRAL + NULLABLE;
private final String NULLABLE_INTEGRAL_ARRAY = INTEGRAL_ARRAY + NULLABLE;
private final String NULLABLE_FLOATING_POINT_ARRAY = FLOATING_POINT_ARRAY + NULLABLE;
private final String NULLABLE_FLOATING_POINT = FLOATING_POINT + NULLABLE;
private final String NODE = "Node"; private final String NODE = "Node";
private final String RELATIONSHIP = "Relationship"; private final String RELATIONSHIP = "Relationship";


Expand Down Expand Up @@ -93,21 +102,19 @@ private List<SchemaInfoResult> produceResultsForRelationships()
String name = relationshipTypIdToRelationshipNameMapping.get( typeId ); String name = relationshipTypIdToRelationshipNameMapping.get( typeId );


// lookup property value types // lookup property value types
Set<Integer> propertyIds = relationshipTypeIdToPropertyKeysMapping.get( typeId ); MutableIntSet propertyIds = relationshipTypeIdToPropertyKeysMapping.get( typeId );
if ( propertyIds.size() == 0 ) if ( propertyIds.size() == 0 )
{ {
results.add( new SchemaInfoResult( RELATIONSHIP, Collections.singletonList( name ), null, null ) ); results.add( new SchemaInfoResult( RELATIONSHIP, Collections.singletonList( name ), null, null ) );
} }
else else
{ {
for ( Integer propId : propertyIds ) propertyIds.forEach( propId -> {
{
// lookup propId name and valueGroup // lookup propId name and valueGroup
String propName = propertyIdToPropertylNameMapping.get( propId ); String propName = propertyIdToPropertylNameMapping.get( propId );
ValueTypeDecider valueTypeDecider = relationshipTypeIdANDPropertyTypeIdToValueTypeMapping.get( Pair.of( typeId, propId ) ); ValueTypeDecider valueTypeDecider = relationshipTypeIdANDPropertyTypeIdToValueTypeMapping.get( Pair.of( typeId, propId ) );
results.add( new SchemaInfoResult( RELATIONSHIP, Collections.singletonList( name ), propName, results.add( new SchemaInfoResult( RELATIONSHIP, Collections.singletonList( name ), propName, valueTypeDecider.getCypherTypeString() ) );
valueTypeDecider.getCypherTypeString() ) ); } );
}
} }
} }
return results; return results;
Expand All @@ -127,40 +134,30 @@ private List<SchemaInfoResult> produceResultsForNodes()
} }


// lookup property value types // lookup property value types
Set<Integer> propertyIds = labelSetToPropertyKeysMapping.get( labelSet ); MutableIntSet propertyIds = labelSetToPropertyKeysMapping.get( labelSet );
if ( propertyIds.size() == 0 ) if ( propertyIds.size() == 0 )
{ {
results.add( new SchemaInfoResult( NODE, labelNames, null, null ) ); results.add( new SchemaInfoResult( NODE, labelNames, null, null ) );
} }
else else
{ {
for ( Integer propId : propertyIds ) propertyIds.forEach( propId -> {
{
// lookup propId name and valueGroup // lookup propId name and valueGroup
String propName = propertyIdToPropertylNameMapping.get( propId ); String propName = propertyIdToPropertylNameMapping.get( propId );
ValueTypeDecider valueTypeDecider = labelSetANDNodePropertyKeyIdToValueTypeMapping.get( Pair.of( labelSet, propId ) ); ValueTypeDecider valueTypeDecider = labelSetANDNodePropertyKeyIdToValueTypeMapping.get( Pair.of( labelSet, propId ) );
results.add( new SchemaInfoResult( NODE, labelNames, propName, valueTypeDecider.getCypherTypeString() ) ); results.add( new SchemaInfoResult( NODE, labelNames, propName, valueTypeDecider.getCypherTypeString() ) );
} } );
} }
} }
return results; return results;
} }


public Stream<SchemaProcedure.GraphResult> calculateGraphResultStream()
{
calculateSchema();
//TODO finish this
throw new NotImplementedException();
}

//TODO: Parallelize this: (Nodes | Rels) and/or more
//TODO: If we would have this schema information in the count store (or somewhere), this could be super fast //TODO: If we would have this schema information in the count store (or somewhere), this could be super fast
private void calculateSchema() private void calculateSchema()
{ {
// this one does most of the work // this one does most of the work
Read dataRead = ktx.dataRead(); Read dataRead = ktx.dataRead();
TokenRead tokenRead = ktx.tokenRead(); TokenRead tokenRead = ktx.tokenRead();
SchemaRead schemaRead = ktx.schemaRead();
CursorFactory cursors = ktx.cursors(); CursorFactory cursors = ktx.cursors();


// setup mappings // setup mappings
Expand Down Expand Up @@ -195,7 +192,7 @@ private void scanEverythingBelongingToRelationships( Read dataRead, CursorFactor
{ {
int typeId = relationshipScanCursor.type(); int typeId = relationshipScanCursor.type();
relationshipScanCursor.properties( propertyCursor ); relationshipScanCursor.properties( propertyCursor );
Set<Integer> propertyIds = new HashSet<>(); // is Set really the best fit here? MutableIntSet propertyIds = IntSets.mutable.empty();


while ( propertyCursor.next() ) while ( propertyCursor.next() )
{ {
Expand All @@ -209,7 +206,7 @@ private void scanEverythingBelongingToRelationships( Read dataRead, CursorFactor
} }
propertyCursor.close(); propertyCursor.close();


Set<Integer> oldPropertyKeySet = relationshipTypeIdToPropertyKeysMapping.getOrDefault( typeId, emptyPropertyIdSet ); MutableIntSet oldPropertyKeySet = relationshipTypeIdToPropertyKeysMapping.getOrDefault( typeId, emptyPropertyIdSet );


// find out which old properties we did not visited and mark them as nullable // find out which old properties we did not visited and mark them as nullable
if ( !(oldPropertyKeySet == emptyPropertyIdSet) ) if ( !(oldPropertyKeySet == emptyPropertyIdSet) )
Expand Down Expand Up @@ -238,7 +235,7 @@ private void scanEverythingBelongingToNodes( Read dataRead, CursorFactory cursor
// each node // each node
LabelSet labels = nodeCursor.labels(); LabelSet labels = nodeCursor.labels();
nodeCursor.properties( propertyCursor ); nodeCursor.properties( propertyCursor );
Set<Integer> propertyIds = new HashSet<>(); // is Set really the best fit here? MutableIntSet propertyIds = IntSets.mutable.empty();


while ( propertyCursor.next() ) while ( propertyCursor.next() )
{ {
Expand All @@ -251,7 +248,7 @@ private void scanEverythingBelongingToNodes( Read dataRead, CursorFactory cursor
} }
propertyCursor.close(); propertyCursor.close();


Set<Integer> oldPropertyKeySet = labelSetToPropertyKeysMapping.getOrDefault( labels, emptyPropertyIdSet ); MutableIntSet oldPropertyKeySet = labelSetToPropertyKeysMapping.getOrDefault( labels, emptyPropertyIdSet );


// find out which old properties we did not visited and mark them as nullable // find out which old properties we did not visited and mark them as nullable
if ( !(oldPropertyKeySet == emptyPropertyIdSet) ) if ( !(oldPropertyKeySet == emptyPropertyIdSet) )
Expand Down Expand Up @@ -298,7 +295,8 @@ private class ValueTypeDecider
private Value concreteValue; private Value concreteValue;
private ValueGroup valueGroup; private ValueGroup valueGroup;
private ValueStatus valueStatus; private ValueStatus valueStatus;
private Boolean isNullable = false; private boolean isNullable;
private Boolean isIntegral; // this is only important if we have a NumberValue or NumberArray. In those cases false means FloatingPoint


ValueTypeDecider( Value v ) ValueTypeDecider( Value v )
{ {
Expand All @@ -309,13 +307,35 @@ private class ValueTypeDecider
this.concreteValue = v; this.concreteValue = v;
this.valueGroup = v.valueGroup(); this.valueGroup = v.valueGroup();
this.valueStatus = VALUE; this.valueStatus = VALUE;

this.isIntegral = isIntegral( v );
} }


private void setNullable( ) private void setNullable( )
{ {
isNullable = true; isNullable = true;
} }


/***
* Checks if the given value is an Integral value, a Floating Point value or none
* @param v the given value
* @return true, if v is an IntegralValue or -Array (e.g. Long), false if v is a FloatingPoint or -Array (e.g. Double)
* or null if v is not neither NUMBER nor NUMBER_ARRAY
*/

private Boolean isIntegral( Value v )
{
if ( v.valueGroup() == ValueGroup.NUMBER_ARRAY )
{
return v instanceof IntegralArray;
}
else if ( v.valueGroup() == ValueGroup.NUMBER )
{
return v instanceof IntegralValue;
}
return null;
}

/* /*
This method translates an ValueTypeDecider into the correct String This method translates an ValueTypeDecider into the correct String
*/ */
Expand All @@ -326,6 +346,36 @@ String getCypherTypeString()
case VALUE: case VALUE:
return isNullable ? concreteValue.getTypeName().toUpperCase() + NULLABLE return isNullable ? concreteValue.getTypeName().toUpperCase() + NULLABLE
: concreteValue.getTypeName().toUpperCase(); : concreteValue.getTypeName().toUpperCase();
case CONSISTENT_NUMBER_VALUE:
if ( isIntegral == null )
{
throw new IllegalStateException( "isIntegral should have been set in this state" );
}
if ( valueGroup == ValueGroup.NUMBER )
{
if ( isIntegral )
{
return isNullable ? NULLABLE_INTEGRAL
: INTEGRAL;
}
else
{
return isNullable ? NULLABLE_FLOATING_POINT
: FLOATING_POINT;
}
}
// NUMBER_ARRAY
if ( isIntegral )
{
return isNullable ? NULLABLE_INTEGRAL_ARRAY
: INTEGRAL_ARRAY;
}
else
{
return isNullable ? NULLABLE_FLOATING_POINT_ARRAY
: FLOATING_POINT_ARRAY;
}

case VALUE_GROUP: case VALUE_GROUP:
return isNullable ? valueGroup.name() + NULLABLE return isNullable ? valueGroup.name() + NULLABLE
: valueGroup.name(); : valueGroup.name();
Expand All @@ -340,8 +390,9 @@ String getCypherTypeString()
/* /*
This method is needed to handle conflicting property values and sets valueStatus accordingly to: This method is needed to handle conflicting property values and sets valueStatus accordingly to:
A) VALUE if current and new value match on class level A) VALUE if current and new value match on class level
B) VALUE_GROUP, if at least the ValueGroups of the current and new values match B) CONSISTENT_NUMBER_VALUE if current and new value are NUMBER or NUMBER_ARRAY and both are integral or both are floating point values
C) ANY, if nothing matches C) VALUE_GROUP if at least the ValueGroups of the current and new values match
D) ANY if nothing matches
*/ */
void compareAndPutValueType( Value newValue ) void compareAndPutValueType( Value newValue )
{ {
Expand All @@ -353,14 +404,29 @@ void compareAndPutValueType( Value newValue )
switch ( valueStatus ) switch ( valueStatus )
{ {
case VALUE: case VALUE:
// check if classes match // check if classes match -> if so, do nothing
if ( !concreteValue.getClass().equals( newValue.getClass() ) ) if ( !concreteValue.getClass().equals( newValue.getClass() ) )
{ {
// Clases don't match -> update needed // Clases don't match -> update needed
if ( valueGroup.equals( newValue.valueGroup() ) ) if ( valueGroup.equals( newValue.valueGroup() ) )
{ {
// same valueGroup -> set that // same valueGroup -> set that (default, can be overriden if they are Numbers and consistency checks out)
valueStatus = VALUE_GROUP; valueStatus = VALUE_GROUP;

if ( valueGroup == ValueGroup.NUMBER_ARRAY || valueGroup == ValueGroup.NUMBER )
{
Boolean newValueIsIntegral = isIntegral( newValue );
if ( isIntegral == null || newValueIsIntegral == null )
{
throw new IllegalStateException(
"isIntegral should have been set in this state and method should have returned non null for new value" );
}
// test consistency
if ( isIntegral == newValueIsIntegral )
{
valueStatus = CONSISTENT_NUMBER_VALUE;
}
}
} }
else else
{ {
Expand All @@ -369,6 +435,29 @@ void compareAndPutValueType( Value newValue )
} }
} }
break; break;

case CONSISTENT_NUMBER_VALUE:
if ( !valueGroup.equals( newValue.valueGroup() ) )
{
// not same valueGroup -> update to AnyValue
valueStatus = ANY;
}
else
{
// same value-group
// -> update to VALUE_GROUP if new value brakes consistency
Boolean newValueIsIntegral = isIntegral( newValue );
if ( isIntegral == null || newValueIsIntegral == null )
{
throw new IllegalStateException(
"isIntegral should have been set in this state and method should have returned non null for new value" );
}
if ( ! (isIntegral == newValueIsIntegral) )
{
valueStatus = VALUE_GROUP;
}
}
break;
case VALUE_GROUP: case VALUE_GROUP:
if ( !valueGroup.equals( newValue.valueGroup() ) ) if ( !valueGroup.equals( newValue.valueGroup() ) )
{ {
Expand All @@ -388,6 +477,7 @@ void compareAndPutValueType( Value newValue )
enum ValueStatus enum ValueStatus
{ {
VALUE, VALUE,
CONSISTENT_NUMBER_VALUE,
VALUE_GROUP, VALUE_GROUP,
ANY ANY
} }
Expand Down
3 changes: 3 additions & 0 deletions community/kernel/src/test/java/org/neo4j/kernel/builtinprocs/BuiltInProceduresTest.java
View file
Expand Up @@ -234,6 +234,9 @@ public void shouldListCorrectBuiltinProcedures() throws Throwable
record( "db.schema", record( "db.schema",
"db.schema() :: (nodes :: LIST? OF NODE?, relationships :: LIST? OF RELATIONSHIP?)", "db.schema() :: (nodes :: LIST? OF NODE?, relationships :: LIST? OF RELATIONSHIP?)",
"Show the schema of the data.", "READ" ), "Show the schema of the data.", "READ" ),
record( "db.propertySchema",
"db.propertySchema() :: (type :: STRING?, nodeLabelsOrRelType :: LIST? OF STRING?, property :: STRING?, cypherType :: STRING?)",
"Show the derived property schema of the data in tabular form.", "READ" ),
record( "db.index.explicit.searchNodes", record( "db.index.explicit.searchNodes",
"db.index.explicit.searchNodes(indexName :: STRING?, query :: ANY?) :: (node :: NODE?, weight :: FLOAT?)", "db.index.explicit.searchNodes(indexName :: STRING?, query :: ANY?) :: (node :: NODE?, weight :: FLOAT?)",
"Search nodes in explicit index. Replaces `START n=node:nodes('key:foo*')`", "READ" ), "Search nodes in explicit index. Replaces `START n=node:nodes('key:foo*')`", "READ" ),
Expand Down

0 comments on commit 02be46b

Please sign in to comment.