Skip to content

Commit

Permalink
Enrich range-scan acceptance tests and fix minor discovered issues
Browse files Browse the repository at this point in the history
Also:
 - rebase fixes
 - migrate IndexingTestSupport to CypherComparisonSupport
  • Loading branch information
fickludd authored and Lojjs committed Mar 16, 2018
1 parent 848878b commit 9be64e3
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 20 deletions.
Expand Up @@ -239,9 +239,18 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona
case e: NotFoundException => throw new EntityNotFoundException(s"Relationship with id $relationshipId", e)
}

val SEEKABLE_VALUE_GROUPS = Array(ValueGroup.NUMBER,
ValueGroup.TEXT,
ValueGroup.GEOMETRY,
ValueGroup.DATE,
ValueGroup.LOCAL_DATE_TIME,
ValueGroup.ZONED_DATE_TIME,
ValueGroup.LOCAL_TIME,
ValueGroup.ZONED_TIME,
ValueGroup.DURATION)

override def indexSeek(index: IndexReference, predicates: Seq[IndexQuery]): Iterator[NodeValue] = {

val SEEKABLE_VALUE_GROUPS = Array(ValueGroup.NUMBER, ValueGroup.TEXT, ValueGroup.GEOMETRY, ValueGroup.DATE)
val impossiblePredicate =
predicates.exists {
case p:IndexQuery.ExactPredicate => p.value() == Values.NO_VALUE
Expand Down
Expand Up @@ -142,8 +142,6 @@ trait NodeIndexSeeker {

private def computeExactQueries(state: QueryState, row: ExecutionContext): Seq[Seq[IndexQuery.ExactPredicate]] =
valueExpr match {
case ScanQueryExpression(expr) => throw new IllegalArgumentException("this won't happen!")

// Index exact value seek on single value
case SingleQueryExpression(expr) =>
val seekValue = makeValueNeoSafe(expr(row, state))
Expand Down
Expand Up @@ -114,7 +114,6 @@ public PrimitiveLongResourceIterator query( IndexQuery... predicates ) throws In
return instances[TEMPORAL].query( predicates );
default: // fall through
}
// TODO: support temporal range queries
}

// todo: There will be no ordering of the node ids here. Is this a problem?
Expand Down Expand Up @@ -172,11 +171,10 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder
case LOCAL_TIME:
case ZONED_TIME:
case DURATION:
temporalReader.query( cursor, indexOrder, predicates );
instances[TEMPORAL].query( cursor, indexOrder, predicates );
return;
default: // fall through
}
// TODO: support temporal range queries
}

// todo: There will be no ordering of the node ids here. Is this a problem?
Expand Down
Expand Up @@ -21,8 +21,8 @@

import org.junit.Test;

import org.neo4j.kernel.api.schema.index.IndexDescriptor;
import org.neo4j.kernel.api.schema.index.IndexDescriptorFactory;
import org.neo4j.kernel.api.schema.index.SchemaIndexDescriptor;
import org.neo4j.kernel.api.schema.index.SchemaIndexDescriptorFactory;
import org.neo4j.storageengine.api.schema.IndexProgressor;

