existential pattern match

[DvirDukhan]

No description provided.

[hvub]

The accepted CIP is the relevant baseline for this PR. WHERE <pattern> we should not have in scenarios. EXISTS( <pattern> ) (not mentioned in the CIP) is probably still best tested in expressions/pattern/Pattern1. The "simple subquery exists" and "subquery exists" functionality described in the CIP should probably go as two features in a new separate category, such as expressions/existentialSubquery/existentialSubquery1 (and .../existentialSubquery2).

[hvub]

Suggested change

	MATCH (n) WHERE (n)-->() RETURN n
	MATCH (n) WHERE exists( (n)-->() ) RETURN n

[hvub]

I have no confirmation yet, however:

• the CIP does not mention exists( <pattern> )
• size( <pattern> ) gets deprecated because if this kind of ugly syntax ambiguity between parenthesized expressions and node patterns
• exists( <pattern> ) suffers the same kind of ugly syntax ambiguity between parenthesized expressions and node patterns

Hence, it would be kind of odd to keep exists( <pattern> ). So, based on that thinking, it looks much more likely that is deprecated too and hence it seems smarter to not invest into exists( <pattern> ) any effort.

Further, assuming exists( <pattern> ) goes out like size( <pattern> ) does, the category pattern will on contain pattern comprehensions. That give us the possibility to spilt pattern comprehension into multiple feature. However, that can happen in separate PR at a different time. Just something to keep in mind.

[hvub]

I just realized that size(<pattern>) fails on the openCypher grammar, when the <pattern> is a node pattern. I reckon exists(<pattern>) will fail, too. From viewpoint of the grammar both are simply FunctionInvocations, which only allow expressions as parameters. Expressions currently allow RelationshipsPatterns, but I assume that will go away, too.

Anyway, the practical consequence of this is that scenarios that test the failing of these functions on a pattern argument require a @skipGrammarCheck tag, otherwise the integrity check will complain:
the query of step `When executing query:` in line XX has either a syntax conforming to the grammar or the scenario has the @skipGrammarCheck tag

[hvub]

Update: Given the state of the discussion on <pattern> and exist( <pattern> ) in expressions that I am observing, the best is probably to not touch pattern/Pattern1.feature either was for the time being.

Nevertheless, working on the existential subquery part in a separate existentialSubquery category is good. That is stable.

[hvub]

See comments.

[hvub]

Suggested change

	MATCH (n), (m) WHERE (n)-->(m) RETURN n. m
	MATCH (n), (m) WHERE (n)-->(m) RETURN n, m

[hvub]

This scenario should actually work, so I would change it to something like

Suggested change

	Scenario: [13] Fail on matching two nodes on a single undirected connection between them
	Given any graph
	When executing query:
	"""
	MATCH (n), (m) WHERE (n)--(m) RETURN n. m
	"""
	Then a SyntaxError should be raised at compile time: RequiresDirectedRelationship
	Scenario: [13] Fail on matching two nodes on a single undirected connection between them
	Given an empty graph
	And having executed:
	"""
	CREATE (a:A)-[:REL1]->(b:B), (b)-[:REL2]->(a), (a)-[:REL3]->(:C), (a)-[:REL1]->(:D)
	"""
	When executing query:
	"""
	MATCH (n), (m) WHERE (n)--(m) RETURN n, m
	"""
	Then the result should be, in any order:
	| n | m |
	| (:A) | (:B) |
	| (:B) | (:A) |
	| (:A) | (:C) |
	| (:A) | (:D) |
	| (:C) | (:A) |
	| (:D) | (:A) |
	And no side effects

[hvub]

I would expand this scenario to an outline to cover more failing patterns:

Suggested change

	Scenario: [10] Fail on introducing unbounded variable in pattern
	Given any graph
	When executing query:
	"""
	MATCH (n) WHERE (n)-->(m) RETURN n
	"""
	Then a SyntaxError should be raised at compile time: UndefinedVariable
	Scenario Outline: [10] Fail on introducing unbounded variables in pattern
	Given any graph
	When executing query:
	"""
	MATCH (n) WHERE <pattern> RETURN n
	"""
	Then a SyntaxError should be raised at compile time: UndefinedVariable
	Examples:
	| pattern |
	| (a) |
	| (n)-->(a) |
	| (a)-->(n) |
	| (n)<--(a) |
	| (n)--(a) |
	| (n)-[r]->() |
	| ()-[r]->(n) |
	| (n)<-[r]-() |
	| (n)-[r]-() |
	| ()-[r]->() |
	| ()<-[r]-() |
	| ()-[r]-() |
	| (n)-[r:REL]->(a {num: 5}) |
	| (n)-[r:REL*0..2]->(a {num: 5}) |
	| (n)-[r:REL]->(:C)<-[s:REL]-(a {num: 5}) |

