Skip to content

Commit

Permalink
Optimization: GroupStepFilter to handle within (#331)
Browse files Browse the repository at this point in the history
- resolves #329

Signed-off-by: Dwitry dwitry@users.noreply.github.com
  • Loading branch information
dwitry committed Nov 6, 2019
1 parent 13a6ea1 commit 4b70694
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ object GroupStepFilters extends GremlinRewriter {
case ChooseT3(Seq(Constant(value)), _, _) :: Is(_) :: As(_) :: SelectK(stepLabel) :: ChooseP2(_, Seq(Id)) :: Is(_) :: WhereP(
_: Eq) :: Nil =>
(stepLabel, HasP(T.id.getAccessor, Eq(value))) :: Nil
case ChooseT3(Seq(Constant(value)), _, _) :: Is(_) :: As(_) :: SelectK(stepLabel) :: ChooseP2(_, Seq(Id)) :: Is(_) :: WhereP(
_: Within) :: Nil =>
(stepLabel, HasP(T.id.getAccessor, Within(value))) :: Nil
case SelectK(stepLabel) :: rest if rest.forall(_.isInstanceOf[HasLabel]) =>
rest.map((stepLabel, _))
case _ =>
Expand All @@ -116,6 +119,9 @@ object GroupStepFilters extends GremlinRewriter {
case ChooseT3(Seq(Constant(_)), _, _) :: Is(_) :: As(_) :: SelectK(alias) :: ChooseP2(_, Seq(Id)) :: Is(_) :: WhereP(
_: Eq) :: Nil if aliases.contains(alias) =>
None
case ChooseT3(Seq(Constant(_)), _, _) :: Is(_) :: As(_) :: SelectK(alias) :: ChooseP2(_, Seq(Id)) :: Is(_) :: WhereP(
_: Within) :: Nil if aliases.contains(alias) =>
None
case SelectK(alias) :: rest if aliases.contains(alias) && rest.forall(_.isInstanceOf[HasLabel]) =>
None
case other =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.junit.Test
import org.opencypher.gremlin.translation.CypherAst.parse
import org.opencypher.gremlin.translation.Tokens
import org.opencypher.gremlin.translation.Tokens.{GENERATED, NULL, UNNAMED}
import org.opencypher.gremlin.translation.ir.builder.IRGremlinBindings
import org.opencypher.gremlin.translation.ir.helpers.CypherAstAssert.{P, __}
import org.opencypher.gremlin.translation.ir.helpers.CypherAstAssertions.assertThat
import org.opencypher.gremlin.translation.ir.model.GremlinBinding
Expand Down Expand Up @@ -283,4 +284,17 @@ class GroupStepFiltersTest {
.rewritingWith(GroupStepFilters)
.keeps(__.addV().as("a").property(single, "x", __.constant(1)))
}

@Test
def collectionOfParameters(): Unit = {
val ids = new IRGremlinBindings().bind("ids", 1)

This comment has been minimized.

Copy link
@mad

mad Nov 7, 2019

Contributor

Could you provide test where "ids" is array of elements?

I think current translation is wrong, because from my tests P.within will accept new Object[] { List } instead of simple List

This comment has been minimized.

Copy link
@dwitry

dwitry Nov 11, 2019

Author Contributor

You are right, thanks for reporting. Created bug #334.


assertThat(parse("MATCH (p:Person) WHERE id(p) in {ids} RETURN p.name"))
.withFlavor(flavor)
.rewritingWith(GroupStepFilters)
.removes(__.choose(__.constant(ids), __.constant(ids), __.constant(NULL)))
.removes(__.where(P.within(GENERATED + "1")))
.adds(__.has("~id", P.within(ids)))
.debug()
}
}

0 comments on commit 4b70694

Please sign in to comment.