Set needsValue conditionally in NodeValueCursor

The `DefaultNodeValueIndexCursor` would alwazs (when possible)
set `needsValue` to true and deserialize values even if the caller
of the index operation did not need the property values.

Now, the caller (e.g. NodeIndexSeeker) decides whether values are
needed.

Adds parametrized tests in `NodeTransactionStateTestBase` to hit
both code paths when values and needed and when not.

community/community-it/index-it/src/test/java/org/neo4j/concurrencytest/ConstraintIndexConcurrencyTest.java

@@ -85,8 +85,7 @@ public void shouldNotAllowConcurrentViolationOfConstraint() throws Exception
             Read read = ktx.dataRead();
             try ( NodeValueIndexCursor cursor = ktx.cursors().allocateNodeValueIndexCursor() )
             {
-                read.nodeIndexSeek( ktx.schemaRead().index( labelId, propertyKeyId ), cursor, IndexOrder.NONE,
-                        IndexQuery.exact( index.schema().getPropertyId(),
+                read.nodeIndexSeek( ktx.schemaRead().index( labelId, propertyKeyId ), cursor, IndexOrder.NONE, false, IndexQuery.exact( index.schema().getPropertyId(),
                                 "The value is irrelevant, we just want to perform some sort of lookup against this " +
                                 "index" ) );
             }

community/community-it/index-it/src/test/java/org/neo4j/kernel/impl/api/MultipleOpenCursorsTest.java

@@ -824,7 +824,7 @@ void assertSameContent( List<Long> actual, List<Long> expected )
         NodeValueIndexCursor indexQuery( KernelTransaction ktx, IndexDescriptor indexDescriptor, IndexQuery... indexQueries ) throws KernelException
         {
             NodeValueIndexCursor cursor = ktx.cursors().allocateNodeValueIndexCursor();
-            ktx.dataRead().nodeIndexSeek( indexDescriptor, cursor, IndexOrder.NONE, indexQueries );
+            ktx.dataRead().nodeIndexSeek( indexDescriptor, cursor, IndexOrder.NONE, false, indexQueries );
             return cursor;
         }
     }

community/community-it/index-it/src/test/java/org/neo4j/kernel/impl/index/schema/NativeStringIndexingIT.java

@@ -155,8 +155,7 @@ public void shouldHandleCompositeSizesCloseToTheLimit() throws KernelException
             {
                 ktx.dataRead().nodeIndexSeek( TestIndexDescriptorFactory
                                                       .forLabel( labelId, propertyKeyId1, propertyKeyId2 ),
-                                              cursor, IndexOrder.NONE,
-                                              IndexQuery.exact( propertyKeyId1, string1 ),
+                                              cursor, IndexOrder.NONE, false, IndexQuery.exact( propertyKeyId1, string1 ),
                                               IndexQuery.exact( propertyKeyId2, string2 ) );
                 assertTrue( cursor.next() );
                 assertEquals( node.getId(), cursor.nodeReference() );

community/community-it/index-it/src/test/java/org/neo4j/kernel/impl/newapi/NodeValueIndexCursorLuceneTest.java

@@ -50,4 +50,16 @@ protected boolean spatialRangeSupport()
     {
         return true;
     }
+
+    @Override
+    protected boolean indexProvidesStringValues()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean indexProvidesNumericValues()
+    {
+        return false;
+    }
 }

community/community-it/index-it/src/test/java/org/neo4j/kernel/impl/newapi/NodeValueIndexCursorNative10Test.java

@@ -48,4 +48,16 @@ protected boolean spatialRangeSupport()
     {
         return true;
     }
+
+    @Override
+    protected boolean indexProvidesStringValues()
+    {
+        return false;
+    }
+
+    @Override
+    protected boolean indexProvidesNumericValues()
+    {
+        return true;
+    }
 }

community/community-it/index-it/src/test/java/org/neo4j/kernel/impl/newapi/NodeValueIndexCursorNative20Test.java

@@ -48,4 +48,16 @@ protected boolean spatialRangeSupport()
     {
         return true;
     }
+
+    @Override
+    protected boolean indexProvidesStringValues()
+    {
+        return true;
+    }
+
+    @Override
+    protected boolean indexProvidesNumericValues()
+    {
+        return true;
+    }
 }

community/community-it/kernel-it/src/test/java/org/neo4j/kernel/api/CompositeIndexingIT.java

