Skip to content

Commit

Permalink
Merge pull request #8738 from davidegrohmann/3.2-add-warning-rule-pla…
Browse files Browse the repository at this point in the history
…nner

Warn and fallback to CYPHER 3.1 if planner=rule is selected
  • Loading branch information
henriknyman committed Jan 31, 2017
2 parents 56cfed4 + ed76a17 commit 763f464
Show file tree
Hide file tree
Showing 12 changed files with 196 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ enum Statement implements Status
"might be used in order to find the requested shortest path." ),

// client notifications (not supported/deprecated)
PlannerUnavailableWarning( ClientNotification,
"The RULE planner is not available in the current CYPHER version, the query has been run by an older " +
"CYPHER version." ),
PlannerUnsupportedWarning( ClientNotification,
"This query is not supported by the COST planner." ),
RuntimeUnsupportedWarning( ClientNotification,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import java.time.Clock

import org.neo4j.cypher.internal.compatibility.v3_2.exceptionHandler
import org.neo4j.cypher.internal.compiler.v3_2._
import org.neo4j.cypher.internal.compiler.v3_2.codegen.CodeGenConfiguration
import org.neo4j.cypher.internal.frontend.v3_2.InputPosition
import org.neo4j.cypher.{InvalidArgumentException, SyntaxException, _}
import org.neo4j.graphdb.factory.GraphDatabaseSettings
import org.neo4j.graphdb.impl.notification.NotificationCode.RULE_PLANNER_UNAVAILABLE_FALLBACK
import org.neo4j.kernel.GraphDatabaseQueryService
import org.neo4j.kernel.api.KernelAPI
import org.neo4j.kernel.configuration.Config
Expand All @@ -36,7 +36,7 @@ import org.neo4j.logging.{Log, LogProvider}
object CypherCompiler {
val DEFAULT_QUERY_CACHE_SIZE: Int = 128
val DEFAULT_QUERY_PLAN_TTL: Long = 1000 // 1 second
val CLOCK = Clock.systemUTC()
val CLOCK: Clock = Clock.systemUTC()
val DEFAULT_STATISTICS_DIVERGENCE_THRESHOLD = 0.5
val DEFAULT_NON_INDEXED_LABEL_WARNING_THRESHOLD = 10000
}
Expand All @@ -45,7 +45,7 @@ case class PreParsedQuery(statement: String, rawStatement: String, version: Cyph
executionMode: CypherExecutionMode, planner: CypherPlanner, runtime: CypherRuntime,
codeGenMode: CypherCodeGenMode, updateStrategy: CypherUpdateStrategy)
(val offset: InputPosition) {
val statementWithVersionAndPlanner = {
val statementWithVersionAndPlanner: String = {
val plannerInfo = planner match {
case CypherPlanner.default => ""
case _ => s"planner=${planner.name}"
Expand Down Expand Up @@ -136,16 +136,32 @@ class CypherCompiler(graph: GraphDatabaseQueryService,
import org.neo4j.cypher.internal.compatibility.v2_3.helpers._
import org.neo4j.cypher.internal.compatibility.v3_1.helpers._

var version = preParsedQuery.version
val planner = preParsedQuery.planner
val runtime = preParsedQuery.runtime
val codeGenMode = preParsedQuery.codeGenMode
val updateStrategy = preParsedQuery.updateStrategy

preParsedQuery.version match {
case CypherVersion.v3_2 => planners(PlannerSpec_v3_2(planner, runtime, updateStrategy, codeGenMode)).produceParsedQuery(preParsedQuery, tracer)
case CypherVersion.v3_1 => planners(PlannerSpec_v3_1(planner, runtime, updateStrategy)).produceParsedQuery(preParsedQuery, as3_1(tracer))
case CypherVersion.v2_3 => planners(PlannerSpec_v2_3(planner, runtime)).produceParsedQuery(preParsedQuery, as2_3(tracer))
var preParsingNotifications: Set[org.neo4j.graphdb.Notification] = Set.empty
if (version == CypherVersion.v3_2 && planner == CypherPlanner.rule) {
val position = preParsedQuery.offset;
preParsingNotifications = preParsingNotifications + rulePlannerUnavaibleFallbackNotification(position)
version = CypherVersion.v3_1
}

version match {
case CypherVersion.v3_2 => planners(PlannerSpec_v3_2(planner, runtime, updateStrategy, codeGenMode))
.produceParsedQuery(preParsedQuery, tracer, preParsingNotifications)
case CypherVersion.v3_1 => planners(PlannerSpec_v3_1(planner, runtime, updateStrategy))
.produceParsedQuery(preParsedQuery, as3_1(tracer), preParsingNotifications)
case CypherVersion.v2_3 => planners(PlannerSpec_v2_3(planner, runtime))
.produceParsedQuery(preParsedQuery, as2_3(tracer), preParsingNotifications)
}
}

private def rulePlannerUnavaibleFallbackNotification(offset: InputPosition): org.neo4j.graphdb.Notification = {
val pos = new org.neo4j.graphdb.InputPosition(offset.offset, offset.line, offset.column)
RULE_PLANNER_UNAVAILABLE_FALLBACK.notification(pos)
}

private def getQueryCacheSize : Int = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ trait Compatibility {

implicit val executionMonitor = kernelMonitors.newMonitor(classOf[QueryExecutionMonitor])

def produceParsedQuery(preParsedQuery: PreParsedQuery, tracer: CompilationPhaseTracer) = {
def produceParsedQuery(preParsedQuery: PreParsedQuery, tracer: CompilationPhaseTracer, preParsingNotifications: Set[org.neo4j.graphdb.Notification]) = {
import org.neo4j.cypher.internal.compatibility.v2_3.helpers.as2_3
val notificationLogger = new RecordingNotificationLogger
val preparedQueryForV_2_3 =
Expand All @@ -79,14 +79,15 @@ trait Compatibility {
// Log notifications/warnings from planning
planImpl.notifications(planContext).foreach(notificationLogger += _)

(new ExecutionPlanWrapper(planImpl), extractedParameters)
(new ExecutionPlanWrapper(planImpl, preParsingNotifications), extractedParameters)
}

override def hasErrors = preparedQueryForV_2_3.isFailure
}
}

class ExecutionPlanWrapper(inner: ExecutionPlan_v2_3) extends org.neo4j.cypher.internal.ExecutionPlan {
class ExecutionPlanWrapper(inner: ExecutionPlan_v2_3, preParsingNotifications: Set[org.neo4j.graphdb.Notification])
extends org.neo4j.cypher.internal.ExecutionPlan {

private def queryContext(transactionalContext: TransactionalContextWrapper): QueryContext =
new ExceptionTranslatingQueryContext(new TransactionBoundQueryContext(transactionalContext))
Expand All @@ -104,7 +105,7 @@ trait Compatibility {
val innerResult = inner.run(queryContext(transactionalContext), transactionalContext.statement, innerExecutionMode, params)
new ClosingExecutionResult(
query,
new ExecutionResultWrapper(innerResult, inner.plannerUsed, inner.runtimeUsed),
new ExecutionResultWrapper(innerResult, inner.plannerUsed, inner.runtimeUsed, preParsingNotifications),
exceptionHandler.runSafely
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ object ExecutionResultWrapper {
}
}

class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: PlannerName, val runtime: RuntimeName)
extends ExecutionResult {
class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: PlannerName, val runtime: RuntimeName,
preParsingNotifications: Set[org.neo4j.graphdb.Notification]) extends ExecutionResult {

override def planDescriptionRequested = inner.planDescriptionRequested

Expand Down Expand Up @@ -101,7 +101,7 @@ class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: Pl

def executionType: QueryExecutionType = inner.executionType

def notifications = inner.notifications.map(asKernelNotification)
def notifications = inner.notifications.map(asKernelNotification) ++ preParsingNotifications

private def asKernelNotification(notification: InternalNotification) = notification match {
case CartesianProductNotification(pos, variables) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import org.neo4j.logging.Log

import scala.util.Try


trait Compatibility {
val graph: GraphDatabaseQueryService
val queryCacheSize: Int
Expand All @@ -58,18 +57,20 @@ trait Compatibility {

implicit val executionMonitor = kernelMonitors.newMonitor(classOf[QueryExecutionMonitor])

def produceParsedQuery(preParsedQuery: PreParsedQuery, tracer: CompilationPhaseTracer) = {
val notificationLogger = new RecordingNotificationLogger
val preparedSyntacticQueryForV_3_1 =
def produceParsedQuery(preParsedQuery: PreParsedQuery, tracer: CompilationPhaseTracer,
preParsingNotifications: Set[org.neo4j.graphdb.Notification]): ParsedQuery = {
val notificationLogger = new RecordingNotificationLogger
val preparedSyntacticQueryForV_3_1 =
Try(compiler.prepareSyntacticQuery(preParsedQuery.statement,
preParsedQuery.rawStatement,
notificationLogger,
preParsedQuery.planner.name,
Some(as3_1(preParsedQuery.offset)), tracer))
new ParsedQuery {
def isPeriodicCommit = preparedSyntacticQueryForV_3_1.map(_.isPeriodicCommit).getOrElse(false)
override def isPeriodicCommit: Boolean = preparedSyntacticQueryForV_3_1.map(_.isPeriodicCommit).getOrElse(false)

def plan(transactionalContext: TransactionalContextWrapperV3_2, tracer: v3_2.CompilationPhaseTracer): (ExecutionPlan, Map[String, Any]) =
override def plan(transactionalContext: TransactionalContextWrapperV3_2, tracer: v3_2.CompilationPhaseTracer):
(ExecutionPlan, Map[String, Any]) =
exceptionHandler.runSafely {
val tc = TransactionalContextWrapperV3_1(transactionalContext.tc)
val planContext = new ExceptionTranslatingPlanContext(new TransactionBoundPlanContext(tc, notificationLogger))
Expand All @@ -79,14 +80,14 @@ trait Compatibility {
// Log notifications/warnings from planning
planImpl.notifications(planContext).foreach(notificationLogger += _)

(new ExecutionPlanWrapper(planImpl), extractedParameters)
(new ExecutionPlanWrapper(planImpl, preParsingNotifications), extractedParameters)
}

override def hasErrors = preparedSyntacticQueryForV_3_1.isFailure
override def hasErrors: Boolean = preparedSyntacticQueryForV_3_1.isFailure
}
}

class ExecutionPlanWrapper(inner: ExecutionPlan_v3_1) extends ExecutionPlan {
class ExecutionPlanWrapper(inner: ExecutionPlan_v3_1, preParsingNotifications: Set[org.neo4j.graphdb.Notification]) extends ExecutionPlan {

private val searchMonitor = kernelMonitors.newMonitor(classOf[IndexSearchMonitor])

Expand All @@ -106,7 +107,7 @@ trait Compatibility {
val innerResult = inner.run(queryContext(transactionalContext), innerExecutionMode, innerParams)
new ClosingExecutionResult(
transactionalContext.tc.executingQuery(),
new ExecutionResultWrapper(innerResult, inner.plannerUsed, inner.runtimeUsed),
new ExecutionResultWrapper(innerResult, inner.plannerUsed, inner.runtimeUsed, preParsingNotifications),
exceptionHandler.runSafely
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ import org.neo4j.graphdb.impl.notification.{NotificationCode, NotificationDetail

import scala.collection.JavaConverters._


class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: PlannerName, val runtime: RuntimeName)
class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: PlannerName, val runtime: RuntimeName,
preParsingNotification: Set[org.neo4j.graphdb.Notification])
extends ExecutionResult {

override def planDescriptionRequested = inner.planDescriptionRequested
Expand Down Expand Up @@ -99,7 +99,7 @@ class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: Pl
}
}

override def notifications = inner.notifications.map(asKernelNotification)
override def notifications = inner.notifications.map(asKernelNotification) ++ preParsingNotification

private def asKernelNotification(notification: InternalNotification) = notification match {
case CartesianProductNotification(pos, variables) =>
Expand Down Expand Up @@ -167,4 +167,4 @@ object ExecutionResultWrapper {
case wrapper: ExecutionResultWrapper => Some((wrapper.inner, wrapper.planner, wrapper.runtime))
case _ => None
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ trait Compatibility {

protected val compiler: v3_2.CypherCompiler

implicit val executionMonitor = kernelMonitors.newMonitor(classOf[QueryExecutionMonitor])
implicit val executionMonitor: QueryExecutionMonitor = kernelMonitors.newMonitor(classOf[QueryExecutionMonitor])

def produceParsedQuery(preParsedQuery: PreParsedQuery, tracer: CompilationPhaseTracer) = {
def produceParsedQuery(preParsedQuery: PreParsedQuery, tracer: CompilationPhaseTracer,
preParsingNotifications: Set[org.neo4j.graphdb.Notification]): ParsedQuery = {
val notificationLogger = new RecordingNotificationLogger
val preparedSyntacticQueryForV_3_2 =
Try(compiler.parseQuery(preParsedQuery.statement,
Expand All @@ -61,24 +62,26 @@ trait Compatibility {
preParsedQuery.planner.name,
Some(preParsedQuery.offset), tracer))
new ParsedQuery {
def isPeriodicCommit = preparedSyntacticQueryForV_3_2.map(_.isPeriodicCommit).getOrElse(false)
override def isPeriodicCommit: Boolean = preparedSyntacticQueryForV_3_2.map(_.isPeriodicCommit).getOrElse(false)

def plan(transactionalContext: TransactionalContextWrapper, tracer: CompilationPhaseTracer): (ExecutionPlan, Map[String, Any]) = exceptionHandler.runSafely {
override def plan(transactionalContext: TransactionalContextWrapper, tracer: CompilationPhaseTracer):
(ExecutionPlan, Map[String, Any]) = exceptionHandler.runSafely {
val planContext = new ExceptionTranslatingPlanContext(new TransactionBoundPlanContext(transactionalContext, notificationLogger))
val syntacticQuery = preparedSyntacticQueryForV_3_2.get
val (planImpl, extractedParameters) = compiler.planPreparedQuery(syntacticQuery, notificationLogger, planContext, Some(preParsedQuery.offset), tracer)

// Log notifications/warnings from planning
planImpl.notifications(planContext).foreach(notificationLogger.log)

(new ExecutionPlanWrapper(planImpl), extractedParameters)
(new ExecutionPlanWrapper(planImpl, preParsingNotifications), extractedParameters)
}

override def hasErrors = preparedSyntacticQueryForV_3_2.isFailure
override def hasErrors: Boolean = preparedSyntacticQueryForV_3_2.isFailure
}
}

class ExecutionPlanWrapper(inner: ExecutionPlan_v3_2) extends ExecutionPlan {
class ExecutionPlanWrapper(inner: ExecutionPlan_v3_2, preParsingNotifications: Set[org.neo4j.graphdb.Notification])
extends ExecutionPlan {

private val searchMonitor = kernelMonitors.newMonitor(classOf[IndexSearchMonitor])

Expand All @@ -98,7 +101,7 @@ trait Compatibility {
val innerResult = inner.run(queryContext(transactionalContext), innerExecutionMode, innerParams)
new ClosingExecutionResult(
transactionalContext.tc.executingQuery(),
new ExecutionResultWrapper(innerResult, inner.plannerUsed, inner.runtimeUsed),
new ExecutionResultWrapper(innerResult, inner.plannerUsed, inner.runtimeUsed, preParsingNotifications),
exceptionHandler.runSafely
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ object ExecutionResultWrapper {
}
}

class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: PlannerName, val runtime: RuntimeName)
extends ExecutionResult {
class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: PlannerName, val runtime: RuntimeName,
preParsingNotifications: Set[org.neo4j.graphdb.Notification]) extends ExecutionResult {

override def planDescriptionRequested = inner.planDescriptionRequested
override def javaIterator = inner.javaIterator
Expand Down Expand Up @@ -106,7 +106,7 @@ class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: Pl
}
}

override def notifications = inner.notifications.map(asKernelNotification)
override def notifications = inner.notifications.map(asKernelNotification) ++ preParsingNotifications

private def asKernelNotification(notification: InternalNotification) = notification match {
case CartesianProductNotification(pos, variables) =>
Expand Down Expand Up @@ -166,4 +166,4 @@ class ExecutionResultWrapper(val inner: InternalExecutionResult, val planner: Pl
private implicit class ConvertibleCompilerInputPosition(pos: frontend.v3_2.InputPosition) {
def asInputPosition = new graphdb.InputPosition(pos.offset, pos.line, pos.column)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Copyright (c) 2002-2017 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.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.javacompat;

import org.junit.Rule;
import org.junit.Test;

import java.util.Map;
import java.util.stream.Stream;

import org.neo4j.graphdb.InputPosition;
import org.neo4j.graphdb.Result;
import org.neo4j.helpers.collection.Iterables;
import org.neo4j.kernel.internal.GraphDatabaseAPI;
import org.neo4j.test.rule.ImpermanentDatabaseRule;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.neo4j.graphdb.impl.notification.NotificationCode.RULE_PLANNER_UNAVAILABLE_FALLBACK;

public class NotificationAcceptanceTest
{
@Rule
public final ImpermanentDatabaseRule rule = new ImpermanentDatabaseRule();

@Test
public void shouldNotifyWhenUsingCypher3_1ForTheRulePlannerWhenCypherVersionIsTheDefault() throws Exception
{
// when
Result result = db().execute( "CYPHER planner=rule RETURN 1" );
InputPosition position = new InputPosition( 20, 1, 21 );

// then
assertThat( result.getNotifications(), contains( RULE_PLANNER_UNAVAILABLE_FALLBACK.notification( position ) ) );
Map<String,Object> arguments = result.getExecutionPlanDescription().getArguments();
assertThat( arguments.get( "version" ), equalTo( "CYPHER 3.1" ) );
assertThat( arguments.get( "planner" ), equalTo( "RULE" ) );
result.close();
}

@Test
public void shouldNotifyWhenUsingCypher3_1ForTheRulePlannerWhenCypherVersionIs3_2() throws Exception
{
// when
Result result = db().execute( "CYPHER 3.2 planner=rule RETURN 1" );
InputPosition position = new InputPosition( 24, 1, 25 );

// then
assertThat( result.getNotifications(), contains( RULE_PLANNER_UNAVAILABLE_FALLBACK.notification( position ) ) );
Map<String,Object> arguments = result.getExecutionPlanDescription().getArguments();
assertThat( arguments.get( "version" ), equalTo( "CYPHER 3.1" ) );
assertThat( arguments.get( "planner" ), equalTo( "RULE" ) );
result.close();
}

@Test
public void shouldNotNotifyWhenUsingTheRulePlannerWhenCypherVersionIsNot3_2() throws Exception
{
Stream.of( "CYPHER 3.1", "CYPHER 2.3" ).forEach( version ->
{
// when
Result result = db().execute( version + " planner=rule RETURN 1" );

// then
assertThat( Iterables.asList( result.getNotifications() ), empty() );
Map<String,Object> arguments = result.getExecutionPlanDescription().getArguments();
assertThat( arguments.get( "version" ), equalTo( version ) );
assertThat( arguments.get( "planner" ), equalTo( "RULE" ) );
result.close();
});
}

private GraphDatabaseAPI db()
{
return rule.getGraphDatabaseAPI();
}
}

0 comments on commit 763f464

Please sign in to comment.