Allow SET += / = Syntax on variables with no semantic information

SET x += {prop: 5} would previously fail if x had no semantic
information, even if it turned out to be a node/relationship at
runtime. This is now fixed.

This also fixes a tangential problem with invalidation of cached
properties in +=.

This also fixes a tangential problem with eagerization of SET for
different cases.

community/community-it/cypher-it/src/test/scala/org/neo4j/cypher/CypherIsolationIntegrationTest.scala

@@ -78,6 +78,26 @@ class CypherIsolationIntegrationTest extends ExecutionEngineFunSuite {
     nodeGetProperty(n, "x") should equal(THREADS * UPDATES)
   }
 
+  test("Should work around read isolation limitations using explicit lock for cached node properties with map +=") {
+    // Given
+    val n = createLabeledNode(Map("x" -> 0L), "L")
+    graph.createIndex("L", "x")
+
+    val query =
+      """MATCH (n:L) WHERE exists(n.x)
+        |SET n += {_LOCK_: true}
+        |WITH n, n.x AS x
+        |SET n.x = x + 1
+        |REMOVE n._LOCK_""".stripMargin
+
+
+    // When
+    race(query)
+
+    // Then
+    nodeGetProperty(n, "x") should equal(THREADS * UPDATES)
+  }
+
   private def race(query: String): Unit = {
     val executor = Executors.newFixedThreadPool(THREADS)
 

community/cypher/cypher-logical-plans-3.5/src/main/scala/org/neo4j/cypher/internal/v3_5/logical/plans/SetPropertiesFromMap.scala

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2002-2019 "Neo4j,"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.cypher.internal.v3_5.logical.plans
+
+import org.neo4j.cypher.internal.ir.v3_5.StrictnessMode
+import org.neo4j.cypher.internal.v3_5.expressions.Expression
+import org.neo4j.cypher.internal.v3_5.util.attribution.IdGen
+
+/**
+  * for ( row <- source )
+  * thing = row.get(idName)
+  * for ( (key,value) <- row.evaluate( expression ) )
+  *     thing.setProperty( key, value )
+  *
+  * produce row
+  */
+case class SetPropertiesFromMap(source: LogicalPlan,
+                                entity: Expression,
+                                expression: Expression,
+                                removeOtherProps: Boolean
+                               )(implicit idGen: IdGen) extends LogicalPlan(idGen) {
+
+  override def lhs: Option[LogicalPlan] = Some(source)
+
+  override val availableSymbols: Set[String] = source.availableSymbols
+
+  override def rhs: Option[LogicalPlan] = None
+
+  override def strictness: StrictnessMode = source.strictness
+}

community/cypher/cypher-planner-3.5/src/main/scala/org/neo4j/cypher/internal/compiler/v3_5/ast/convert/plannerQuery/ClauseConverters.scala

@@ -333,11 +333,17 @@ object ClauseConverters {
     case SetExactPropertiesFromMapItem(rel, expression) if semanticTable.isRelationship(rel) =>
       SetRelationshipPropertiesFromMapPattern(rel.name, expression, removeOtherProps = true)
 
+    case SetExactPropertiesFromMapItem(vr, expression) =>
+      SetPropertiesFromMapPattern(vr, expression, removeOtherProps = true)
+
     case SetIncludingPropertiesFromMapItem(node, expression) if semanticTable.isNode(node) =>
       SetNodePropertiesFromMapPattern(node.name, expression, removeOtherProps = false)
 
     case SetIncludingPropertiesFromMapItem(rel, expression) if semanticTable.isRelationship(rel) =>
       SetRelationshipPropertiesFromMapPattern(rel.name, expression, removeOtherProps = false)
+
+    case SetIncludingPropertiesFromMapItem(vr, expression) =>
+      SetPropertiesFromMapPattern(vr, expression, removeOtherProps = false)
   }
 
   private def addMergeToLogicalPlanInput(builder: PlannerQueryBuilder, clause: Merge): PlannerQueryBuilder = {

community/cypher/cypher-planner-3.5/src/main/scala/org/neo4j/cypher/internal/compiler/v3_5/planner/logical/Eagerness.scala

@@ -331,12 +331,36 @@ object Eagerness {
         solveds.copy(apply.id, res.id)
         res
 
+      // L Ax (Sp R) => Sp Ax (L R)
+      case apply@Apply(lhs, set@SetRelationshipProperty(rhs, idName, key, value)) =>
+        val res = set.copy(source = Apply(lhs, rhs)(SameId(apply.id)), idName, key, value)(attributes.copy(set.id))
+        solveds.copy(apply.id, res.id)
+        res
+
+      // L Ax (Sp R) => Sp Ax (L R)
+      case apply@Apply(lhs, set@SetProperty(rhs, idName, key, value)) =>
+        val res = set.copy(source = Apply(lhs, rhs)(SameId(apply.id)), idName, key, value)(attributes.copy(set.id))
+        solveds.copy(apply.id, res.id)
+        res
+
       // L Ax (Sm R) => Sm Ax (L R)
       case apply@Apply(lhs, set@SetNodePropertiesFromMap(rhs, idName, expr, removes)) =>
         val res = set.copy(source = Apply(lhs, rhs)(SameId(apply.id)), idName, expr, removes)(attributes.copy(set.id))
         solveds.copy(apply.id, res.id)
         res
 
+      // L Ax (Sm R) => Sm Ax (L R)
+      case apply@Apply(lhs, set@SetRelationshipPropertiesFromMap(rhs, idName, expr, removes)) =>
+        val res = set.copy(source = Apply(lhs, rhs)(SameId(apply.id)), idName, expr, removes)(attributes.copy(set.id))
+        solveds.copy(apply.id, res.id)
+        res
+
+      // L Ax (Sm R) => Sm Ax (L R)
+      case apply@Apply(lhs, set@SetPropertiesFromMap(rhs, idName, expr, removes)) =>
+        val res = set.copy(source = Apply(lhs, rhs)(SameId(apply.id)), idName, expr, removes)(attributes.copy(set.id))
+        solveds.copy(apply.id, res.id)
+        res
+
       // L Ax (Sl R) => Sl Ax (L R)
       case apply@Apply(lhs, set@SetLabels(rhs, idName, labelNames)) =>
         val res = set.copy(source = Apply(lhs, rhs)(SameId(apply.id)), idName, labelNames)(attributes.copy(set.id))

community/cypher/cypher-planner-3.5/src/main/scala/org/neo4j/cypher/internal/compiler/v3_5/planner/logical/PlanUpdates.scala

@@ -125,6 +125,12 @@ case object PlanUpdates extends UpdatesPlanner {
         val updatedPattern = pattern.copy(expression = newExpression)
         context.logicalPlanProducer.planSetRelationshipPropertiesFromMap(updatedSource, updatedPattern, pattern, context)
 
+      //SET x += {p1: ..., p2: ...}
+      case pattern@SetPropertiesFromMapPattern(_, expression, _) =>
+        val (updatedSource, newExpression) = patternExpressionSolver.apply(source, expression, interestingOrder, context)
+        val updatedPattern = pattern.copy(expression = newExpression)
+        context.logicalPlanProducer.planSetPropertiesFromMap(updatedSource, updatedPattern, pattern, context)
+
       //REMOVE n:Foo:Bar
       case pattern: RemoveLabelPattern => context.logicalPlanProducer.planRemoveLabel(source, pattern, context)
 

community/cypher/cypher-planner-3.5/src/main/scala/org/neo4j/cypher/internal/compiler/v3_5/planner/logical/steps/LogicalPlanProducer.scala

@@ -737,6 +737,13 @@ case class LogicalPlanProducer(cardinalityModel: CardinalityModel, planningAttri
     annotate(SetRelationshipPropertiesFromMap(inner, patternToPlan.idName, patternToPlan.expression, patternToPlan.removeOtherProps), solved, providedOrders.get(inner.id), context)
   }
 
+  def planSetPropertiesFromMap(inner: LogicalPlan, patternToPlan: SetPropertiesFromMapPattern, solvedPattern: SetPropertiesFromMapPattern, context: LogicalPlanningContext): LogicalPlan = {
+
+    val solved = solveds.get(inner.id).amendQueryGraph(_.addMutatingPatterns(solvedPattern))
+
+    annotate(SetPropertiesFromMap(inner, patternToPlan.entityExpression, patternToPlan.expression, patternToPlan.removeOtherProps), solved, providedOrders.get(inner.id), context)
+  }
+
   def planSetProperty(inner: LogicalPlan, patternToPlan: SetPropertyPattern, solvedPattern: SetPropertyPattern, context: LogicalPlanningContext): LogicalPlan = {
     val solved = solveds.get(inner.id).amendQueryGraph(_.addMutatingPatterns(solvedPattern))
 

community/cypher/cypher-planner-3.5/src/test/scala/org/neo4j/cypher/internal/compiler/v3_5/planner/logical/unnestEagerTest.scala

@@ -75,6 +75,24 @@ class unnestEagerTest extends CypherFunSuite with LogicalPlanningTestSupport {
     rewrite(input) should equal(SetNodeProperty(Apply(lhs, rhs), "a", PropertyKeyName("prop")(pos), null))
   }
 
+  test("should unnest set rel property from rhs of apply") {
+    val lhs = newMockedLogicalPlan()
+    val rhs = newMockedLogicalPlan()
+    val set = SetRelationshipProperty(rhs, "a", PropertyKeyName("prop")(pos), null)
+    val input = Apply(lhs, set)
+
+    rewrite(input) should equal(SetRelationshipProperty(Apply(lhs, rhs), "a", PropertyKeyName("prop")(pos), null))
+  }
+
+  test("should unnest set generic property from rhs of apply") {
+    val lhs = newMockedLogicalPlan()
+    val rhs = newMockedLogicalPlan()
+    val set = SetProperty(rhs, varFor("a"), PropertyKeyName("prop")(pos), null)
+    val input = Apply(lhs, set)
+
+    rewrite(input) should equal(SetProperty(Apply(lhs, rhs), varFor("a"), PropertyKeyName("prop")(pos), null))
+  }
+
   test("should unnest set node property from map from rhs of apply") {
     val lhs = newMockedLogicalPlan()
     val rhs = newMockedLogicalPlan()
@@ -84,6 +102,24 @@ class unnestEagerTest extends CypherFunSuite with LogicalPlanningTestSupport {
     rewrite(input) should equal(SetNodePropertiesFromMap(Apply(lhs, rhs), "a", null, removeOtherProps = false))
   }
 
+  test("should unnest set relationship property from map from rhs of apply") {
+    val lhs = newMockedLogicalPlan()
+    val rhs = newMockedLogicalPlan()
+    val set = SetRelationshipPropertiesFromMap(rhs, "a", null, removeOtherProps = false)
+    val input = Apply(lhs, set)
+
+    rewrite(input) should equal(SetRelationshipPropertiesFromMap(Apply(lhs, rhs), "a", null, removeOtherProps = false))
+  }
+
+  test("should unnest set generic property from map from rhs of apply") {
+    val lhs = newMockedLogicalPlan()
+    val rhs = newMockedLogicalPlan()
+    val set = SetPropertiesFromMap(rhs, varFor("a"), null, removeOtherProps = false)
+    val input = Apply(lhs, set)
+
+    rewrite(input) should equal(SetPropertiesFromMap(Apply(lhs, rhs), varFor("a"), null, removeOtherProps = false))
+  }
+
   test("should unnest set labels from rhs of apply") {
     val lhs = newMockedLogicalPlan()
     val rhs = newMockedLogicalPlan()

community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/InterpretedPipeBuilder.scala

@@ -294,6 +294,10 @@ case class InterpretedPipeBuilder(recurse: LogicalPlan => Pipe,
         SetPipe(source,
           SetNodePropertyFromMapOperation(name, buildExpression(expression), removeOtherProps, needsExclusiveLock))(id = id)
 
+      case SetPropertiesFromMap(_, entityExpr, expression, removeOtherProps) =>
+        SetPipe(source,
+          SetPropertyFromMapOperation(buildExpression(entityExpr), buildExpression(expression), removeOtherProps))(id = id)
+
       case SetRelationshipProperty(_, name, propertyKey, expression) =>
         val needsExclusiveLock = ASTExpression.hasPropertyReadDependency(name, expression, propertyKey)
         SetPipe(source,

community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/pipes/SetOperation.scala

@@ -165,15 +165,17 @@ case class SetPropertyOperation(entityExpr: Expression, propertyKey: LazyPropert
   override def set(executionContext: ExecutionContext, state: QueryState) = {
     val resolvedEntity = entityExpr(executionContext, state)
     if (resolvedEntity != Values.NO_VALUE) {
-      val (entityId, ops) = resolvedEntity match {
-        case node: VirtualNodeValue => (node.id(), state.query.nodeOps)
-        case rel: VirtualRelationshipValue => (rel.id(), state.query.relationshipOps)
+      val (entityId, ops, invalidation) = resolvedEntity match {
+        case node: VirtualNodeValue => (node.id(), state.query.nodeOps, (id:Long) => executionContext.invalidateCachedProperties(id))
+        case rel: VirtualRelationshipValue => (rel.id(), state.query.relationshipOps, (_:Long) => {})
         case _ => throw new InvalidArgumentException(
           s"The expression $entityExpr should have been a node or a relationship, but got $resolvedEntity")
       }
       // better safe than sorry let's lock the entity
       ops.acquireExclusiveLock(entityId)
 
+      invalidation(entityId)
+
       try {
         setProperty(executionContext, state, ops, entityId, propertyKey, expression)
       } finally ops.releaseExclusiveLock(entityId)
@@ -188,29 +190,10 @@ case class SetPropertyOperation(entityExpr: Expression, propertyKey: LazyPropert
   }
 }
 
-abstract class SetPropertyFromMapOperation[T](itemName: String, expression: Expression,
-                                                                   removeOtherProps: Boolean) extends SetOperation {
-  override def set(executionContext: ExecutionContext, state: QueryState) = {
-    val item = executionContext(itemName)
-    if (item != Values.NO_VALUE) {
-      val ops = operations(state.query)
-      val itemId = id(item)
-      if (needsExclusiveLock) ops.acquireExclusiveLock(itemId)
-
-      try {
-        val map = SetOperation.toMap(executionContext, state, expression)
-
-        setPropertiesFromMap(state.query, ops, itemId, map, removeOtherProps)
-      } finally if (needsExclusiveLock) ops.releaseExclusiveLock(itemId)
-    }
-  }
-
-  protected def id(item: Any): Long
-
-  protected def operations(qtx: QueryContext): Operations[T]
+abstract class AbstractSetPropertyFromMapOperation(expression: Expression) extends SetOperation {
 
-  private def setPropertiesFromMap(qtx: QueryContext, ops: Operations[T], itemId: Long,
-                                   map: Map[Int, AnyValue], removeOtherProps: Boolean) {
+  protected def setPropertiesFromMap[T](qtx: QueryContext, ops: Operations[T], itemId: Long,
+                                     map: Map[Int, AnyValue], removeOtherProps: Boolean) {
 
     /*Set all map values on the property container*/
     for ((k, v) <- map) {
@@ -229,30 +212,97 @@ abstract class SetPropertyFromMapOperation[T](itemName: String, expression: Expr
       }
     }
   }
-
   override def registerOwningPipe(pipe: Pipe): Unit = expression.registerOwningPipe(pipe)
 }
 
+abstract class SetNodeOrRelPropertyFromMapOperation[T](itemName: String,
+                                                       expression: Expression,
+                                                       removeOtherProps: Boolean) extends AbstractSetPropertyFromMapOperation(expression) {
+  override def set(executionContext: ExecutionContext, state: QueryState) = {
+    val item = executionContext(itemName)
+    if (item != Values.NO_VALUE) {
+      val ops = operations(state.query)
+      val itemId = id(item)
+      if (needsExclusiveLock) ops.acquireExclusiveLock(itemId)
+
+      invalidateCachedProperties(executionContext, itemId)
+
+      try {
+        val map = SetOperation.toMap(executionContext, state, expression)
+
+        setPropertiesFromMap(state.query, ops, itemId, map, removeOtherProps)
+      } finally if (needsExclusiveLock) ops.releaseExclusiveLock(itemId)
+    }
+  }
+
+  protected def id(item: Any): Long
+
+  protected def operations(qtx: QueryContext): Operations[T]
+
+  protected def invalidateCachedProperties(executionContext: ExecutionContext, id: Long): Unit
+}
+
 case class SetNodePropertyFromMapOperation(nodeName: String, expression: Expression,
                                            removeOtherProps: Boolean, needsExclusiveLock: Boolean = true)
-  extends SetPropertyFromMapOperation[NodeValue](nodeName, expression, removeOtherProps) {
+  extends SetNodeOrRelPropertyFromMapOperation[NodeValue](nodeName, expression, removeOtherProps) {
 
   override def name = "SetNodePropertyFromMap"
 
   override protected def id(item: Any) = CastSupport.castOrFail[VirtualNodeValue](item).id()
 
   override protected def operations(qtx: QueryContext) = qtx.nodeOps
+
+  override protected def invalidateCachedProperties(executionContext: ExecutionContext, id: Long): Unit =
+    executionContext.invalidateCachedProperties(id)
 }
 
 case class SetRelationshipPropertyFromMapOperation(relName: String, expression: Expression,
                                                    removeOtherProps: Boolean, needsExclusiveLock: Boolean = true)
-  extends SetPropertyFromMapOperation[RelationshipValue](relName, expression, removeOtherProps) {
+  extends SetNodeOrRelPropertyFromMapOperation[RelationshipValue](relName, expression, removeOtherProps) {
 
   override def name = "SetRelationshipPropertyFromMap"
 
   override protected def id(item: Any) = CastSupport.castOrFail[VirtualRelationshipValue](item).id()
 
   override protected def operations(qtx: QueryContext) = qtx.relationshipOps
+
+  override protected def invalidateCachedProperties(executionContext: ExecutionContext, id: Long): Unit = {} // we do not cache relationships
+}
+
+case class SetPropertyFromMapOperation(entityExpr: Expression,
+                                       expression: Expression,
+                                       removeOtherProps: Boolean)
+  extends AbstractSetPropertyFromMapOperation(expression) {
+
+  override def name = "SetPropertyFromMap"
+
+  override def set(executionContext: ExecutionContext, state: QueryState) = {
+    val resolvedEntity = entityExpr(executionContext, state)
+    if (resolvedEntity != Values.NO_VALUE) {
+      val (entityId, ops, invalidation) = resolvedEntity match {
+        case node: VirtualNodeValue => (node.id(), state.query.nodeOps, (id:Long) => executionContext.invalidateCachedProperties(id))
+        case rel: VirtualRelationshipValue => (rel.id(), state.query.relationshipOps, (_:Long) => {})
+        case _ => throw new InvalidArgumentException(
+          s"The expression $entityExpr should have been a node or a relationship, but got $resolvedEntity")
+      }
+      ops.acquireExclusiveLock(entityId)
+
+      invalidation(entityId)
+
+      try {
+        val map = SetOperation.toMap(executionContext, state, expression)
+
+        setPropertiesFromMap(state.query, ops, entityId, map, removeOtherProps)
+      } finally ops.releaseExclusiveLock(entityId)
+    }
+  }
+
+  override def needsExclusiveLock = true
+
+  override def registerOwningPipe(pipe: Pipe): Unit = {
+    entityExpr.registerOwningPipe(pipe)
+    expression.registerOwningPipe(pipe)
+  }
 }
 
 case class SetLabelsOperation(nodeName: String, labels: Seq[LazyLabel]) extends SetOperation {

community/cypher/ir-3.5/src/main/scala/org/neo4j/cypher/internal/ir/v3_5/MutatingPattern.scala

@@ -61,6 +61,10 @@ case class SetRelationshipPropertiesFromMapPattern(idName: String, expression: E
   override def dependencies: Set[String] = deps(expression) + idName
 }
 
+case class SetPropertiesFromMapPattern(entityExpression: Expression, expression: Expression, removeOtherProps: Boolean) extends SetMutatingPattern {
+  override def dependencies: Set[String] = (entityExpression.dependencies ++ expression.dependencies).map(_.name)
+}
+
 case class SetNodePropertyPattern(idName: String, propertyKey: PropertyKeyName, expression: Expression) extends SetMutatingPattern {
   override def dependencies: Set[String] = deps(expression) + idName
 }