Skip to content

Commit

Permalink
Fixing tests and addressing review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
SaschaPeukert committed Aug 22, 2019
1 parent 878e2aa commit cad62d9
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 22 deletions.
Expand Up @@ -19,19 +19,12 @@
*/
package org.neo4j.cypher.internal.compiler.v3_5.ast.convert.plannerQuery

import org.neo4j.cypher.internal.compiler.v3_5.planner.LogicalPlanningTestSupport
import org.neo4j.cypher.internal.compiler.v3_5.planner._
import org.neo4j.cypher.internal.compiler.v3_5.planner.{LogicalPlanningTestSupport, _}
import org.neo4j.cypher.internal.ir.v3_5._
import org.neo4j.cypher.internal.v3_5.logical.plans.FieldSignature
import org.neo4j.cypher.internal.v3_5.logical.plans.ProcedureReadOnlyAccess
import org.neo4j.cypher.internal.v3_5.logical.plans.ProcedureSignature
import org.neo4j.cypher.internal.v3_5.logical.plans.QualifiedName
import org.neo4j.cypher.internal.v3_5.ast.Hint
import org.neo4j.cypher.internal.v3_5.ast.UsingIndexHint
import org.neo4j.cypher.internal.v3_5.expressions.SemanticDirection.BOTH
import org.neo4j.cypher.internal.v3_5.expressions.SemanticDirection.INCOMING
import org.neo4j.cypher.internal.v3_5.expressions.SemanticDirection.OUTGOING
import org.neo4j.cypher.internal.v3_5.ast.{Hint, UsingIndexHint}
import org.neo4j.cypher.internal.v3_5.expressions.SemanticDirection.{BOTH, INCOMING, OUTGOING}
import org.neo4j.cypher.internal.v3_5.expressions._
import org.neo4j.cypher.internal.v3_5.logical.plans.{FieldSignature, ProcedureReadOnlyAccess, ProcedureSignature, QualifiedName}
import org.neo4j.cypher.internal.v3_5.util.helpers.StringHelper._
import org.neo4j.cypher.internal.v3_5.util.symbols._
import org.neo4j.cypher.internal.v3_5.util.test_helpers.CypherFunSuite
Expand Down Expand Up @@ -836,7 +829,7 @@ class StatementConvertersTest extends CypherFunSuite with LogicalPlanningTestSup
val result = query.toString

val expectation =
"""RegularPlannerQuery(QueryGraph {Nodes: [' candidate@60', ' origin@7', 'c'], Rels: ['( origin@7)--[r1:KNOWS:WORKS_AT]--(c)', '(c)--[r2:KNOWS:WORKS_AT]--( candidate@60)'], Predicates: ['not r1 = r2', 'not EXISTS((` origin@7`)-[` REL143`:KNOWS]-(` candidate@60`))', 'type(r1) = type(r2)', '` origin@7`.name IN ["Clark Kent"]']},InterestingOrder(List(Desc(boost)),List()),AggregatingQueryProjection(Map(origin -> Property(Variable( origin@7),PropertyKeyName(name)), candidate -> Property(Variable( candidate@60),PropertyKeyName(name))),Map(boost -> FunctionInvocation(Namespace(List()),FunctionName(SUM),false,Vector(FunctionInvocation(Namespace(List()),FunctionName(ROUND),false,Vector(Add(Property(Variable(r2),PropertyKeyName(weight)),Multiply(FunctionInvocation(Namespace(List()),FunctionName(COALESCE),false,Vector(Property(Variable(r2),PropertyKeyName(activity)), SignedDecimalIntegerLiteral(0))),SignedDecimalIntegerLiteral(2)))))))),QueryShuffle(List(DescSortItem(Variable(boost))),None,Some(SignedDecimalIntegerLiteral(10))),Selections(Set())),None)"""
"""RegularPlannerQuery(QueryGraph {Nodes: [' candidate@60', ' origin@7', 'c'], Rels: ['( origin@7)--[r1:KNOWS:WORKS_AT]--(c)', '(c)--[r2:KNOWS:WORKS_AT]--( candidate@60)'], Predicates: ['not r1 = r2', 'not EXISTS((` origin@7`)-[` REL143`:KNOWS]-(` candidate@60`))', 'type(r1) = type(r2)', '` origin@7`.name IN ["Clark Kent"]']},InterestingOrder(List(Desc(boost)),List()),AggregatingQueryProjection(Map(origin -> Property(Variable( origin@7),PropertyKeyName(name)), candidate -> Property(Variable( candidate@60),PropertyKeyName(name))),Map(boost -> FunctionInvocation(Namespace(List()),FunctionName(SUM),false,Vector(FunctionInvocation(Namespace(List()),FunctionName(ROUND),false,Vector(Add(Property(Variable(r2),PropertyKeyName(weight)),Multiply(FunctionInvocation(Namespace(List()),FunctionName(COALESCE),false,Vector(Property(Variable(r2),PropertyKeyName(activity)), SignedDecimalIntegerLiteral(0)),false),SignedDecimalIntegerLiteral(2)))),false)),false)),QueryShuffle(List(DescSortItem(Variable(boost))),None,Some(SignedDecimalIntegerLiteral(10))),Selections(Set())),None)"""

