Merge pull request #7047 from davidegrohmann/2.3-fix-idp-solver-configs

Fix IDP solver configs

community/cypher/cypher-compiler-2.3/src/main/scala/org/neo4j/cypher/internal/compiler/v2_3/planner/CostBasedPipeBuilderFactory.scala

@@ -42,9 +42,9 @@ object CostBasedPipeBuilderFactory {
 
     def createQueryGraphSolver(n: CostBasedPlannerName): QueryGraphSolver = n match {
       case IDPPlannerName =>
-        IDPQueryGraphSolver(monitors.newMonitor[IDPQueryGraphSolverMonitor](), solverConfig = ConfigurableIDPSolverConfig(
+        IDPQueryGraphSolver(monitors.newMonitor[IDPQueryGraphSolverMonitor](), solverConfig = new ConfigurableIDPSolverConfig(
           maxTableSize = idpMaxTableSize,
-          iterationDuration = idpIterationDuration
+          iterationDurationLimit = idpIterationDuration
         ))
 
       case DPPlannerName =>

community/cypher/cypher-compiler-2.3/src/main/scala/org/neo4j/cypher/internal/compiler/v2_3/planner/logical/idp/IDPSolverConfig.scala

@@ -47,27 +47,26 @@ case object DefaultIDPSolverConfig extends IDPSolverConfig {
 
 /* The default settings for IDP uses a maxTableSize and a inner loop duration threshold
    to improve planning performance with minimal impact of plan quality */
-case class ConfigurableIDPSolverConfig(override val maxTableSize: Int, iterationDuration: Long) extends IDPSolverConfig {
-  override def iterationDurationLimit = iterationDuration
+class ConfigurableIDPSolverConfig(override val maxTableSize: Int,
+                                       override val iterationDurationLimit: Long) extends IDPSolverConfig {
   override def solvers(queryGraph: QueryGraph) = Seq(joinSolverStep(_), expandSolverStep(_))
 }
 
 /* For testing IDP we sometimes limit the solver to expands only */
-case object ExpandOnlyIDPSolverConfig extends IDPSolverConfig {
+case object ExpandOnlyIDPSolverConfig extends ConfigurableIDPSolverConfig(256, Long.MaxValue) {
   override def solvers(queryGraph: QueryGraph) = Seq(expandSolverStep(_))
 }
 
 /* For testing IDP we sometimes limit the solver to joins only */
-case object JoinOnlyIDPSolverConfig extends IDPSolverConfig {
+case object JoinOnlyIDPSolverConfig extends ConfigurableIDPSolverConfig(256, Long.MaxValue) {
   override def solvers(queryGraph: QueryGraph) = Seq(joinSolverStep(_))
 }
 
 /* One approach to optimizing planning of very long patterns is to only allow expands.
    This making planning very fast, but can lead to non-optimal plans where joins make more sense */
 case object ExpandOnlyWhenPatternIsLong extends IDPSolverConfig {
   override def solvers(queryGraph: QueryGraph) =
-    if(queryGraph.patternRelationships.size > 10) Seq(expandSolverStep(_))
-    else Seq(joinSolverStep(_), expandSolverStep(_))
+    Seq(AdaptiveSolverStep(_, (qg, goal) => qg.patternRelationships.size > 10))
 }
 
 /* One more advanced approach is to allow the inner loop to automatically switch from
@@ -78,5 +77,19 @@ case object ExpandOnlyWhenPatternIsLong extends IDPSolverConfig {
    time to develop more confidence in the approach. */
 case class AdaptiveChainPatternConfig(patternLengthThreshold: Int) extends IDPSolverConfig {
   override def solvers(queryGraph: QueryGraph) =
-    Seq(AdaptiveSolverStep(_, patternLengthThreshold))
+    Seq(AdaptiveSolverStep(_, (qg, goal) => goal.size >= patternLengthThreshold))
+}
+
+case class AdaptiveSolverStep(qg: QueryGraph, predicate: (QueryGraph, Goal) => Boolean) extends IDPSolverStep[PatternRelationship, LogicalPlan, LogicalPlanningContext] {
+
+  private val join = joinSolverStep(qg)
+  private val expand = expandSolverStep(qg)
+
+  override def apply(registry: IdRegistry[PatternRelationship], goal: Goal, table: IDPCache[LogicalPlan])
+                    (implicit context: LogicalPlanningContext): Iterator[LogicalPlan] = {
+    if (!registry.compacted() && predicate(qg, goal))
+      expand(registry, goal, table)
+    else
+      expand(registry, goal, table) ++ join(registry, goal, table)
+  }
 }

community/cypher/cypher-compiler-2.3/src/main/scala/org/neo4j/cypher/internal/compiler/v2_3/planner/logical/idp/IdRegistry.scala

@@ -28,6 +28,8 @@ import scala.collection.immutable.BitSet
 // can be stored efficiently using immutable BitSets
 //
 trait IdRegistry[I] {
+  def compacted(): Boolean
+
   // register elem and returns it's assigned id
   def register(elem: I): Int
 
@@ -113,6 +115,8 @@ class DefaultIdRegistry[I] extends IdRegistry[I] {
     }
     target.result()
   }
+
+  override def compacted() = compactionMap.nonEmpty
 }
 
 

community/cypher/cypher-compiler-2.3/src/test/scala/org/neo4j/cypher/internal/compiler/v2_3/planner/logical/idp/IDPQueryGraphSolverTest.scala

@@ -21,9 +21,9 @@ package org.neo4j.cypher.internal.compiler.v2_3.planner.logical.idp
 
 import org.mockito.Mockito.{times, verify, verifyNoMoreInteractions}
 import org.neo4j.cypher.internal.compiler.v2_3.pipes.LazyLabel
-import org.neo4j.cypher.internal.compiler.v2_3.planner.logical.Cardinality
+import org.neo4j.cypher.internal.compiler.v2_3.planner.logical.{Cardinality, LogicalPlanningContext}
 import org.neo4j.cypher.internal.compiler.v2_3.planner.logical.plans._
-import org.neo4j.cypher.internal.compiler.v2_3.planner.{PlannerQuery, LogicalPlanningTestSupport2, QueryGraph, Selections}
+import org.neo4j.cypher.internal.compiler.v2_3.planner.{LogicalPlanningTestSupport2, PlannerQuery, QueryGraph, Selections}
 import org.neo4j.cypher.internal.frontend.v2_3.SemanticDirection
 import org.neo4j.cypher.internal.frontend.v2_3.SemanticDirection.OUTGOING
 import org.neo4j.cypher.internal.frontend.v2_3.ast._
@@ -487,9 +487,17 @@ class IDPQueryGraphSolverTest extends CypherFunSuite with LogicalPlanningTestSup
     val solverConfigsToTest = Seq(
       ExpandOnlyIDPSolverConfig,
       ExpandOnlyWhenPatternIsLong,
-      ConfigurableIDPSolverConfig(maxTableSize = 32, iterationDuration = Long.MaxValue), // table limited
-      ConfigurableIDPSolverConfig(maxTableSize = Int.MaxValue, iterationDuration = 500), // time limited
-      AdaptiveChainPatternConfig(10)
+      new IDPSolverConfig {
+        override def solvers(queryGraph: QueryGraph): Seq[(QueryGraph) => IDPSolverStep[PatternRelationship, LogicalPlan, LogicalPlanningContext]] =
+          ExpandOnlyWhenPatternIsLong.solvers(queryGraph)
+        override def iterationDurationLimit: Long = 100
+      },
+      new ConfigurableIDPSolverConfig(maxTableSize = 32, iterationDurationLimit = Long.MaxValue), // table limited
+      new ConfigurableIDPSolverConfig(maxTableSize = Int.MaxValue, iterationDurationLimit = 500), // time limited
+      AdaptiveChainPatternConfig(10), // default
+      new AdaptiveChainPatternConfig(5) { // make sure it works on comprehsions for very long patterns
+        override def iterationDurationLimit: Long = 20
+      }
     )
 
     solverConfigsToTest.foreach { solverConfig =>