Skip to content

Commit

Permalink
Fix filtering for spatial in kernel instead of cypher
Browse files Browse the repository at this point in the history
also rename method
  • Loading branch information
OliviaYtterbrink committed Feb 14, 2018
1 parent 824b1ce commit 729bcff
Show file tree
Hide file tree
Showing 19 changed files with 110 additions and 65 deletions.
Expand Up @@ -244,7 +244,7 @@ private PrimitiveLongIterator queryIndexOrEmpty( IndexReader reader, IndexQuery[
Arrays.toString( query ) ), e ); Arrays.toString( query ) ), e );
} }


return reader.hasFullNumberPrecision( query ) return reader.hasFullValuePrecision( query )
? indexedNodeIds : LookupFilter.exactIndexMatches( propertyReader, indexedNodeIds, query ); ? indexedNodeIds : LookupFilter.exactIndexMatches( propertyReader, indexedNodeIds, query );
} }


Expand Down
Expand Up @@ -50,8 +50,6 @@ object InequalitySeekRange {
sealed trait InequalitySeekRange[+V] extends SeekRange[V] { sealed trait InequalitySeekRange[+V] extends SeekRange[V] {
def mapBounds[P](f: V => P): InequalitySeekRange[P] def mapBounds[P](f: V => P): InequalitySeekRange[P]


def existsBound(f: V => Boolean): Boolean

def groupBy[K](f: Bound[V] => K): Map[K, InequalitySeekRange[V]] def groupBy[K](f: Bound[V] => K): Map[K, InequalitySeekRange[V]]


// Test if value falls into this range using the implicitly given ordering // Test if value falls into this range using the implicitly given ordering
Expand All @@ -69,8 +67,6 @@ sealed trait HalfOpenSeekRange[+V] extends InequalitySeekRange[V] {


override def mapBounds[P](f: V => P): HalfOpenSeekRange[P] override def mapBounds[P](f: V => P): HalfOpenSeekRange[P]


override def existsBound(f: V => Boolean): Boolean = bounds.exists(b => f(b.endPoint))

// returns the limit of this half open seek range, i.e. // returns the limit of this half open seek range, i.e.
// the greatest bound if this is a RangeGreaterThan and // the greatest bound if this is a RangeGreaterThan and
// the smallest bound if this is a RangeLessThan // the smallest bound if this is a RangeLessThan
Expand All @@ -86,8 +82,6 @@ sealed trait HalfOpenSeekRange[+V] extends InequalitySeekRange[V] {
final case class RangeBetween[+V](greaterThan: RangeGreaterThan[V], lessThan: RangeLessThan[V]) extends InequalitySeekRange[V] { final case class RangeBetween[+V](greaterThan: RangeGreaterThan[V], lessThan: RangeLessThan[V]) extends InequalitySeekRange[V] {
override def mapBounds[P](f: V => P): RangeBetween[P] = copy(greaterThan = greaterThan.mapBounds(f), lessThan = lessThan.mapBounds(f)) override def mapBounds[P](f: V => P): RangeBetween[P] = copy(greaterThan = greaterThan.mapBounds(f), lessThan = lessThan.mapBounds(f))


override def existsBound(f: V => Boolean): Boolean = greaterThan.existsBound(f) || lessThan.existsBound(f)

override def groupBy[K](f: Bound[V] => K): Map[K, InequalitySeekRange[V]] = { override def groupBy[K](f: Bound[V] => K): Map[K, InequalitySeekRange[V]] = {
val greaterThanBounds = greaterThan.bounds.map(Left(_)) val greaterThanBounds = greaterThan.bounds.map(Left(_))
val lessThanBounds = lessThan.bounds.map(Right(_)) val lessThanBounds = lessThan.bounds.map(Right(_))
Expand Down
Expand Up @@ -26,7 +26,7 @@ import org.neo4j.cypher.internal.runtime.interpreted._
import org.neo4j.cypher.internal.util.v3_4.{CypherTypeException, InternalException} import org.neo4j.cypher.internal.util.v3_4.{CypherTypeException, InternalException}
import org.neo4j.cypher.internal.v3_4.logical.plans._ import org.neo4j.cypher.internal.v3_4.logical.plans._
import org.neo4j.values.AnyValue import org.neo4j.values.AnyValue
import org.neo4j.values.storable.{PointValue, Values} import org.neo4j.values.storable.Values
import org.neo4j.values.virtual.NodeValue import org.neo4j.values.virtual.NodeValue


import scala.collection.GenTraversableOnce import scala.collection.GenTraversableOnce
Expand All @@ -42,19 +42,8 @@ object indexQuery extends GraphElementPropertyFunctions {


// Index exact value seek on single value // Index exact value seek on single value
case SingleQueryExpression(inner) => case SingleQueryExpression(inner) =>
assert(1 == propertyNames.size)
val value: AnyValue = inner(m, state) val value: AnyValue = inner(m, state)
val indexValues = lookupNodes(Seq(value), index) lookupNodes(Seq(value), index)
value match {
case _: PointValue =>
val propertyKeyId = state.query.getPropertyKeyId(propertyNames.head)
indexValues.filter { nv =>
val propertyValue = state.query.nodeOps.getProperty(nv.id(), propertyKeyId)
value.equals(propertyValue)
}
case _ =>
indexValues
}


// Index exact value seek on multiple values, by combining the results of multiple index seeks // Index exact value seek on multiple values, by combining the results of multiple index seeks
case ManyQueryExpression(inner) => case ManyQueryExpression(inner) =>
Expand Down Expand Up @@ -88,17 +77,7 @@ object indexQuery extends GraphElementPropertyFunctions {
case InequalitySeekRangeExpression(innerRange) => case InequalitySeekRangeExpression(innerRange) =>
innerRange.mapBounds(expression => makeValueNeoSafe(expression(m, state)).asObject()) innerRange.mapBounds(expression => makeValueNeoSafe(expression(m, state)).asObject())
} }
range match { index(Seq(range)).toIterator
case r:InequalitySeekRange[_] if r.existsBound { value => value.isInstanceOf[PointValue] } =>
val correctlyTypedRange = r.asInstanceOf[InequalitySeekRange[PointValue]]
val propertyKeyId = state.query.getPropertyKeyId(propertyNames.head)
index(Seq(range)).toIterator.filter { nv =>
val propertyValue = state.query.nodeOps.getProperty(nv.id(), propertyKeyId).asInstanceOf[PointValue]
correctlyTypedRange.includes(propertyValue)(CypherOrdering.BY_POINT)
}
case _ =>
index(Seq(range)).toIterator
}
} }


private def lookupNodes(values: Seq[AnyValue], index: Seq[Any] => GenTraversableOnce[NodeValue]): Iterator[NodeValue] = { private def lookupNodes(values: Seq[AnyValue], index: Seq[Any] => GenTraversableOnce[NodeValue]): Iterator[NodeValue] = {
Expand Down
Expand Up @@ -408,12 +408,6 @@ public boolean acceptsValue( Value value )
PointValue point = (PointValue) value; PointValue point = (PointValue) value;
return point.withinRange( from, fromInclusive, to, toInclusive ); return point.withinRange( from, fromInclusive, to, toInclusive );
} }
else if ( value instanceof LongValue )
{
// This is called from the hilbert space filling curve index which decomposed 2D range into 1D long curve index
// This will return false positives that need to be filtered later
return true;
}
return false; return false;
} }


Expand Down
Expand Up @@ -839,7 +839,7 @@ public long nodeGetFromUniqueIndexSeek(
* a fresh reader that isn't associated with the current transaction and hence will not be * a fresh reader that isn't associated with the current transaction and hence will not be
* automatically closed. */ * automatically closed. */
PrimitiveLongResourceIterator committed = resourceIterator( reader.query( query ), reader ); PrimitiveLongResourceIterator committed = resourceIterator( reader.query( query ), reader );
PrimitiveLongResourceIterator exactMatches = reader.hasFullNumberPrecision( query ) PrimitiveLongResourceIterator exactMatches = reader.hasFullValuePrecision( query )
? committed : LookupFilter.exactIndexMatches( this, state, committed, query ); ? committed : LookupFilter.exactIndexMatches( this, state, committed, query );
PrimitiveLongIterator changesFiltered = PrimitiveLongIterator changesFiltered =
filterIndexStateChangesForSeek( state, exactMatches, index, IndexQuery.asValueTuple( query ) ); filterIndexStateChangesForSeek( state, exactMatches, index, IndexQuery.asValueTuple( query ) );
Expand All @@ -853,7 +853,7 @@ public PrimitiveLongResourceIterator indexQuery( KernelStatement state, IndexDes
StorageStatement storeStatement = state.getStoreStatement(); StorageStatement storeStatement = state.getStoreStatement();
IndexReader reader = storeStatement.getIndexReader( index ); IndexReader reader = storeStatement.getIndexReader( index );
PrimitiveLongResourceIterator committed = reader.query( predicates ); PrimitiveLongResourceIterator committed = reader.query( predicates );
PrimitiveLongResourceIterator exactMatches = reader.hasFullNumberPrecision( predicates ) PrimitiveLongResourceIterator exactMatches = reader.hasFullValuePrecision( predicates )
? committed : LookupFilter.exactIndexMatches( this, state, committed, predicates ); ? committed : LookupFilter.exactIndexMatches( this, state, committed, predicates );


IndexQuery firstPredicate = predicates[0]; IndexQuery firstPredicate = predicates[0];
Expand Down
Expand Up @@ -30,8 +30,8 @@


public class NativeHitIndexProgressor<KEY extends NativeSchemaKey, VALUE extends NativeSchemaValue> implements IndexProgressor public class NativeHitIndexProgressor<KEY extends NativeSchemaKey, VALUE extends NativeSchemaValue> implements IndexProgressor
{ {
private final RawCursor<Hit<KEY,VALUE>,IOException> seeker; final RawCursor<Hit<KEY,VALUE>,IOException> seeker;
private final NodeValueClient client; final NodeValueClient client;
private final Collection<RawCursor<Hit<KEY,VALUE>,IOException>> toRemoveFromOnClose; private final Collection<RawCursor<Hit<KEY,VALUE>,IOException>> toRemoveFromOnClose;
private boolean closed; private boolean closed;


Expand Down
Expand Up @@ -145,7 +145,7 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder
} }


@Override @Override
public abstract boolean hasFullNumberPrecision( IndexQuery... predicates ); public abstract boolean hasFullValuePrecision( IndexQuery... predicates );


abstract void validateQuery( IndexOrder indexOrder, IndexQuery[] predicates ); abstract void validateQuery( IndexOrder indexOrder, IndexQuery[] predicates );


Expand Down
Expand Up @@ -121,7 +121,7 @@ private void initFromForRange( NumberRangePredicate rangePredicate, KEY treeKeyF
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
return true; return true;
} }
Expand Down
@@ -0,0 +1,59 @@
/*
* Copyright (c) 2002-2018 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.index.schema;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Collection;

import org.neo4j.cursor.RawCursor;
import org.neo4j.index.internal.gbptree.Hit;
import org.neo4j.storageengine.api.schema.IndexProgressor;
import org.neo4j.values.storable.Value;

public class SpatialHitIndexProgressor<KEY extends NativeSchemaKey, VALUE extends NativeSchemaValue> extends NativeHitIndexProgressor<KEY, VALUE>
{
SpatialHitIndexProgressor( RawCursor<Hit<KEY,VALUE>,IOException> seeker, NodeValueClient client,
Collection<RawCursor<Hit<KEY,VALUE>,IOException>> toRemoveFromOnClose )
{
super( seeker, client, toRemoveFromOnClose );
}

@Override
public boolean next()
{
try
{
while ( seeker.next() )
{
KEY key = seeker.get().key();
if ( client.acceptNode( key.getEntityId(), (Value[]) null ) )
{
return true;
}
}
return false;
}
catch ( IOException e )
{
throw new UncheckedIOException( e );
}
}
}
Expand Up @@ -140,7 +140,7 @@ private void startSeekForRange( IndexProgressor.NodeValueClient client, Geometry
treeKeyFrom.fromDerivedValue( Long.MIN_VALUE, range.min ); treeKeyFrom.fromDerivedValue( Long.MIN_VALUE, range.min );
treeKeyTo.fromDerivedValue( Long.MAX_VALUE, range.max + 1 ); treeKeyTo.fromDerivedValue( Long.MAX_VALUE, range.max + 1 );
RawCursor<Hit<KEY,VALUE>,IOException> seeker = makeIndexSeeker( treeKeyFrom, treeKeyTo ); RawCursor<Hit<KEY,VALUE>,IOException> seeker = makeIndexSeeker( treeKeyFrom, treeKeyTo );
IndexProgressor hitProgressor = new NativeHitIndexProgressor<>( seeker, client, openSeekers ); IndexProgressor hitProgressor = new SpatialHitIndexProgressor<>( seeker, client, openSeekers );
multiProgressor.initialize( descriptor, hitProgressor, query ); multiProgressor.initialize( descriptor, hitProgressor, query );
} }
} }
Expand All @@ -156,7 +156,27 @@ private void startSeekForRange( IndexProgressor.NodeValueClient client, Geometry
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) void startSeekForInitializedRange( IndexProgressor.NodeValueClient client, KEY treeKeyFrom, KEY treeKeyTo, IndexQuery[] query )
{
if ( layout.compare( treeKeyFrom, treeKeyTo ) > 0 )
{
client.initialize( descriptor, IndexProgressor.EMPTY, query );
return;
}
try
{
RawCursor<Hit<KEY,VALUE>,IOException> seeker = makeIndexSeeker( treeKeyFrom, treeKeyTo );
IndexProgressor hitProgressor = new SpatialHitIndexProgressor<>( seeker, client, openSeekers );
client.initialize( descriptor, hitProgressor, query );
}
catch ( IOException e )
{
throw new UncheckedIOException( e );
}
}

@Override
public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
return false; return false;
} }
Expand Down
Expand Up @@ -164,7 +164,7 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
if ( predicates.length > 1 ) if ( predicates.length > 1 )
{ {
Expand All @@ -176,10 +176,10 @@ public boolean hasFullNumberPrecision( IndexQuery... predicates )
{ {
Value value = ((ExactPredicate) predicate).value(); Value value = ((ExactPredicate) predicate).value();
return selector.select( return selector.select(
nativeReader.hasFullNumberPrecision( predicates ), nativeReader.hasFullValuePrecision( predicates ),
spatialReader.hasFullNumberPrecision( predicates ), spatialReader.hasFullValuePrecision( predicates ),
luceneReader.hasFullNumberPrecision( predicates ), value ); luceneReader.hasFullValuePrecision( predicates ), value );
} }
return predicates[0] instanceof NumberRangePredicate && nativeReader.hasFullNumberPrecision( predicates ); return predicates[0] instanceof NumberRangePredicate && nativeReader.hasFullValuePrecision( predicates );
} }
} }
Expand Up @@ -149,7 +149,7 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
return false; return false;
} }
Expand Down
Expand Up @@ -100,16 +100,17 @@ public final void nodeIndexSeek(
((DefaultNodeValueIndexCursor) cursor).setRead( this ); ((DefaultNodeValueIndexCursor) cursor).setRead( this );
IndexProgressor.NodeValueClient target = (DefaultNodeValueIndexCursor) cursor; IndexProgressor.NodeValueClient target = (DefaultNodeValueIndexCursor) cursor;
IndexReader reader = indexReader( index ); IndexReader reader = indexReader( index );
if ( !reader.hasFullNumberPrecision( query ) ) if ( !reader.hasFullValuePrecision( query ) )
{ {
IndexQuery[] filters = new IndexQuery[query.length]; IndexQuery[] filters = new IndexQuery[query.length];
int j = 0; int j = 0;
for ( IndexQuery q : query ) for ( IndexQuery q : query )
{ {
switch ( q.type() ) switch ( q.type() )
{ {
case rangeGeometric:
case rangeNumeric: case rangeNumeric:
if ( !reader.hasFullNumberPrecision( q ) ) if ( !reader.hasFullValuePrecision( q ) )
{ {
filters[j++] = q; filters[j++] = q;
} }
Expand All @@ -118,7 +119,7 @@ public final void nodeIndexSeek(
Value value = ((IndexQuery.ExactPredicate) q).value(); Value value = ((IndexQuery.ExactPredicate) q).value();
if ( value.valueGroup() == ValueGroup.NUMBER || Values.isArrayValue( value ) || value.valueGroup() == ValueGroup.GEOMETRY ) if ( value.valueGroup() == ValueGroup.NUMBER || Values.isArrayValue( value ) || value.valueGroup() == ValueGroup.GEOMETRY )
{ {
if ( !reader.hasFullNumberPrecision( q ) ) if ( !reader.hasFullValuePrecision( q ) )
{ {
filters[j++] = q; filters[j++] = q;
} }
Expand Down
Expand Up @@ -62,14 +62,12 @@ void query(
IndexQuery... query ) throws IndexNotApplicableKernelException; IndexQuery... query ) throws IndexNotApplicableKernelException;


/** /**
* @param predicates query to determine whether or not index has full number precision for. * @param predicates query to determine whether or not index has full value precision for.
* @return whether or not this reader will only return 100% matching results from {@link #query(IndexQuery...)} * @return whether or not this reader will only return 100% matching results from {@link #query(IndexQuery...)}.
* when calling with predicates involving numbers, such as {@link IndexQuery#exact(int, Object)}
* w/ a {@link Number} or {@link IndexQuery#range(int, Number, boolean, Number, boolean)}.
* If {@code false} is returned this means that the caller of {@link #query(IndexQuery...)} will have to * If {@code false} is returned this means that the caller of {@link #query(IndexQuery...)} will have to
* do additional filtering, double-checking of actual property values, externally. * do additional filtering, double-checking of actual property values, externally.
*/ */
boolean hasFullNumberPrecision( IndexQuery... predicates ); boolean hasFullValuePrecision( IndexQuery... predicates );


IndexReader EMPTY = new IndexReader() IndexReader EMPTY = new IndexReader()
{ {
Expand Down Expand Up @@ -104,7 +102,7 @@ public void close()
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
return true; return true;
} }
Expand Down
Expand Up @@ -294,7 +294,7 @@ public PrimitiveLongResourceIterator query( IndexQuery... predicates )
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
return false; return false;
} }
Expand Down
Expand Up @@ -579,7 +579,7 @@ public void shouldNotDecorateNumberQuerResultsWIthLookupFilterIfIndexHasFullNumb
} ); } );
when( kernelStatement.getStoreStatement() ).thenReturn( storeStatement ); when( kernelStatement.getStoreStatement() ).thenReturn( storeStatement );
IndexReader indexReader = mock( IndexReader.class ); IndexReader indexReader = mock( IndexReader.class );
when( indexReader.hasFullNumberPrecision( any() ) ).thenReturn( true ); when( indexReader.hasFullValuePrecision( any() ) ).thenReturn( true );
when( indexReader.query( any() ) ) when( indexReader.query( any() ) )
.thenAnswer( invocation -> PrimitiveLongResourceCollections.iterator( null, nodeId ) ); .thenAnswer( invocation -> PrimitiveLongResourceCollections.iterator( null, nodeId ) );
when( storeStatement.getFreshIndexReader( any() ) ).thenReturn( indexReader ); when( storeStatement.getFreshIndexReader( any() ) ).thenReturn( indexReader );
Expand Down
Expand Up @@ -72,7 +72,7 @@ public PrimitiveLongResourceIterator query( IndexQuery... predicates )
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
return false; return false;
} }
Expand Down
Expand Up @@ -109,7 +109,7 @@ public void query( IndexProgressor.NodeValueClient client, IndexOrder indexOrder
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
return false; return false;
} }
Expand Down
Expand Up @@ -169,7 +169,7 @@ private Query toLuceneQuery( IndexQuery... predicates ) throws IndexNotApplicabl
} }


@Override @Override
public boolean hasFullNumberPrecision( IndexQuery... predicates ) public boolean hasFullValuePrecision( IndexQuery... predicates )
{ {
return false; return false;
} }
Expand Down

0 comments on commit 729bcff

Please sign in to comment.