Deprecate legacy pattern syntax #9160

thobe · 2017-04-05T21:36:43Z

Ensure that Neo4j Cypher produces warning for pattern matching syntax that will be unsupported or have different semantics in future versions of Cypher.

Binding of relationships from variable length patterns is deprecated.
Queries such as:

MATCH (n)-[rs*]-() RETURN rs

Will generate a warning. The canonical way to write such a query is:

MATCH p=(n)-[*]-() RETURN relationships(p) AS rs

Using colon for each relationship type in a listing of multiple alternative relationship types in conjunction with using in-lined property predicates will change semantics in future versions.
This query:

MATCH ()-[:A|:B|:C{foo:'bar'}]-()

Currently means that the property predicate should apply to all relationships, but in future versions it will mean that the property predicate applies to relationships with type C, while no property predicates apply for relationships of type A or B. On order to achieve the current behaviour in future versions as well, only a single initial colon should be used, like so:

MATCH ()-[:A|B|C{foo:'bar'}]-()

changelog: Deprecated syntax for legacy patterns:

Binding a variable in a variable length pattern
Binding a variable when using colon for each relationship type in a pattern with multiple types
Inlining a property predicate when using colon for each relationship type in a pattern with multiple types
Using colon for each relationship type in a variable length pattern with multiple types

Mats-SX

I'd like to see a number of acceptance tests for this.

Mats-SX · 2017-04-06T07:39:35Z

...er/frontend-3.2/src/main/scala/org/neo4j/cypher/internal/frontend/v3_2/parser/Patterns.scala

-    | EMPTY ~ push(Seq())
+  private def RelationshipTypes: Rule1[MaybeLegacyRelTypes] = rule("relationship types") (
+    (":" ~~ RelTypeName ~~ zeroOrMore(WS ~ "|" ~~ LegacyCompatibleRelTypeName)) ~~>> (
+      (first: ast.RelTypeName, more:List[(Boolean,ast.RelTypeName)]) => (pos:InputPosition) => {


Code style: space after type-spec colon, space after comma.

Mats-SX · 2017-04-06T07:39:49Z

...er/frontend-3.2/src/main/scala/org/neo4j/cypher/internal/frontend/v3_2/parser/Patterns.scala

  )

+  private def LegacyCompatibleRelTypeName: Rule1[(Boolean,ast.RelTypeName)] =


code style: space after comma.

Mats-SX · 2017-04-06T07:40:04Z

...er/frontend-3.2/src/main/scala/org/neo4j/cypher/internal/frontend/v3_2/parser/Patterns.scala

  )

+  private def LegacyCompatibleRelTypeName: Rule1[(Boolean,ast.RelTypeName)] =
+    ((":" ~ push(true)) | EMPTY ~ push(false)) ~~ RelTypeName ~~>> (
+      (legacy:Boolean, name:ast.RelTypeName) => (pos:InputPosition) => (legacy,name))


Code style: space after colon and comma.

Mats-SX · 2017-04-06T07:40:23Z

...er/frontend-3.2/src/main/scala/org/neo4j/cypher/internal/frontend/v3_2/parser/Patterns.scala

@@ -122,3 +129,5 @@ trait Patterns extends Parser
    optional(WS ~ (MapLiteral | Parameter))
  )
 }
+
+case class MaybeLegacyRelTypes(types:Seq[ast.RelTypeName] = Seq.empty, legacySeparator:Boolean = false)


code style: space after colon.

Mats-SX · 2017-04-06T07:40:37Z

...rc/main/scala/org/neo4j/cypher/internal/frontend/v3_2/phases/SyntaxDeprecationWarnings.scala

