Skip to content

Commit

Permalink
Manage resource iterators explicitly
Browse files Browse the repository at this point in the history
Knowledge about iterators' inherent resources must be reflected in
API contracts, not hidden under the hood as implicit "instanceof" check.
  • Loading branch information
Andrei Koval committed Mar 21, 2018
1 parent 81fa131 commit 595d882
Show file tree
Hide file tree
Showing 17 changed files with 202 additions and 224 deletions.
Expand Up @@ -829,19 +829,20 @@ public long nodeGetFromUniqueIndexSeek(
KernelStatement state, SchemaIndexDescriptor index, IndexQuery.ExactPredicate... query ) KernelStatement state, SchemaIndexDescriptor index, IndexQuery.ExactPredicate... query )
throws IndexNotFoundKernelException, IndexNotApplicableKernelException throws IndexNotFoundKernelException, IndexNotApplicableKernelException
{ {
IndexReader reader = state.getStoreStatement().getFreshIndexReader( index ); try ( IndexReader reader = state.getStoreStatement().getFreshIndexReader( index ) )

{
/* Here we have an intricate scenario where we need to return the PrimitiveLongIterator /* Here we have an intricate scenario where we need to return the PrimitiveLongIterator
* since subsequent filtering will happen outside, but at the same time have the ability to * since subsequent filtering will happen outside, but at the same time have the ability to
* close the IndexReader when done iterating over the lookup result. This is because we get * close the IndexReader when done iterating over the lookup result. This is because we get
* 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 = reader.query( query );
PrimitiveLongResourceIterator exactMatches = reader.hasFullValuePrecision( query ) PrimitiveLongResourceIterator exactMatches = reader.hasFullValuePrecision( query )
? committed : LookupFilter.exactIndexMatches( this, state, committed, query ); ? committed : LookupFilter.exactIndexMatches( this, state, committed, query );
PrimitiveLongIterator changesFiltered = PrimitiveLongResourceIterator changesFiltered =
filterIndexStateChangesForSeek( state, exactMatches, index, IndexQuery.asValueTuple( query ) ); filterIndexStateChangesForSeek( state, exactMatches, index, IndexQuery.asValueTuple( query ) );
return single( resourceIterator( changesFiltered, committed ), NO_SUCH_NODE ); return single( resourceIterator( changesFiltered, committed ), NO_SUCH_NODE );
}
} }


@Override @Override
Expand Down
Expand Up @@ -33,7 +33,6 @@
import org.neo4j.collection.primitive.PrimitiveIntSet; import org.neo4j.collection.primitive.PrimitiveIntSet;
import org.neo4j.collection.primitive.PrimitiveLongIterator; import org.neo4j.collection.primitive.PrimitiveLongIterator;
import org.neo4j.collection.primitive.PrimitiveLongObjectMap; import org.neo4j.collection.primitive.PrimitiveLongObjectMap;
import org.neo4j.collection.primitive.PrimitiveLongResourceIterator;
import org.neo4j.collection.primitive.PrimitiveLongSet; import org.neo4j.collection.primitive.PrimitiveLongSet;
import org.neo4j.cursor.Cursor; import org.neo4j.cursor.Cursor;
import org.neo4j.helpers.collection.Iterables; import org.neo4j.helpers.collection.Iterables;
Expand Down Expand Up @@ -1264,7 +1263,7 @@ private PropertyChanges nodePropertyChanges()
} }


@Override @Override
public PrimitiveLongResourceIterator augmentNodesGetAll( PrimitiveLongIterator committed ) public PrimitiveLongIterator augmentNodesGetAll( PrimitiveLongIterator committed )
{ {
return addedAndRemovedNodes().augment( committed ); return addedAndRemovedNodes().augment( committed );
} }
Expand Down

This file was deleted.

Expand Up @@ -21,6 +21,7 @@


import java.util.Iterator; import java.util.Iterator;
import java.util.Set; import java.util.Set;
import javax.annotation.Nullable;


import org.neo4j.collection.primitive.PrimitiveLongCollections.PrimitiveLongBaseIterator; import org.neo4j.collection.primitive.PrimitiveLongCollections.PrimitiveLongBaseIterator;
import org.neo4j.collection.primitive.PrimitiveLongIterator; import org.neo4j.collection.primitive.PrimitiveLongIterator;
Expand All @@ -32,7 +33,7 @@
* If the given source is a Resource, then so is this DiffApplyingPrimitiveLongIterator. * If the given source is a Resource, then so is this DiffApplyingPrimitiveLongIterator.
*/ */


public class DiffApplyingLongIterator extends PrimitiveLongBaseIterator implements PrimitiveLongResourceIterator class DiffApplyingLongIterator extends PrimitiveLongBaseIterator implements PrimitiveLongResourceIterator
{ {
protected enum Phase protected enum Phase
{ {
Expand Down Expand Up @@ -70,15 +71,28 @@ boolean fetchNext( DiffApplyingLongIterator self )
private final Iterator<?> addedElementsIterator; private final Iterator<?> addedElementsIterator;
private final Set<?> addedElements; private final Set<?> addedElements;
private final Set<?> removedElements; private final Set<?> removedElements;
@Nullable
private final Resource resource;
protected Phase phase; protected Phase phase;


DiffApplyingLongIterator( PrimitiveLongIterator source, Set<?> addedElements, Set<?> removedElements ) DiffApplyingLongIterator( PrimitiveLongIterator source, Set<?> addedElements, Set<?> removedElements, @Nullable Resource resource )
{ {
this.source = source; this.source = source;
this.addedElements = addedElements; this.addedElements = addedElements;
this.addedElementsIterator = addedElements.iterator(); this.addedElementsIterator = addedElements.iterator();
this.removedElements = removedElements; this.removedElements = removedElements;
phase = Phase.FILTERED_SOURCE; this.resource = resource;
this.phase = Phase.FILTERED_SOURCE;
}

static PrimitiveLongIterator augment( PrimitiveLongIterator source, Set<?> addedElements, Set<?> removedElements )
{
return new DiffApplyingLongIterator( source, addedElements, removedElements, null );
}

static PrimitiveLongResourceIterator augment( PrimitiveLongResourceIterator source, Set<?> addedElements, Set<?> removedElements )
{
return new DiffApplyingLongIterator( source, addedElements, removedElements, source );
} }


@Override @Override
Expand All @@ -103,20 +117,20 @@ private boolean computeNextFromSourceAndFilter()


private void transitionToAddedElements() private void transitionToAddedElements()
{ {
phase = !addedElementsIterator.hasNext() ? Phase.NO_ADDED_ELEMENTS : Phase.ADDED_ELEMENTS; phase = addedElementsIterator.hasNext() ? Phase.ADDED_ELEMENTS : Phase.NO_ADDED_ELEMENTS;
} }


private boolean computeNextFromAddedElements() private boolean computeNextFromAddedElements()
{ {
return addedElementsIterator.hasNext() && next((Long) addedElementsIterator.next()); return addedElementsIterator.hasNext() && next( (Long) addedElementsIterator.next() );
} }


@Override @Override
public void close() public void close()
{ {
if ( source instanceof Resource ) if ( resource != null )
{ {
((Resource) source).close(); resource.close();
} }
} }
} }
Expand Up @@ -19,16 +19,18 @@
*/ */
package org.neo4j.kernel.impl.util.diffsets; package org.neo4j.kernel.impl.util.diffsets;


import javax.annotation.Nullable;

import org.neo4j.collection.primitive.PrimitiveLongCollections.PrimitiveLongBaseIterator; import org.neo4j.collection.primitive.PrimitiveLongCollections.PrimitiveLongBaseIterator;
import org.neo4j.collection.primitive.PrimitiveLongIterator; import org.neo4j.collection.primitive.PrimitiveLongIterator;
import org.neo4j.collection.primitive.PrimitiveLongResourceIterator;
import org.neo4j.collection.primitive.PrimitiveLongSet; import org.neo4j.collection.primitive.PrimitiveLongSet;
import org.neo4j.graphdb.Resource; import org.neo4j.graphdb.Resource;


/** /**
* Applies a diffset to the provided {@link PrimitiveLongIterator}. * Applies a diffset to the provided {@link PrimitiveLongIterator}.
* If the given source is a {@link Resource}, it will be closed on {@link #close()}.
*/ */
public class DiffApplyingPrimitiveLongIterator extends PrimitiveLongBaseIterator implements Resource class DiffApplyingPrimitiveLongIterator extends PrimitiveLongBaseIterator implements PrimitiveLongResourceIterator
{ {
protected enum Phase protected enum Phase
{ {
Expand Down Expand Up @@ -66,16 +68,29 @@ boolean fetchNext( DiffApplyingPrimitiveLongIterator self )
private final PrimitiveLongIterator addedElementsIterator; private final PrimitiveLongIterator addedElementsIterator;
private final PrimitiveLongSet addedElements; private final PrimitiveLongSet addedElements;
private final PrimitiveLongSet removedElements; private final PrimitiveLongSet removedElements;
protected Phase phase; @Nullable
private final Resource resource;
private Phase phase;


DiffApplyingPrimitiveLongIterator( PrimitiveLongIterator source, PrimitiveLongSet addedElements, private DiffApplyingPrimitiveLongIterator( PrimitiveLongIterator source, PrimitiveLongSet addedElements, PrimitiveLongSet removedElements,
PrimitiveLongSet removedElements ) @Nullable Resource resource )
{ {
this.source = source; this.source = source;
this.addedElements = addedElements; this.addedElements = addedElements;
this.addedElementsIterator = addedElements.iterator(); this.addedElementsIterator = addedElements.iterator();
this.removedElements = removedElements; this.removedElements = removedElements;
phase = Phase.FILTERED_SOURCE; this.resource = resource;
this.phase = Phase.FILTERED_SOURCE;
}

static PrimitiveLongIterator augment( PrimitiveLongIterator source, PrimitiveLongSet addedElements, PrimitiveLongSet removedElements )
{
return new DiffApplyingPrimitiveLongIterator( source, addedElements, removedElements, null );
}

static PrimitiveLongResourceIterator augment( PrimitiveLongResourceIterator source, PrimitiveLongSet addedElements, PrimitiveLongSet removedElements )
{
return new DiffApplyingPrimitiveLongIterator( source, addedElements, removedElements, source );
} }


@Override @Override
Expand All @@ -100,7 +115,7 @@ private boolean computeNextFromSourceAndFilter()


private void transitionToAddedElements() private void transitionToAddedElements()
{ {
phase = !addedElementsIterator.hasNext() ? Phase.NO_ADDED_ELEMENTS : Phase.ADDED_ELEMENTS; phase = addedElementsIterator.hasNext() ? Phase.ADDED_ELEMENTS : Phase.NO_ADDED_ELEMENTS;
} }


private boolean computeNextFromAddedElements() private boolean computeNextFromAddedElements()
Expand All @@ -111,9 +126,9 @@ private boolean computeNextFromAddedElements()
@Override @Override
public void close() public void close()
{ {
if ( source instanceof Resource ) if ( resource != null )
{ {
((Resource) source).close(); resource.close();
} }
} }
} }
Expand Up @@ -29,16 +29,14 @@
* Applies a diffset to the given source {@link RelationshipIterator}. * Applies a diffset to the given source {@link RelationshipIterator}.
* If the given source is a {@link Resource}, then so is this {@link DiffApplyingRelationshipIterator}. * If the given source is a {@link Resource}, then so is this {@link DiffApplyingRelationshipIterator}.
*/ */
public class DiffApplyingRelationshipIterator extends DiffApplyingLongIterator implements RelationshipIterator class DiffApplyingRelationshipIterator extends DiffApplyingLongIterator implements RelationshipIterator
{ {
private final RelationshipVisitor.Home sourceHome; private final RelationshipVisitor.Home sourceHome;
private final RelationshipVisitor.Home addedHome; private final RelationshipVisitor.Home addedHome;


public DiffApplyingRelationshipIterator( RelationshipIterator source, DiffApplyingRelationshipIterator( RelationshipIterator source, Set<?> addedElements, Set<?> removedElements, RelationshipVisitor.Home addedHome )
Set<?> addedElements, Set<?> removedElements,
RelationshipVisitor.Home addedHome )
{ {
super( source, addedElements, removedElements ); super( source, addedElements, removedElements, null );
this.sourceHome = source; this.sourceHome = source;
this.addedHome = addedHome; this.addedHome = addedHome;
} }
Expand Down
Expand Up @@ -23,7 +23,6 @@
import java.util.Set; import java.util.Set;
import java.util.function.Predicate; import java.util.function.Predicate;


import org.neo4j.collection.primitive.PrimitiveIntIterator;
import org.neo4j.collection.primitive.PrimitiveLongIterator; import org.neo4j.collection.primitive.PrimitiveLongIterator;
import org.neo4j.collection.primitive.PrimitiveLongResourceIterator; import org.neo4j.collection.primitive.PrimitiveLongResourceIterator;
import org.neo4j.helpers.collection.Iterables; import org.neo4j.helpers.collection.Iterables;
Expand All @@ -37,7 +36,7 @@
* *
* @param <T> type of elements * @param <T> type of elements
*/ */
public class DiffSets<T> extends SuperDiffSets<T,PrimitiveLongResourceIterator, PrimitiveLongIterator> implements ReadableDiffSets<T> public class DiffSets<T> extends SuperDiffSets<T> implements ReadableDiffSets<T>
{ {
public DiffSets() public DiffSets()
{ {
Expand All @@ -50,21 +49,21 @@ public DiffSets( Set<T> addedElements, Set<T> removedElements )
} }


@Override @Override
public PrimitiveLongResourceIterator augment( final PrimitiveLongIterator source ) public PrimitiveLongIterator augment( final PrimitiveLongIterator source )
{ {
return new DiffApplyingLongIterator( source, added( false ), removed( false ) ); return DiffApplyingLongIterator.augment( source, added( false ), removed( false ) );
} }


@Override @Override
public PrimitiveIntIterator augment( final PrimitiveIntIterator source ) public PrimitiveLongResourceIterator augment( PrimitiveLongResourceIterator source )
{ {
return new DiffApplyingIntIterator( source, added( false ), removed( false ) ); return DiffApplyingLongIterator.augment( source, added( false ), removed( false ) );
} }


@Override @Override
public PrimitiveLongResourceIterator augmentWithRemovals( final PrimitiveLongIterator source ) public PrimitiveLongResourceIterator augmentWithRemovals( PrimitiveLongResourceIterator source )
{ {
return new DiffApplyingLongIterator( source, Collections.emptySet(), removed( false ) ); return DiffApplyingLongIterator.augment( source, Collections.emptySet(), removed( false ) );
} }


@Override @Override
Expand Down

0 comments on commit 595d882

Please sign in to comment.