@@ -337,7 +337,7 @@ private long createNode()
     private NodeValueIndexCursor seek( KernelTransaction transaction ) throws KernelException
     {
         NodeValueIndexCursor cursor = transaction.cursors().allocateNodeValueIndexCursor();
-        transaction.dataRead().nodeIndexSeek( index, cursor, IndexOrder.NONE, exactQuery() );
+        transaction.dataRead().nodeIndexSeek( index, cursor, IndexOrder.NONE, false, exactQuery() );
         return cursor;
     }
 

community/community-it/kernel-it/src/test/java/org/neo4j/kernel/api/index/SimpleIndexPopulatorCompatibility.java

@@ -201,7 +201,7 @@ public void shouldPopulateAndUpdate() throws Exception
                 for ( NodeAndValue entry : Iterables.concat( valueSet1, valueSet2 ) )
                 {
                     NodeValueIterator nodes = new NodeValueIterator();
-                    reader.query( nodes, IndexOrder.NONE, IndexQuery.exact( propertyKeyId, entry.value ) );
+                    reader.query( nodes, IndexOrder.NONE, false , IndexQuery.exact( propertyKeyId, entry.value ) );
                     assertEquals( entry.nodeId, nodes.next() );
                     assertFalse( nodes.hasNext() );
                 }
@@ -231,7 +231,7 @@ private void assertHasAllValues( List<NodeAndValue> values ) throws IOException,
                 for ( NodeAndValue entry : values )
                 {
                     NodeValueIterator nodes = new NodeValueIterator();
-                    reader.query( nodes, IndexOrder.NONE, IndexQuery.exact( propertyKeyId, entry.value ) );
+                    reader.query( nodes, IndexOrder.NONE, false, IndexQuery.exact( propertyKeyId, entry.value ) );
                     assertEquals( entry.nodeId, nodes.next() );
                     assertFalse( nodes.hasNext() );
                 }
@@ -267,7 +267,7 @@ public void shouldProvidePopulatorThatAcceptsDuplicateEntries() throws Exception
                     for ( NodeAndValue entry : valueSet1 )
                     {
                         NodeValueIterator nodes = new NodeValueIterator();
-                        reader.query( nodes, IndexOrder.NONE, IndexQuery.exact( propertyKeyId, entry.value ) );
+                        reader.query( nodes, IndexOrder.NONE, false, IndexQuery.exact( propertyKeyId, entry.value ) );
                         assertEquals( entry.value.toString(), asSet( entry.nodeId, entry.nodeId + offset ), PrimitiveLongCollections.toSet( nodes ) );
                     }
                 }

community/community-it/kernel-it/src/test/java/org/neo4j/kernel/impl/newapi/NodeValueIndexCursorInMemoryTest.java

@@ -44,4 +44,16 @@ protected boolean spatialRangeSupport()
     {
         return false;
     }
+
+    @Override
+    protected boolean indexProvidesStringValues()
+    {
+        return true;
+    }
+
+    @Override
+    protected boolean indexProvidesNumericValues()
+    {
+        return true;
+    }
 }

community/cypher/cypher/src/main/java/org/neo4j/cypher/internal/codegen/CompiledIndexUtils.java

@@ -65,7 +65,8 @@ public static NodeValueIndexCursor indexSeek( Read read, CursorFactory cursors,
         {
             NodeValueIndexCursor cursor = cursors.allocateNodeValueIndexCursor();
             IndexQuery.ExactPredicate query = exact( index.properties()[0], makeValueNeoSafe( value ) );
-            read.nodeIndexSeek( index, cursor, IndexOrder.NONE, query );
+            //TODO figure out how to deal with indexes that provide values
+            read.nodeIndexSeek( index, cursor, IndexOrder.NONE, false, query );
             return cursor;
         }
     }

community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v2_3/TransactionBoundQueryContext.scala

@@ -22,11 +22,7 @@ package org.neo4j.cypher.internal.spi.v2_3
 import java.net.URL
 import java.util.function.Predicate
 
-import org.eclipse.collections.api.collection.primitive.ImmutableLongCollection
 import org.eclipse.collections.api.iterator.LongIterator
-import org.eclipse.collections.impl.factory.primitive.LongSets
-import org.eclipse.collections.impl.iterator.ImmutableEmptyLongIterator
-import org.eclipse.collections.impl.utility.internal.primitive.LongIteratorIterate
 import org.neo4j.cypher.InternalException
 import org.neo4j.cypher.internal.compiler.v2_3.MinMaxOrdering.{BY_NUMBER, BY_STRING, BY_VALUE}
 import org.neo4j.cypher.internal.compiler.v2_3._