result should equal(expectation)
}
Expand All @@ -852,7 +845,7 @@ class StatementConvertersTest extends CypherFunSuite with LogicalPlanningTestSup
val result = query.toString

val expectation =
"""RegularPlannerQuery(QueryGraph {Nodes: ['owner']},InterestingOrder(List(),List()),AggregatingQueryProjection(Map(owner -> Variable(owner)),Map(xyz -> CountStar()),QueryShuffle(List(),None,None),Selections(Set())),Some(RegularPlannerQuery(QueryGraph {Arguments: ['owner', 'xyz']},InterestingOrder(List(),List()),RegularQueryProjection(Map(owner -> Variable(owner), collection -> GreaterThan(Variable(xyz),SignedDecimalIntegerLiteral(0))),QueryShuffle(List(),None,None),Selections(Set(Predicate(Set(owner, REL90, NODE92),FunctionInvocation(Namespace(List()),FunctionName(EXISTS),false,Vector(PatternExpression(RelationshipsPattern(RelationshipChain(NodePattern(Some(Variable(owner)),List(),None,None),RelationshipPattern(Some(Variable( REL90)),List(),None,None,BOTH,false,None),NodePattern(Some(Variable( NODE92)),List(),None,None)))))))))),Some(RegularPlannerQuery(QueryGraph {Arguments: ['collection', 'owner']},InterestingOrder(List(),List()),RegularQueryProjection(Map(owner -> Variable(owner)),QueryShuffle(List(),None,None),Selections(Set())),None)))))"""
"""RegularPlannerQuery(QueryGraph {Nodes: ['owner']},InterestingOrder(List(),List()),AggregatingQueryProjection(Map(owner -> Variable(owner)),Map(xyz -> CountStar()),QueryShuffle(List(),None,None),Selections(Set())),Some(RegularPlannerQuery(QueryGraph {Arguments: ['owner', 'xyz']},InterestingOrder(List(),List()),RegularQueryProjection(Map(owner -> Variable(owner), collection -> GreaterThan(Variable(xyz),SignedDecimalIntegerLiteral(0))),QueryShuffle(List(),None,None),Selections(Set(Predicate(Set(owner, REL90, NODE92),FunctionInvocation(Namespace(List()),FunctionName(EXISTS),false,Vector(PatternExpression(RelationshipsPattern(RelationshipChain(NodePattern(Some(Variable(owner)),List(),None,None),RelationshipPattern(Some(Variable( REL90)),List(),None,None,BOTH,false,None),NodePattern(Some(Variable( NODE92)),List(),None,None))))),false))))),Some(RegularPlannerQuery(QueryGraph {Arguments: ['collection', 'owner']},InterestingOrder(List(),List()),RegularQueryProjection(Map(owner -> Variable(owner)),QueryShuffle(List(),None,None),Selections(Set())),None)))))"""
result should equal(expectation)
}

Expand Down
Expand Up @@ -76,4 +76,7 @@ trait AstConstructionTestSupport extends CypherTestSupport {

def function(name: String, args: Expression*): FunctionInvocation = FunctionInvocation(FunctionName(name)(pos),
distinct = false, args.toIndexedSeq)(pos)

def function(name: String, deprecated: Boolean, args: Expression*): FunctionInvocation = FunctionInvocation(FunctionName(name)(pos),
distinct = false, args.toIndexedSeq, deprecated)(pos)
}
Expand Up @@ -28,6 +28,8 @@ object FunctionInvocation {
FunctionInvocation(Namespace()(name.position), name, distinct = false, IndexedSeq(expression))(name.position)
def apply(functionName: FunctionName, distinct: Boolean, args: IndexedSeq[Expression])(position: InputPosition): FunctionInvocation =
FunctionInvocation(Namespace()(position), functionName, distinct, args)(position)
def apply(functionName: FunctionName, distinct: Boolean, args: IndexedSeq[Expression], deprecated: Boolean)(position: InputPosition): FunctionInvocation =
FunctionInvocation(Namespace()(position), functionName, distinct, args, deprecated)(position)
}

