Clean up from code review

community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/planner/UpdateGraph.scala

@@ -94,7 +94,7 @@ case class UpdateGraph(mutatingPatterns: Seq[MutatingPattern] = Seq.empty) {
     mergeNodePatterns.nonEmpty || setLabelPatterns.nonEmpty || setNodePropertyPatterns.nonEmpty
 
   /*
-   * Does update graph contains MERGE
+   * Does this UpdateGraph contains merges
    */
   def containsMerge: Boolean = mergeNodePatterns.nonEmpty
 

community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/planner/logical/PlanSingleQuery.scala

@@ -36,11 +36,12 @@ case class PlanSingleQuery(planPart: (PlannerQuery, LogicalPlanningContext, Opti
 
   override def apply(in: PlannerQuery)(implicit context: LogicalPlanningContext): LogicalPlan = {
     val partPlan = countStorePlanner(in).getOrElse(planPart(in, context, None))
+   //always use eager if configured to do so
+    val alwaysEager = context.config.updateStrategy.alwaysEager
 
-    //we cannot force eagerness for merge queries
-    val alwaysEager = context.config.updateStrategy.alwaysEager && !in.updateGraph.containsMerge
+    ///we cannot force eagerness here for MERGE since it must be able to read it own writes
     val planWithEffect =
-      if (alwaysEager || conflicts(partPlan, in))
+      if ((alwaysEager && !in.updateGraph.containsMerge) ||conflicts(partPlan, in))
         context.logicalPlanProducer.planEager(partPlan)
       else partPlan
     val planWithUpdates = planUpdates(in, planWithEffect)(context)

community/cypher/cypher-compiler-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/planner/logical/PlanWithTail.scala

@@ -69,7 +69,7 @@ case class PlanWithTail(expressionRewriterFactory: (LogicalPlanningContext => Re
       case Some(query) =>
         val lhsContext = context.recurse(lhs)
         val partPlan = planPart(query, lhsContext, Some(context.logicalPlanProducer.planQueryArgumentRow(query.queryGraph)))
-        //we cannot force eagerness for merge queries
+        ///we cannot force eagerness for MERGE since it must be able to read it own writes
         val alwaysEager = context.config.updateStrategy.alwaysEager && !query.updateGraph.containsMerge
         //If reads interfere with writes, make it a RepeatableRead
         val planWithEffects =

community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/CypherCompiler.scala

@@ -91,9 +91,8 @@ class CypherCompiler(graph: GraphDatabaseService,
   def preParseQuery(queryText: String): PreParsedQuery = {
     val logger = new RecordingNotificationLogger
     val preParsedStatement = CypherPreParser(queryText)
-    val CypherStatementWithOptions(statement, offset, version, planner, runtime, updateStrategy, mode, notifications) = CypherStatementWithOptions(
-      preParsedStatement)
-    notifications.foreach( logger += _ )
+    val CypherStatementWithOptions(statement, offset, version, planner, runtime, updateStrategy, mode) =
+      CypherStatementWithOptions(preParsedStatement)
 
     val cypherVersion = version.getOrElse(configuredVersion)
     val pickedExecutionMode = mode.getOrElse(CypherExecutionMode.default)

community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/CypherStatementWithOptions.scala

@@ -20,22 +20,21 @@
 package org.neo4j.cypher.internal
 
 import org.neo4j.cypher.internal.frontend.v3_0.InputPosition
-import org.neo4j.cypher.internal.frontend.v3_0.notification.{InternalNotification, LegacyPlannerNotification}
+import org.neo4j.cypher.internal.frontend.v3_0.notification.InternalNotification
 import org.neo4j.cypher.{CypherPlanner, CypherRuntime, CypherUpdateStrategy, CypherVersion, InvalidArgumentException}
 
 import scala.annotation.tailrec
 
 object CypherStatementWithOptions {
   def apply(input: PreParsedStatement): CypherStatementWithOptions = {
-    val notifications = input.options.collectFirst { case _: PlannerPreParserOption => LegacyPlannerNotification }.toSeq
 
     @tailrec
     def recurse(options: List[PreParserOption], version: Option[CypherVersion],
                 planner: Option[CypherPlanner], runtime: Option[CypherRuntime],
                 updateStrategy: Option[CypherUpdateStrategy],
                 executionMode: Option[CypherExecutionMode]): CypherStatementWithOptions = options match {
       case Nil => CypherStatementWithOptions(input.statement, input.offset,
-                                             version, planner, runtime, updateStrategy, executionMode, notifications)
+                                             version, planner, runtime, updateStrategy, executionMode)
       case option :: tail =>
         option match {
           case e: ExecutionModePreParserOption  =>
@@ -73,5 +72,4 @@ case class CypherStatementWithOptions(statement: String, offset: InputPosition,
                                       planner: Option[CypherPlanner],
                                       runtime: Option[CypherRuntime],
                                       updateStrategy: Option[CypherUpdateStrategy],
-                                      executionMode: Option[CypherExecutionMode],
-                                      notifications: Seq[InternalNotification])
+                                      executionMode: Option[CypherExecutionMode])

community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/compatibility/CompatibilityFor3_0.scala

@@ -30,7 +30,7 @@ import org.neo4j.cypher.internal.compiler.v3_0.planDescription.InternalPlanDescr
 import org.neo4j.cypher.internal.compiler.v3_0.planDescription.{Argument, InternalPlanDescription, PlanDescriptionArgumentSerializer}
 import org.neo4j.cypher.internal.compiler.v3_0.tracing.rewriters.RewriterStepSequencer
 import org.neo4j.cypher.internal.compiler.v3_0.{CypherCompilerFactory, DPPlannerName, ExplainMode => ExplainModev3_0, GreedyPlannerName, IDPPlannerName, InfoLogger, Monitors, NormalMode => NormalModev3_0, PlannerName, ProfileMode => ProfileModev3_0, _}
-import org.neo4j.cypher.internal.frontend.v3_0.notification.{InternalNotification, LegacyPlannerNotification, PlannerUnsupportedNotification, RuntimeUnsupportedNotification, _}
+import org.neo4j.cypher.internal.frontend.v3_0.notification.{InternalNotification, PlannerUnsupportedNotification, RuntimeUnsupportedNotification, _}
 import org.neo4j.cypher.internal.frontend.v3_0.spi.MapToPublicExceptions
 import org.neo4j.cypher.internal.frontend.v3_0.{CypherException => InternalCypherException}
 import org.neo4j.cypher.internal.spi.v3_0.{GeneratedQueryStructure, TransactionBoundGraphStatistics, TransactionBoundPlanContext, TransactionBoundQueryContext}
@@ -56,7 +56,6 @@ object helpersv3_0 {
 }
 
 object exceptionHandlerFor3_0 extends MapToPublicExceptions[CypherException] {
-  import helpersv3_0._
   def syntaxException(message: String, query: String, offset: Option[Int], cause: Throwable) = new SyntaxException(message, query, offset, cause)
 
   def arithmeticException(message: String, cause: Throwable) = new ArithmeticException(message, cause)
@@ -140,7 +139,6 @@ case class WrappedMonitors3_0(kernelMonitors: KernelMonitors) extends Monitors {
 }
 
 trait CompatibilityFor3_0 {
-  import org.neo4j.cypher.internal.compatibility.helpersv3_0._
   val graph: GraphDatabaseService
   val queryCacheSize: Int
   val kernelMonitors: KernelMonitors
@@ -304,8 +302,6 @@ case class ExecutionResultWrapperFor3_0(inner: InternalExecutionResult, planner:
   private def asKernelNotification(notification: InternalNotification) = notification match {
     case CartesianProductNotification(pos, variables) =>
       NotificationCode.CARTESIAN_PRODUCT.notification(pos.asInputPosition, NotificationDetail.Factory.cartesianProduct(variables.asJava))
-    case LegacyPlannerNotification =>
-      NotificationCode.LEGACY_PLANNER.notification(InputPosition.empty)
     case LengthOnNonPathNotification(pos) =>
       NotificationCode.LENGTH_ON_NON_PATH.notification(pos.asInputPosition)
     case PlannerUnsupportedNotification =>

community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/helpers/wrappersFor2_3.scala

@@ -27,7 +27,7 @@ import org.neo4j.cypher.internal.compiler.v3_0.{CompilationPhaseTracer, CypherCo
 import org.neo4j.cypher.internal.frontend.v2_3.notification.{InternalNotification => InternalNotification2_3}
 import org.neo4j.cypher.internal.frontend.v2_3.{InputPosition => InputPosition2_3}
 import org.neo4j.cypher.internal.frontend.v3_0.InputPosition
-import org.neo4j.cypher.internal.frontend.v3_0.notification.{CartesianProductNotification, EagerLoadCsvNotification, IndexHintUnfulfillableNotification, IndexLookupUnfulfillableNotification, InternalNotification, JoinHintUnfulfillableNotification, JoinHintUnsupportedNotification, LargeLabelWithLoadCsvNotification, LegacyPlannerNotification, LengthOnNonPathNotification, MissingLabelNotification, MissingPropertyNameNotification, MissingRelTypeNotification, PlannerUnsupportedNotification, RuntimeUnsupportedNotification, UnboundedShortestPathNotification}
+import org.neo4j.cypher.internal.frontend.v3_0.notification.{CartesianProductNotification, EagerLoadCsvNotification, IndexHintUnfulfillableNotification, IndexLookupUnfulfillableNotification, InternalNotification, JoinHintUnfulfillableNotification, JoinHintUnsupportedNotification, LargeLabelWithLoadCsvNotification, LengthOnNonPathNotification, MissingLabelNotification, MissingPropertyNameNotification, MissingRelTypeNotification, PlannerUnsupportedNotification, RuntimeUnsupportedNotification, UnboundedShortestPathNotification}
 import org.neo4j.cypher.internal.frontend.{v2_3 => frontend2_3}
 
 /**
@@ -82,8 +82,6 @@ object wrappersFor2_3 {
   def as2_3(notification: InternalNotification): InternalNotification2_3 = notification match {
     case CartesianProductNotification(pos, variables) =>
       frontend2_3.notification.CartesianProductNotification(as2_3(pos), variables)
-    case LegacyPlannerNotification =>
-      frontend2_3.notification.LegacyPlannerNotification
     case LengthOnNonPathNotification(pos) =>
       frontend2_3.notification.LengthOnNonPathNotification(as2_3(pos))
     case PlannerUnsupportedNotification =>
@@ -116,7 +114,7 @@ object wrappersFor2_3 {
     case frontend2_3.notification.CartesianProductNotification(pos, variables) =>
       CartesianProductNotification(as3_0(pos), variables)
     case frontend2_3.notification.LegacyPlannerNotification =>
-      LegacyPlannerNotification
+      throw new InternalException("Syntax PLANNER COST/RULE no longer supported")
     case frontend2_3.notification.LengthOnNonPathNotification(pos) =>
       LengthOnNonPathNotification(as3_0(pos))
     case  frontend2_3.notification.PlannerUnsupportedNotification =>

community/cypher/cypher/src/test/java/org/neo4j/cypher/internal/CypherStatementWithOptionsTest.scala

@@ -24,10 +24,8 @@ import org.neo4j.cypher.internal.frontend.v3_0.test_helpers.CypherFunSuite
 
 class CypherStatementWithOptionsTest extends CypherFunSuite {
 
-  test("should not allow inconsistent planner options with new notation, old notation and mixed notation") {
-    intercept[InvalidArgumentException](CypherStatementWithOptions("CYPHER planner=cost PLANNER RULE RETURN 42"))
+  test("should not allow inconsistent planner options") {
     intercept[InvalidArgumentException](CypherStatementWithOptions("CYPHER planner=cost planner=rule RETURN 42"))
-    intercept[InvalidArgumentException](CypherStatementWithOptions("CYPHER PLANNER COST PLANNER RULE RETURN 42"))
   }
 
   test("should not allow inconsistent runtime options") {

community/cypher/cypher/src/test/scala/org/neo4j/cypher/EagerStrategyAcceptanceTest.scala

@@ -28,11 +28,4 @@ class EagerStrategyAcceptanceTest  extends ExecutionEngineFunSuite {
   test("should use eagerness when option is provided ") {
     execute("CYPHER strategy=eager MATCH () CREATE ()") should use("Eager")
   }
-
-  test("should not introduce eagerness in merge") {
-    val result = execute("CYPHER strategy=eager MERGE ()")
-
-    result shouldNot use("EagerApply")
-    result shouldNot use("Eager")
-  }
 }

community/cypher/frontend-3.0/src/main/scala/org/neo4j/cypher/internal/frontend/v3_0/notification/InternalNotification.scala

@@ -30,8 +30,6 @@ case class CartesianProductNotification(position: InputPosition, isolatedVariabl
 
 case class LengthOnNonPathNotification(position: InputPosition) extends InternalNotification
 
-case object LegacyPlannerNotification extends InternalNotification
-
 case object PlannerUnsupportedNotification extends InternalNotification
 
 case object RuntimeUnsupportedNotification extends InternalNotification