[hvub]

Since this pattern expression is supposed to be only allowed in WHERE, I would add scenarios that check that is fails in other places.
I would also scenario showing that it is combinable with other predicts in WHERE.

Suggested change

	And no side effects
	And no side effects
	Scenario: [19] Using a negated existential pattern predicate
	Given an empty graph
	And having executed:
	"""
	CREATE (a:A)-[:REL1]->(b:B), (b)-[:REL2]->(a), (a)-[:REL3]->(:C), (a)-[:REL1]->(:D)
	"""
	When executing query:
	"""
	MATCH (n) WHERE NOT (n)-[:REL2]-() RETURN n
	"""
	Then the result should be, in any order:
	| n |
	| (:A) |
	And no side effects
	Scenario: [20] Using two existential pattern predicates in a conjunction
	Given an empty graph
	And having executed:
	"""
	CREATE (a:A)-[:REL1]->(b:B), (b)-[:REL2]->(a), (a)-[:REL3]->(:C), (a)-[:REL1]->(:D)
	"""
	When executing query:
	"""
	MATCH (n) WHERE (n)-[:REL1]-() AND (n)-[:REL3]-() RETURN n
	"""
	Then the result should be, in any order:
	| n |
	| (:A) |
	And no side effects
	Scenario: [21] Using two existential pattern predicates in a disjunction
	Given an empty graph
	And having executed:
	"""
	CREATE (a:A)-[:REL1]->(b:B), (b)-[:REL2]->(a), (a)-[:REL3]->(:C), (a)-[:REL1]->(:D)
	"""
	When executing query:
	"""
	MATCH (n) WHERE (n)-[:REL1]-() OR (n)-[:REL2]-() RETURN n
	"""
	Then the result should be, in any order:
	| n |
	| (:A) |
	| (:B) |
	And no side effects
	Scenario: [22] Fail on using pattern in RETURN projection
	Given any graph
	When executing query:
	"""
	MATCH (n) RETURN (n)-->()
	"""
	Then a SyntaxError should be raised at compile time: UnexpectedSyntax
	Scenario: [23] Fail on using pattern in WITH projection
	Given any graph
	When executing query:
	"""
	MATCH (n) WITH (n)-->() RETURN n
	"""
	Then a SyntaxError should be raised at compile time: UnexpectedSyntax

[hvub]

Hi Dvir, for further work on this PR, I like to draw your attention to PR #493, which has update to the CIP cited above and should form the basis for this feature here. The CIP update is obviously not merged, yet. Nevertheless, I am relatively confident that it illustrates the main direction of things. It also something you may want to make your colleagues aware of. Comment welcome, of course.

[hvub]

The CIP has been merged.

Further, I think we should rename Pattern1 - Existential Pattern Match into Pattern1 - Pattern predicate to cover what the CIP refers to as <RelationshipsPattern> (that is the name of the corresponding non-terminal in the oC grammar).

[hvub]

• Please add at least one scenarios to test the <RelationshipsPattern> is allowed in all subclauses listed in the CIP.
• Please add scenarios "Fail on using pattern in right-hand side of SET", e.g. SET n.prop = head(node(head((a)-[:REL]->()))).x(ideally a bit simpler, though).

[AviAvni]

this query
MATCH (n) SET n.prop = head(nodes(head((n)-[:REL]->()))).x
don't fail in neo4j

[hvub]

this query
MATCH (n) SET n.prop = head(nodes(head((n)-[:REL]->()))).x
don't fail in neo4j

That is right and expected. It is a legacy functionality that evaluates a naked pattern to a list. This will be removed in due course of the usual deprecation cycles.

[hvub]

I suggest a number of changes that should make the build green and fix some inconsistent indentation.

[hvub]

I did a more thorough review now. I have found a number of bugs and suggested fixes. With these done, the PR should be mergeable, I hope.

[hvub]

@AviAvni the PR has merge conflicts now. Please locally rebase the underlying branch onto a current master to draw out and fix the conflicts. Thanks.

[AviAvni]

@hvub done