case class FunctionInvocation(namespace: Namespace, functionName: FunctionName, distinct: Boolean, args: IndexedSeq[Expression],
Expand Down
Expand Up @@ -33,17 +33,17 @@ class ReplaceAliasedFunctionInvocationsTest extends CypherFunSuite with AstConst

test("should rewrite deprecated names regardless of casing") {
for ((oldName, newName) <- deprecatedNameMap ) {
rewriter(function(oldName, varFor("arg"))) should equal(function(oldName, varFor("arg")).copy(functionName = FunctionName(newName)(pos))(pos))
rewriter(function(oldName.toLowerCase(), varFor("arg"))) should equal(function(newName, varFor("arg")))
rewriter(function(oldName.toUpperCase(), varFor("arg"))) should equal(function(newName, varFor("arg")))
rewriter(function(oldName, varFor("arg"))) should equal(function(oldName, deprecated = true, varFor("arg")).copy(functionName = FunctionName(newName)(pos))(pos))
rewriter(function(oldName.toLowerCase(), varFor("arg"))) should equal(function(newName, deprecated = true, varFor("arg")))
rewriter(function(oldName.toUpperCase(), varFor("arg"))) should equal(function(newName, deprecated = true, varFor("arg")))
}
}

test("should not touch new names of regardless of casing") {
for (newName <- deprecatedNameMap.values ) {
rewriter(function(newName, varFor("arg"))) should equal(function(newName, varFor("arg")))
rewriter(function(newName.toLowerCase(), varFor("arg"))) should equal(function(newName, varFor("arg")))
rewriter(function(newName.toUpperCase(), varFor("arg"))) should equal(function(newName, varFor("arg")))
rewriter(function(newName.toLowerCase(), varFor("arg"))) should equal(function(newName, deprecated = false, varFor("arg")))
rewriter(function(newName.toUpperCase(), varFor("arg"))) should equal(function(newName, deprecated = false, varFor("arg")))
}
}

Expand All @@ -52,7 +52,7 @@ class ReplaceAliasedFunctionInvocationsTest extends CypherFunSuite with AstConst

val after =
Property(
FunctionInvocation(Namespace()(pos), FunctionName("datetime")(pos), distinct = false, IndexedSeq.empty)(pos),
FunctionInvocation(Namespace()(pos), FunctionName("datetime")(pos), distinct = false, IndexedSeq.empty, deprecated = true)(pos),
PropertyKeyName("epochMillis")(pos))(pos)
rewriter(before) should equal(after)
}
Expand All @@ -62,15 +62,15 @@ class ReplaceAliasedFunctionInvocationsTest extends CypherFunSuite with AstConst

val after =
Property(
FunctionInvocation(Namespace()(pos), FunctionName("datetime")(pos), distinct = false, IndexedSeq.empty)(pos),
FunctionInvocation(Namespace()(pos), FunctionName("datetime")(pos), distinct = false, IndexedSeq.empty, deprecated = true)(pos),
PropertyKeyName("epochMillis")(pos))(pos)
rewriter(before) should equal(after)
}

test("should rewrite extract() in V2") {
val scope = ExtractScope(varFor("a"), None, None)(pos)
val before = ExtractExpression(scope, literalFloat(3.0))(pos)
val expected = ListComprehension(scope, literalFloat(3.0))(pos)
val expected = ListComprehension(scope, literalFloat(3.0), generatedThroughRewrite = true)(pos)

replaceAliasedFunctionInvocations(Deprecations.V1)(before) should equal(before)
replaceAliasedFunctionInvocations(Deprecations.V2)(before) should equal(expected)
Expand All @@ -80,7 +80,7 @@ class ReplaceAliasedFunctionInvocationsTest extends CypherFunSuite with AstConst
val scopePosition = InputPosition(30, 1, 31)
val scope = FilterScope(varFor("a"), Some(TRUE))(scopePosition)
val before = FilterExpression(scope, literalFloat(3.0))(pos)
val expected = ListComprehension(ExtractScope(varFor("a"), Some(TRUE), None)(scopePosition), literalFloat(3.0))(pos)
val expected = ListComprehension(ExtractScope(varFor("a"), Some(TRUE), None)(scopePosition), literalFloat(3.0), generatedThroughRewrite = true)(pos)

replaceAliasedFunctionInvocations(Deprecations.V1)(before) should equal(before)
replaceAliasedFunctionInvocations(Deprecations.V2)(before) should equal(expected)
Expand Down

0 comments on commit cad62d9

Please sign in to comment.