import static org.mockito.Mockito.mock;
Expand All @@ -34,7 +34,7 @@ public class BridgingIndexProgressorTest
@Test
public void closeMustCloseAll()
{
IndexDescriptor index = IndexDescriptorFactory.forLabel( 1, 2, 3 );
SchemaIndexDescriptor index = SchemaIndexDescriptorFactory.forLabel( 1, 2, 3 );
BridgingIndexProgressor progressor = new BridgingIndexProgressor( null, index.schema().getPropertyIds() );

IndexProgressor[] parts = {mock(IndexProgressor.class), mock(IndexProgressor.class)};
Expand Down
Expand Up @@ -30,6 +30,8 @@ class IndexPersistenceAcceptanceTest extends IndexingTestSupport {

private var dbDir = new File("test")

override val cypherComparisonSupport = false

override protected def initTest() {
FileUtils.deleteRecursively(dbDir)
startGraphDatabase(dbDir)
Expand Down
Expand Up @@ -23,6 +23,7 @@ import java.util.stream.Collectors

import org.neo4j.cypher.ExecutionEngineFunSuite
import org.neo4j.graphdb.Node
import org.neo4j.internal.cypher.acceptance.CypherComparisonSupport._
import org.neo4j.values.storable.Value

import scala.collection.JavaConversions._
Expand All @@ -32,6 +33,8 @@ trait IndexingTestSupport extends ExecutionEngineFunSuite with CypherComparisonS
protected val LABEL: String = "Label"
protected val PROPERTY: String = "prop"

def cypherComparisonSupport: Boolean

protected def createIndex(): Unit = {
graph.createIndex(LABEL, PROPERTY)
}
Expand All @@ -51,31 +54,50 @@ trait IndexingTestSupport extends ExecutionEngineFunSuite with CypherComparisonS
}

protected def assertSeekMatchFor(value: Value, nodes: Node*): Unit = {
val query = "CYPHER runtime=slotted MATCH (n:%s) WHERE n.%s = $%s RETURN n".format(LABEL, PROPERTY, PROPERTY)
val query = "MATCH (n:%s) WHERE n.%s = $%s RETURN n".format(LABEL, PROPERTY, PROPERTY)
testIndexedRead(query, Map(PROPERTY -> value.asObject()), "NodeIndexSeek", nodes)
}

protected def assertScanMatch(nodes: Node*): Unit = {
val query = "CYPHER runtime=slotted MATCH (n:%s) WHERE EXISTS(n.%s) RETURN n".format(LABEL, PROPERTY)
val query = "MATCH (n:%s) WHERE EXISTS(n.%s) RETURN n".format(LABEL, PROPERTY)
testIndexedRead(query, Map(), "NodeIndexScan", nodes)
}

protected def assertRangeScanFor(op1: String, bound1: Value, op2: String, bound2: Value, nodes: Node*): Unit = {
val predicate1 = s"n.$PROPERTY $op1 $$param1"
val predicate2 = s"n.$PROPERTY $op2 $$param2"
val query = s"CYPHER runtime=slotted MATCH (n:$LABEL) WHERE $predicate1 AND $predicate2 RETURN n"
val query = s"MATCH (n:$LABEL) WHERE $predicate1 AND $predicate2 RETURN n"
testIndexedRead(query, Map("param1" -> bound1.asObject(), "param2" -> bound2.asObject()), "NodeIndexSeekByRange", nodes)
}

private def testIndexedRead(query: String, params: Map[String, AnyRef], wantedOperator: String, expected: Seq[Node]): Unit = {
val result = graph.execute(query, params)
if (cypherComparisonSupport) {
val result =
executeWith(
TestConfiguration(
Versions(Versions.V3_4, Versions.V3_3, Versions.Default),
Planners(Planners.Cost, Planners.Default),
Runtimes(Runtimes.Interpreted, Runtimes.Slotted, Runtimes.Default)
),
query,
params = params,
planComparisonStrategy = ComparePlansWithAssertion(_ should useOperators(wantedOperator),
expectPlansToFail = Configs.AllRulePlanners)
)
val nodes = result.columnAs("n").toSet
expected.foreach(p => assert(nodes.contains(p)))
nodes.size() should be(expected.size)
} else {
val result = graph.execute("CYPHER runtime=slotted "+query, params)
val nodes = result.columnAs("n").stream().collect(Collectors.toSet)
expected.foreach(p => assert(nodes.contains(p)))
nodes.size() should be(expected.size)

val plan = result.getExecutionPlanDescription.toString
plan should include(wantedOperator)
plan should include (s":$LABEL($PROPERTY)")
}

val nodes = result.columnAs("n").stream().collect(Collectors.toSet)
expected.foreach(p => assert(nodes.contains(p)))
nodes.size() should be(expected.size)

val plan = result.getExecutionPlanDescription.toString
plan should include(wantedOperator)
plan should include (s":$LABEL($PROPERTY)")
}
}
Expand Up @@ -19,12 +19,14 @@
*/
package org.neo4j.internal.cypher.acceptance

import java.time.ZoneOffset
import java.time.{ZoneId, ZoneOffset}

import org.neo4j.values.storable._

class TemporalIndexAcceptanceTest extends IndexingTestSupport {

override val cypherComparisonSupport = true

test("should seek") {
createIndex()
assertSeek(DateValue.epochDate(10000))
Expand All @@ -42,6 +44,30 @@ class TemporalIndexAcceptanceTest extends IndexingTestSupport {
DateValue.epochDate(10002),
DateValue.epochDate(10004),
DateValue.epochDate(10006))

assertRangeScan(
LocalDateTimeValue.localDateTime(10000, 100000000),
LocalDateTimeValue.localDateTime(10002, 100000000),
LocalDateTimeValue.localDateTime(10004, 100000000),
LocalDateTimeValue.localDateTime(10006, 100000000))

assertRangeScan(
DateTimeValue.datetime(10000, 100000000, ZoneId.of("Europe/Stockholm")),
DateTimeValue.datetime(10002, 100000000, ZoneId.of("Europe/Stockholm")),
DateTimeValue.datetime(10004, 100000000, ZoneId.of("Europe/Stockholm")),
DateTimeValue.datetime(10006, 100000000, ZoneId.of("Europe/Stockholm")))

assertRangeScan(
LocalTimeValue.localTime(10000),
LocalTimeValue.localTime(10002),
LocalTimeValue.localTime(10004),
LocalTimeValue.localTime(10006))

assertRangeScan(
TimeValue.time(10000, ZoneId.of("Europe/Stockholm")),
TimeValue.time(10002, ZoneId.of("Europe/Stockholm")),
TimeValue.time(10004, ZoneId.of("Europe/Stockholm")),
TimeValue.time(10006, ZoneId.of("Europe/Stockholm")))
}

def assertSeek(value: Value): Unit = {
Expand All @@ -56,5 +82,6 @@ class TemporalIndexAcceptanceTest extends IndexingTestSupport {
val n4 = createIndexedNode(v4)

assertRangeScanFor(">", v1, "<", v4, n2, n3)
assertRangeScanFor(">=", v1, "<=", v4, n1, n2, n3, n4)
}
}

0 comments on commit 9be64e3

Please sign in to comment.