@@ -35,6 +35,10 @@ object SyntaxDeprecationWarnings extends VisitorPhase[BaseContext, BaseState] {
    statement.treeFold(Set.empty[InternalNotification]) {
      case f@FunctionInvocation(_, FunctionName(name), _, _) if aliases.get(name).nonEmpty =>
        (seq) => (seq + DeprecatedFunctionNotification(f.position, name, aliases(name)), None)
+      case p@RelationshipPattern(Some(variable),_,Some(_),_, _, _) =>


code style: space after comma.

Mats-SX · 2017-04-06T07:40:50Z

...rc/main/scala/org/neo4j/cypher/internal/frontend/v3_2/phases/SyntaxDeprecationWarnings.scala

@@ -35,6 +35,10 @@ object SyntaxDeprecationWarnings extends VisitorPhase[BaseContext, BaseState] {
    statement.treeFold(Set.empty[InternalNotification]) {
      case f@FunctionInvocation(_, FunctionName(name), _, _) if aliases.get(name).nonEmpty =>
        (seq) => (seq + DeprecatedFunctionNotification(f.position, name, aliases(name)), None)
+      case p@RelationshipPattern(Some(variable),_,Some(_),_, _, _) =>
+        (seq) => (seq + DeprecatedVarLengthBindingNotification(p.position, variable.name), None)
+      case p@RelationshipPattern(_,_,_,Some(_),_,true) =>


code style: space after comma.

Mats-SX · 2017-04-06T07:49:32Z

...er/frontend-3.2/src/main/scala/org/neo4j/cypher/internal/frontend/v3_2/parser/Patterns.scala

  )

+  private def LegacyCompatibleRelTypeName: Rule1[(Boolean,ast.RelTypeName)] =
+    ((":" ~ push(true)) | EMPTY ~ push(false)) ~~ RelTypeName ~~>> (


I guess this is the line that sets the warning condition. So the warning will be emitted when doing

MATCH ()-[:FOO | :BAR]->()

but not when doing

MATCH ()-[:FOO | BAR]->()

and the presence of an inlined property predicate is not checked for. It would be nice to have some acceptance tests that confirms this interpretation.

Mats-SX · 2017-04-06T07:51:07Z

community/kernel/src/main/java/org/neo4j/graphdb/impl/notification/NotificationCode.java

+    DEPRECATED_BINDING_VAR_LENGTH_RELATIONSHIP(
+            SeverityLevel.WARNING,
+            Status.Statement.FeatureDeprecationWarning,
+            "Binding relationships to a list in a variable length pattern is deprecated."


I didn't find where this condition is checked, but perhaps that's still TODO?

thobe · 2017-04-06T09:38:03Z

I wonder if we also need to deprecate binding together with colon in alternatives. That would make the future syntax easier to implement.

i.e.

MATCH ()-[x:A|:B|:C]-()

boggle

I looked at this pairing w mats and am in favour of this.

NicholasIoanJones · 2017-05-15T21:25:57Z

I'm trying to model a situation where there is a heirarchy of organisations such that OrgA can be a MEMBEROF OrgB. Similarly User1 can be MEMBEROF OrgA. Finally a person P1 can give CONSENT to access their account details to either a User or an Organisation, with the expectation being that consent can flow from an Organisation to it's member orgs and ultimately to users.
I can then see who has CONSENT to see a persons account by querying
MATCH (p:Person{PersonID:10000002})-[:CONSENTS]->(o)<-[t:MEMBEROF *0..5]-(accessor) RETURN p, o, t, accessor
This seems to work and is reasonably concise but Neo4J says that this is deprecated - relating it to this change. How should I write this query?

Mats-SX · 2017-05-16T07:29:17Z

@NicholasIoanJones The canonical way, as outlined above, suggests your query in this form:

MATCH (p:Person {PersonID: 10000002})-[:CONSENTS]->(o),
  memPath = (o)<-[:MEMBEROF*0..5]-(accessor)
RETURN p, o, relationships(memPath) AS t, accessor

Note: Cypher style guide recommends property keys be written in camelCase with small starting character.

NicholasIoanJones · 2017-05-16T13:24:39Z

Thanks Mats, that works perfectly. I obviously need to read more and try out more to get my head better in tune with this. Style comment noted as well.

thobe added JUST FOR BUILDING NOT READY FOR MERGE labels Apr 5, 2017

thobe requested review from Mats-SX and boggle April 5, 2017 21:36

thobe force-pushed the 3.2-deprecate-legacy-pattern-syntax branch from 28662c1 to bbb2831 Compare April 6, 2017 07:13

Mats-SX requested changes Apr 6, 2017

View reviewed changes

thobe force-pushed the 3.2-deprecate-legacy-pattern-syntax branch from bbb2831 to 52bf95a Compare April 6, 2017 08:19

thobe added 3.2 cypher and removed JUST FOR BUILDING NOT READY FOR MERGE labels Apr 6, 2017

Mats-SX approved these changes Apr 6, 2017

View reviewed changes

Mats-SX self-assigned this Apr 6, 2017

Mats-SX added the changelog label Apr 6, 2017

thobe and others added 5 commits April 10, 2017 09:01

Outline deprecation warning for binding varlength relationship

7c892e1

Outline warning for colon between alt Relationship Types

2fb99d3

Test syntax deprecation warnings

90753ef

Verify that notifications make it end-to-end

57d8356

Deprecate variable length and variable binding in multi-colon-reltypes

b9d1ceb

thobe force-pushed the 3.2-deprecate-legacy-pattern-syntax branch from d11fd04 to b9d1ceb Compare April 10, 2017 07:02

Fix conflicts after rebase

d7e4f3f

boggle approved these changes Apr 10, 2017

View reviewed changes

Mats-SX merged commit 8684110 into neo4j:3.2 Apr 11, 2017

thobe deleted the 3.2-deprecate-legacy-pattern-syntax branch April 11, 2017 07:32

petraselmer mentioned this pull request Oct 6, 2017

Amended deprecated legacy pattern syntax to replacement syntax neo4j/neo4j-documentation#136

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate legacy pattern syntax #9160

Deprecate legacy pattern syntax #9160

thobe commented Apr 5, 2017 •

edited by Mats-SX

Mats-SX left a comment

Mats-SX Apr 6, 2017

Mats-SX Apr 6, 2017

Mats-SX Apr 6, 2017

Mats-SX Apr 6, 2017

Mats-SX Apr 6, 2017

Mats-SX Apr 6, 2017

Mats-SX Apr 6, 2017

Mats-SX Apr 6, 2017

thobe commented Apr 6, 2017 •

edited

boggle left a comment

NicholasIoanJones commented May 15, 2017

Mats-SX commented May 16, 2017 •

edited

NicholasIoanJones commented May 16, 2017

		)

		private def LegacyCompatibleRelTypeName: Rule1[(Boolean,ast.RelTypeName)] =

Deprecate legacy pattern syntax #9160

Deprecate legacy pattern syntax #9160

Conversation

thobe commented Apr 5, 2017 • edited by Mats-SX

Mats-SX left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thobe commented Apr 6, 2017 • edited

boggle left a comment

Choose a reason for hiding this comment

NicholasIoanJones commented May 15, 2017

Mats-SX commented May 16, 2017 • edited

NicholasIoanJones commented May 16, 2017

thobe commented Apr 5, 2017 •

edited by Mats-SX

thobe commented Apr 6, 2017 •

edited

Mats-SX commented May 16, 2017 •

edited