@@ -39,7 +35,7 @@ import org.neo4j.cypher.internal.frontend.v2_3.SemanticDirection.{BOTH, INCOMING
 import org.neo4j.cypher.internal.frontend.v2_3.{Bound, EntityNotFoundException, FailedIndexException, SemanticDirection}
 import org.neo4j.cypher.internal.javacompat.GraphDatabaseCypherService
 import org.neo4j.cypher.internal.runtime.interpreted._
-import org.neo4j.cypher.internal.spi.{CursorIterator, PrimitiveCursorIterator}
+import org.neo4j.cypher.internal.spi.CursorIterator
 import org.neo4j.graphalgo.impl.path.ShortestPath
 import org.neo4j.graphalgo.impl.path.ShortestPath.ShortestPathPredicate
 import org.neo4j.graphdb.RelationshipType._
@@ -231,7 +227,8 @@ final class TransactionBoundQueryContext(tc: TransactionalContextWrapper, val re
 
   private def seek(index: IndexReference, query: IndexQuery*) = {
     val nodeCursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexSeek(index, nodeCursor, IndexOrder.NONE, query:_*)
+    //older planners cannot use values provided by the index
+    reads().nodeIndexSeek(index, nodeCursor, IndexOrder.NONE, false, query:_*)
     new CursorIterator[Node] {
       override protected def fetchNext(): Node = {
         if (nodeCursor.next()) proxySpi.newNodeProxy(nodeCursor.nodeReference())
@@ -242,17 +239,6 @@ final class TransactionBoundQueryContext(tc: TransactionalContextWrapper, val re
     }
   }
 
-  private def scan(index: IndexReference) = {
-    val nodeCursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexScan(index, nodeCursor, IndexOrder.NONE)
-    new PrimitiveCursorIterator {
-      override protected def fetchNext(): Long =
-        if (nodeCursor.next()) nodeCursor.nodeReference() else -1L
-
-      override protected def close(): Unit = nodeCursor.close()
-    }
-  }
-
   private def indexSeekByPrefixRange(index: SchemaTypes.IndexDescriptor, range: InequalitySeekRange[Any]): scala.Iterator[Node] = {
     val groupedRanges = range.groupBy { (bound: Bound[Any]) =>
       bound.endPoint match {
@@ -357,9 +343,10 @@ final class TransactionBoundQueryContext(tc: TransactionalContextWrapper, val re
       }.getOrElse(Iterator.empty)
   }
 
-  def indexScan(index: SchemaTypes.IndexDescriptor) = {
+  def indexScan(index: SchemaTypes.IndexDescriptor): CursorIterator[Node] = {
     val cursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexScan(tc.schemaRead.indexReferenceUnchecked(index.labelId, index.propertyId), cursor, IndexOrder.NONE)
+    //older planners cannot use values provided by the index
+    reads().nodeIndexScan(tc.schemaRead.indexReferenceUnchecked(index.labelId, index.propertyId), cursor, IndexOrder.NONE, false)
     new CursorIterator[Node] {
       override protected def fetchNext(): Node = {
         if (cursor.next()) proxySpi.newNodeProxy(cursor.nodeReference())

community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/spi/v3_1/TransactionBoundQueryContext.scala

@@ -37,8 +37,8 @@ import org.neo4j.cypher.internal.frontend.v3_1.SemanticDirection.{BOTH, INCOMING
 import org.neo4j.cypher.internal.frontend.v3_1.{Bound, EntityNotFoundException, FailedIndexException, SemanticDirection}
 import org.neo4j.cypher.internal.javacompat.GraphDatabaseCypherService
 import org.neo4j.cypher.internal.runtime.interpreted.ResourceManager
+import org.neo4j.cypher.internal.spi.CursorIterator
 import org.neo4j.cypher.internal.spi.v3_1.TransactionBoundQueryContext.IndexSearchMonitor
-import org.neo4j.cypher.internal.spi.{CursorIterator, PrimitiveCursorIterator}
 import org.neo4j.graphalgo.impl.path.ShortestPath
 import org.neo4j.graphalgo.impl.path.ShortestPath.ShortestPathPredicate
 import org.neo4j.graphdb.RelationshipType._
@@ -224,7 +224,8 @@ final class TransactionBoundQueryContext(txContext: TransactionalContextWrapper,
 
   private def seek(index: IndexReference, query: IndexQuery*) = {
     val nodeCursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexSeek(index, nodeCursor, IndexOrder.NONE, query:_*)
+    //older planners cannot use values provided by the index
+    reads().nodeIndexSeek(index, nodeCursor, IndexOrder.NONE, false, query:_*)
     new CursorIterator[Node] {
       override protected def fetchNext(): Node = {
         if (nodeCursor.next()) entityAccessor.newNodeProxy(nodeCursor.nodeReference())
@@ -235,17 +236,6 @@ final class TransactionBoundQueryContext(txContext: TransactionalContextWrapper,
     }
   }
 
-  private def scan(index: IndexReference) = {
-    val nodeCursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexScan(index, nodeCursor, IndexOrder.NONE)
-    new PrimitiveCursorIterator {
-      override protected def fetchNext(): Long =
-        if (nodeCursor.next()) nodeCursor.nodeReference() else -1L
-
-      override protected def close(): Unit = nodeCursor.close()
-    }
-  }
-
   private def indexSeekByPrefixRange(index: IndexDescriptor, range: InequalitySeekRange[Any]): scala.Iterator[Node] = {
     val groupedRanges = range.groupBy { (bound: Bound[Any]) =>
       bound.endPoint match {
@@ -352,7 +342,8 @@ final class TransactionBoundQueryContext(txContext: TransactionalContextWrapper,
 
   override def indexScan(index: IndexDescriptor) = {
     val cursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexScan(txContext.kernelTransaction.schemaRead().indexReferenceUnchecked(index.labelId, index.propertyId), cursor, IndexOrder.NONE)
+    //older planners cannot use values provided by the index
+    reads().nodeIndexScan(txContext.kernelTransaction.schemaRead().indexReferenceUnchecked(index.labelId, index.propertyId), cursor, IndexOrder.NONE, false)
     new CursorIterator[Node] {
       override protected def fetchNext(): Node = {
         if (cursor.next()) entityAccessor.newNodeProxy(cursor.nodeReference())

community/cypher/cypher/src/test/java/org/neo4j/cypher/internal/codegen/CompiledIndexUtilsTest.java

@@ -51,7 +51,7 @@ public void shouldCallIndexSeek() throws KernelException
         CompiledIndexUtils.indexSeek( read, mock( CursorFactory.class ), index, "hello" );
 
         // THEN
-        verify( read, times( 1 ) ).nodeIndexSeek( any(), any(), any(), any() );
+        verify( read, times( 1 ) ).nodeIndexSeek( any(), any(), any(), any(), any() );
     }
 
     @Test
@@ -67,7 +67,7 @@ public void shouldHandleNullInIndexSeek() throws KernelException
                 index, null );
 
         // THEN
-        verify( read, never() ).nodeIndexSeek( any(), any(), any() );
+        verify( read, never() ).nodeIndexSeek( any(), any(), any(), any() );
         assertFalse( cursor.next() );
     }
 }

community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/TransactionBoundQueryContext.scala

@@ -37,6 +37,7 @@ import org.neo4j.graphdb._
 import org.neo4j.graphdb.security.URLAccessValidationError
 import org.neo4j.graphdb.traversal.{Evaluators, TraversalDescription, Uniqueness}
 import org.neo4j.internal.kernel.api
+import org.neo4j.internal.kernel.api.IndexQuery.ExactPredicate
 import org.neo4j.internal.kernel.api._
 import org.neo4j.internal.kernel.api.exceptions.ProcedureException
 import org.neo4j.internal.kernel.api.helpers.RelationshipSelections.{allCursor, incomingCursor, outgoingCursor}
@@ -274,20 +275,37 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
                               properties: Int*): IndexReference =
     transactionalContext.kernelTransaction.schemaRead().index(label, properties: _*)
 
-  private def seek(index: IndexReference, propertyIndicesWithValues: Seq[Int], query: IndexQuery*): CursorIterator[(NodeValue, Seq[Value])] = {
+  private def seek(index: IndexReference, propertyIndicesWithValues: Seq[Int], queries: IndexQuery*): CursorIterator[(NodeValue, Seq[Value])] = {
     val nodeCursor: NodeValueIndexCursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexSeek(index, nodeCursor, IndexOrder.NONE, query: _*)
+    val actualValues =
+      if (queries.forall(_.isInstanceOf[ExactPredicate])) {
+        // We don't need property values from the index for an exact seek
+        Some(propertyIndicesWithValues.map(queries(_).asInstanceOf[ExactPredicate].value()))
+      } else {
+        None
+      }
+
+
+    val needsValuesFromIndexSeek = actualValues.isEmpty && propertyIndicesWithValues.nonEmpty
+    reads().nodeIndexSeek(index, nodeCursor, IndexOrder.NONE, needsValuesFromIndexSeek, queries: _*)
     new CursorIterator[(NodeValue, Seq[Value])] {
       override protected def fetchNext(): (NodeValue, Seq[Value]) = {
         if (nodeCursor.next()) {
           val nodeRef = nodeCursor.nodeReference()
 
-          if (propertyIndicesWithValues.nonEmpty && !nodeCursor.hasValue) {
-            // We were promised at plan time that we can get values everywhere, so this should never happen
-            throw new IllegalStateException("NodeCursor did unexpectedly not have values during index seek.")
-          }
           // Get the actual property values for the requested indices
-          val values = propertyIndicesWithValues.map(nodeCursor.propertyValue)
+          val values = actualValues match {
+            case Some(v) =>
+              v
+            case None =>
+              if (propertyIndicesWithValues.nonEmpty && !nodeCursor.hasValue) {
+                // We were promised at plan time that we can get values everywhere, so this should never happen
+                throw new IllegalStateException("NodeCursor did unexpectedly not have values during index seek.")
+              }
+              propertyIndicesWithValues.map(nodeCursor.propertyValue)
+          }
+
+
           val node = fromNodeProxy(entityAccessor.newNodeProxy(nodeRef))
           (node, values)
         }
@@ -302,7 +320,7 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
 
   override def indexScan(index: IndexReference, propertyIndicesWithValues: Seq[Int]): Iterator[(NodeValue, Seq[Value])] = {
     val nodeCursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexScan(index, nodeCursor, IndexOrder.NONE)
+    reads().nodeIndexScan(index, nodeCursor, IndexOrder.NONE, propertyIndicesWithValues.nonEmpty)
     new CursorIterator[(NodeValue, Seq[Value])] {
       override protected def fetchNext(): (NodeValue, Seq[Value]) = {
         if (nodeCursor.next()) {
@@ -328,7 +346,8 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
 
   override def indexScanPrimitive(index: IndexReference): PrimitiveLongResourceIterator = {
     val nodeCursor = allocateAndTraceNodeValueIndexCursor()
-    reads().nodeIndexScan(index, nodeCursor, IndexOrder.NONE)
+    // for a primitive cursor, we don't need values
+    reads().nodeIndexScan(index, nodeCursor, IndexOrder.NONE, false)
     new PrimitiveCursorIterator {
       override protected def fetchNext(): Long =
         if (nodeCursor.next()) nodeCursor.nodeReference() else -1L
@@ -339,7 +358,7 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
 
     override def indexScanPrimitiveWithValues(index: IndexReference, propertyIndicesWithValues: Seq[Int]): Iterator[(Long, Seq[Value])] = {
       val nodeCursor = allocateAndTraceNodeValueIndexCursor()
-      reads().nodeIndexScan(index, nodeCursor, IndexOrder.NONE)
+      reads().nodeIndexScan(index, nodeCursor, IndexOrder.NONE, propertyIndicesWithValues.nonEmpty)
       new CursorIterator[(Long, Seq[Value])] {
         override protected def fetchNext(): (Long, Seq[Value]) =
           if (nodeCursor.next()) {

community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/IndexOrder.java

@@ -23,8 +23,8 @@
  * Enum used for two purposes:
  * 1. As return value for {@link IndexCapability#orderCapability(org.neo4j.values.storable.ValueCategory...)}.
  * Only {@link #ASCENDING} and {@link #DESCENDING} is valid for this.
- * 2. As parameter for {@link Read#nodeIndexScan(IndexReference, NodeValueIndexCursor, IndexOrder)} and
- * {@link Read#nodeIndexSeek(IndexReference, NodeValueIndexCursor, IndexOrder, IndexQuery...)}. Where {@link #NONE} is used when
+ * 2. As parameter for {@link Read#nodeIndexScan(IndexReference, NodeValueIndexCursor, IndexOrder, boolean)} and
+ * {@link Read#nodeIndexSeek(IndexReference, NodeValueIndexCursor, IndexOrder, boolean, IndexQuery...)}. Where {@link #NONE} is used when
  * no ordering is available or required.
  */
 public enum IndexOrder