Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the performance of IN checking #7410

Merged
merged 2 commits into from
Jun 28, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.neo4j.internal.cypher.acceptance

import org.neo4j.cypher.{SyntaxException, NewPlannerTestSupport, ExecutionEngineFunSuite}
import org.neo4j.graphdb.Node

class AggregationAcceptanceTest extends ExecutionEngineFunSuite with NewPlannerTestSupport {

Expand Down Expand Up @@ -102,7 +101,8 @@ class AggregationAcceptanceTest extends ExecutionEngineFunSuite with NewPlannerT
"""match p = (a:Start)-[*]-> (b)
|return b, avg(length(p))""".stripMargin)

result.columnAs[Node]("b").toSet should equal (Set(b, c))
result.toSet should equal(Set(Map("b" -> c, "avg(length(p))" -> 1.0),
Map("b" -> b, "avg(length(p))" -> 1.0)))
}

test("should be able to do distinct on unbound node") {
Expand Down Expand Up @@ -230,12 +230,13 @@ class AggregationAcceptanceTest extends ExecutionEngineFunSuite with NewPlannerT
// when
val query =
"""MATCH p=(a:T {name: "a"})-[:R*]-(other: T)
|WHERE other <> a WITH a, other, min(length(p)) AS len
|RETURN a.name as name, collect(other.name) AS others, len;""".stripMargin
|WHERE other <> a
|WITH a, other, min(length(p)) AS len ORDER BY other.name
|RETURN a.name as name, collect(other.name) AS others, len""".stripMargin
val result = executeWithAllPlannersAndCompatibilityMode(query)

//then
result.toList should equal(Seq(Map("name" -> "a", "others" -> Seq("c", "b"), "len" -> 1 )))
result.toList should equal(Seq(Map("name" -> "a", "others" -> Seq("b", "c"), "len" -> 1 )))
}

test("should handle subexpression in aggregation also occuring as standalone expression with nested aggregation in a literal map") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1335,13 +1335,13 @@ return b
actual should equal(expected)
}

test("MATCH (a)-[r]->(b) WITH a, r, b, count(*) AS c ORDER BY c MATCH (a)-[r]->(b) RETURN r AS rel") {
test("MATCH (a)-[r]->(b) WITH a, r, b, count(*) AS c ORDER BY c MATCH (a)-[r]->(b) RETURN r AS rel ORDER BY id(rel)") {
// given two disconnected rels
val rel1 = relate(createNode(), createNode())
val rel2 = relate(createNode(), createNode())

// when
val result = executeWithAllPlannersAndCompatibilityMode("MATCH (a)-[r]->(b) WITH a, r, b, count(*) AS c ORDER BY c MATCH (a)-[r]->(b) RETURN r AS rel")
val result = executeWithAllPlannersAndCompatibilityMode("MATCH (a)-[r]->(b) WITH a, r, b, count(*) AS c ORDER BY c MATCH (a)-[r]->(b) RETURN r AS rel ORDER BY id(rel)")

// should give us all rels
val actual = relsById(result.columnAs[Relationship]("rel").toList)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,13 @@ class PatternExpressionAcceptanceTest extends ExecutionEngineFunSuite with Match
relate(start, createNode())
relate(start, createNode())

val result = executeWithAllPlannersAndCompatibilityMode("match (n) with case when id(n) >= 0 then (n)-->() else 42 end as p, count(n) as c return p, c")
val result = executeWithAllPlannersAndCompatibilityMode(
"""match (n)
|with case
| when id(n) >= 0 then (n)-->()
| else 42
| end as p, count(n) as c
|return p, c order by c""".stripMargin)
.toList.head("p").asInstanceOf[Seq[_]]

result should have size 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,6 @@ class UniqueIndexAcceptanceTest extends ExecutionEngineFunSuite with NewPlannerT
result.toList should equal (List(Map("n" -> jake)))
}

test("should be able to use unique index on IN an empty collections") {
//GIVEN
val andres = createLabeledNode(Map("name" -> "Andres"), "Person")
val jake = createLabeledNode(Map("name" -> "Jacob"), "Person")
relate(andres, createNode())
relate(jake, createNode())

graph.createConstraint("Person", "name")

//WHEN
val result = executeWithAllPlannersAndCompatibilityMode("MATCH (n:Person)-->() USING INDEX n:Person(name) WHERE n.name IN [] RETURN n")

//THEN
result.toList should equal (List())
}

test("should be able to use unique index on IN a null value") {
//GIVEN
val andres = createLabeledNode(Map("name" -> "Andres"), "Person")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ import org.neo4j.cypher.internal.compiler.v3_0.IDPPlannerName
import org.neo4j.cypher.internal.compiler.v3_0.planDescription.InternalPlanDescription
import org.neo4j.cypher.internal.compiler.v3_0.planDescription.InternalPlanDescription.Arguments.KeyNames
import org.neo4j.cypher.internal.compiler.v3_0.planner.logical.plans.{NodeHashJoin, NodeIndexSeek}
import org.neo4j.cypher.{ExecutionEngineFunSuite, HintException, IndexHintException, NewPlannerTestSupport, SyntaxException, _}
import org.neo4j.graphdb.schema.Schema
import org.neo4j.graphdb.{QueryExecutionException, Result}
import org.neo4j.cypher.{ExecutionEngineFunSuite, IndexHintException, NewPlannerTestSupport, SyntaxException, _}
import org.neo4j.graphdb.QueryExecutionException
import org.neo4j.graphdb.factory.GraphDatabaseSettings
import org.neo4j.kernel.api.exceptions.Status
import org.scalatest.matchers.{MatchResult, Matcher}
Expand Down Expand Up @@ -200,22 +199,6 @@ class UsingAcceptanceTest extends ExecutionEngineFunSuite with NewPlannerTestSup
result.toList should equal(List(Map("n" -> jake)))
}

test("should be able to use index hints on IN an empty collections") {
//GIVEN
val andres = createLabeledNode(Map("name" -> "Andres"), "Person")
val jake = createLabeledNode(Map("name" -> "Jacob"), "Person")
relate(andres, createNode())
relate(jake, createNode())

graph.createIndex("Person", "name")

//WHEN
val result = executeWithAllPlannersAndCompatibilityMode("MATCH (n:Person)-->() USING INDEX n:Person(name) WHERE n.name IN [] RETURN n")

//THEN
result.toList should equal(List())
}

test("should be able to use index hints on IN a null value") {
//GIVEN
val andres = createLabeledNode(Map("name" -> "Andres"), "Person")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,3 @@ case class ExecutionContext(m: MutableMap[String, Any] = MutableMaps.empty)
copy(m = newMap)
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ object CRS {
def fromSRID(id: Int) = id match {
case Cartesian.`code` => Cartesian
case WGS84.`code` => WGS84
case _ => throw new InvalidArgumentException(s"SRID '$id' does not match any supported coordinate reference system for points, supported CRS are: '${WGS84.name}', '${Cartesian.name}'")
case _ => throw new InvalidArgumentException(s"SRID '$id' does not match any supported coordinate reference system for points, supported CRS are: '${WGS84.code}', '${Cartesian.code}'")
}

def fromURL(url: String) = url match {
case Cartesian.url => Cartesian
case WGS84.url => WGS84
case _ => throw new InvalidArgumentException(s"HREF '$url' does not match any supported coordinate reference system for points, supported CRS are: '${WGS84.url}', '${Cartesian.url}'")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -325,17 +325,25 @@ object ExpressionConverters {
commandexpressions.GetDegree(toCommandExpression(original.node), typ, original.dir)
}


private def regexMatch(e: ast.RegexMatch) = toCommandExpression(e.rhs) match {
case literal: commandexpressions.Literal =>
predicates.LiteralRegularExpression(toCommandExpression(e.lhs), literal)
case command =>
predicates.RegularExpression(toCommandExpression(e.lhs), command)
}

private def in(e: ast.In) = {
val innerEquals = predicates.Equals(toCommandExpression(e.lhs), commandexpressions.Variable("-_-INNER-_-"))
commands.AnyInCollection(toCommandExpression(e.rhs), "-_-INNER-_-", innerEquals)
private def in(e: ast.In) = e.rhs match {
case value: Parameter =>
predicates.ConstantCachedIn(toCommandExpression(e.lhs), toCommandExpression(value))

case value@Collection(expressions) if expressions.isEmpty =>
predicates.Not(predicates.True())

case value@Collection(expressions) if expressions.forall(_.isInstanceOf[Literal]) =>
predicates.ConstantCachedIn(toCommandExpression(e.lhs), toCommandExpression(value))

case _ =>
predicates.DynamicCachedIn(toCommandExpression(e.lhs), toCommandExpression(e.rhs))
}

private def caseExpression(e: ast.CaseExpression) = e.expression match {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* Copyright (c) 2002-2016 "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.cypher.internal.compiler.v3_0.commands.predicates

import scala.collection.mutable

/**
* This is a class that handles IN checking. With a cache. It's a state machine, and
* each checking using the contains() method returns both the result of the IN check and the new state.
*/
trait Checker {
def contains(value: Any): (Option[Boolean], Checker)
}

class BuildUp(iterator: Iterator[Any]) extends Checker {
private val cachedSet: mutable.Set[Equivalent] = new mutable.HashSet[Equivalent]
private var falseResult: Option[Boolean] = Some(false)

assert(iterator.nonEmpty)

override def contains(value: Any): (Option[Boolean], Checker) = {
if (value == null)
return (None, this)

val eqValue = Equivalent(value)
if (cachedSet.contains(eqValue))
(Some(true), this)
else
checkAndBuildUpCache(value)
}

private def checkAndBuildUpCache(value: Any): (Option[Boolean], Checker) = {
while (iterator.nonEmpty) {
val nextValue = iterator.next()

if (nextValue == null) {
falseResult = None
} else {
val next = Equivalent(nextValue)
cachedSet.add(next)
val result = next == value
if (result) return (Some(true), this)
}
}

(falseResult, new SetChecker(cachedSet, falseResult))
}
}

case object AlwaysFalseChecker extends Checker {
override def contains(value: Any): (Option[Boolean], Checker) = (Some(false), this)
}

case object NullListChecker extends Checker {
override def contains(value: Any): (Option[Boolean], Checker) = (None, this)
}

// This is the final form for this cache.
class SetChecker(cachedSet: mutable.Set[Equivalent], falseResult: Option[Boolean]) extends Checker {

assert(cachedSet.nonEmpty)

override def contains(value: Any): (Option[Boolean], Checker) = {
if (value == null)
return (None, this)

val eqValue = Equivalent(value)

val exists = cachedSet.contains(eqValue)
val result = if (exists) Some(true) else falseResult
(result, this)
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ package org.neo4j.cypher.internal.compiler.v3_0.commands.predicates

import org.neo4j.cypher.internal.compiler.v3_0._
import org.neo4j.cypher.internal.compiler.v3_0.commands.expressions.{Expression, Variable, Literal}
import org.neo4j.cypher.internal.compiler.v3_0.helpers.IsCollection
import org.neo4j.cypher.internal.compiler.v3_0.pipes.QueryState
import org.neo4j.cypher.internal.frontend.v3_0.IncomparableValuesException
import org.neo4j.graphdb.{Node, Relationship}

abstract sealed class ComparablePredicate(val left: Expression, val right: Expression) extends Predicate with Comparer {
def compare(comparisonResult: Int): Boolean
Expand Down Expand Up @@ -69,22 +66,12 @@ case class Equals(a: Expression, b: Expression) extends Predicate with Comparer
val b1 = b(m)

(a1, b1) match {
case (null, _) => None
case (_, null) => None
case (IsCollection(l), IsCollection(r)) => Some(l == r)
case (l: Node, r) if !r.isInstanceOf[Node] => incomparable(l, r)
case (l, r: Node) if !l.isInstanceOf[Node] => incomparable(l, r)
case (l: Relationship, r) if !r.isInstanceOf[Relationship] => incomparable(l, r)
case (l, r: Relationship) if !l.isInstanceOf[Relationship] => incomparable(l, r)
case (l: String, r: Character) => Some(l == r.toString)
case (l: Character, r: String) => Some(l.toString == r)
case _ => Some(a1 == b1)
case (null, _) => None
case (_, null) => None
case _ => Some(Equivalent(a1) == b1)
}
}

private def incomparable(lhs: Any, rhs: Any)(implicit state: QueryState): Nothing =
throw new IncomparableValuesException(serializeWithType(lhs), serializeWithType(rhs))

override def toString = s"$a == $b"

def containsIsNull = (a, b) match {
Expand Down