opencypher / cypher-for-gremlin

Cypher for Gremlin adds Cypher support to any Gremlin graph database.
Apache License 2.0
359 stars 48 forks source link

Rewrite where with id #227

Closed mad closed 5 years ago

mad commented 5 years ago

I try fix that issue https://github.com/opencypher/cypher-for-gremlin/issues/223

Query like that

MATCH (n)
WHERE id(n) in [1, 2]
RETURN n

MATCH (n)
WHERE id(n) = 1
RETURN n

may translate to:

g.V().has('~id', within(1, 2))...
g.V().has('~id', 1)...
dwitry commented 5 years ago

Hello @mad, thank you for your contribution. I will improve translation to simplify your change as mentioned in previous comment. Then we will merge this.

mad commented 5 years ago

I found problem with UNWIND

UNWIND {data} as data 
MERGE (n {a: data.name})
SET n.name = data.name
RETURN id(n)

Translated to

g.inject('  cypher.start').choose(__.constant('  cypher.null'), __.constant('  cypher.null'), __.constant('  cypher.null')).unfold().as('data')
.choose(
__.V().as('n').where(__.project('  GENERATED4', '  GENERATED5').by(__.select('data')).by(__.constant('name')).select(values).map(cypherContainerIndex()).is(neq('  cypher.null')).as('  GENERATED3')
.select('n').values('a').where(eq('  GENERATED3'))),
 __.V().as('n').where(__.project('  GENERATED4', ' GENERATED5').by(__.select('data')).by(__.constant('name')).select(values).map(cypherContainerIndex()).is(neq('  cypher.null')).as('  GENERATED3')
.select('n').values('a').where(eq('  GENERATED3'))), __.identity().addV().as('n').property(single, 'a', __.project('  GENERATED1', '  GENERATED2').by(__.select('data')).by(__.constant('name')).select(values).map(cypherContainerIndex())))
.select('n').choose(neq('  cypher.null'), __.choose(__.project('  GENERATED6', '  GENERATED7').by(__.select('data')).by(__.constant('name')).select(values).map(cypherContainerIndex()).is(neq('  cypher.null')).unfold(), __.property(single, 'name', __.project('  GENERATED6', '  GENERATED7').by(__.select('data')).by(__.constant('name')).select(values).map(cypherContainerIndex())), __.sideEffect(__.properties('name').drop()))).select('n').project('id(n)').by(__.choose(neq('  cypher.null'), __.id()))

But extract has(propperty, value is not trivial. I think it's possible unfold unwind to multiple queries with constant and then GroupStepRewriter can rewrite it

dwitry commented 5 years ago

Hello @mad. Regarding UNWIND. You are right, theoretically it should be possible to extract translation of expression to variable in WhereWalker, to reduce amount of steps in where clause, therefore making it easier to match in GroupStepRewriter.

But its significant change, and I suggest to implement it in scope of other PR. If you agree we can merge this.

mad commented 5 years ago

Ok, I'm agree