Fix: revert to ignoring erroneous parameter values if they are not used

Core API users used to be allowed to pass in arbitrary objects as
parameters. If these were not used by Cypher, we would silently ignore
them. From 3.4.0 this was changed to always fail the query if any
parameter was not convertible to a cypher type. This commits reverts
to the previous lenient behaviour, as this change caused problems for
some users.

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

@@ -24,7 +24,7 @@ import org.neo4j.cypher.internal.util.v3_4.test_helpers.CypherFunSuite
 import org.neo4j.cypher.internal.javacompat.GraphDatabaseCypherService
 import org.neo4j.cypher.internal.planner.v3_4.spi.CostBasedPlannerName
 import org.neo4j.graphdb.factory.GraphDatabaseSettings
-import org.neo4j.graphdb.{ExecutionPlanDescription, GraphDatabaseService, Result}
+import org.neo4j.graphdb.{ExecutionPlanDescription, GraphDatabaseService, QueryExecutionException, Result}
 import org.neo4j.test.TestGraphDatabaseFactory
 
 import scala.collection.immutable.Map
@@ -195,6 +195,37 @@ class ExecutionEngineIT extends CypherFunSuite with GraphIcing {
     db.execute("EXPLAIN    MERGE (a:A) ON MATCH SET a.prop = 42 RETURN *").close()
   }
 
+  test("should crash of erroneous parameters values if they are used") {
+    // given
+    db = new TestGraphDatabaseFactory().newImpermanentDatabase()
+
+    // when
+    val params = new java.util.HashMap[String, AnyRef]()
+    params.put("erroneous", ErroneousParameterValue())
+
+    val result = db.execute("RETURN $erroneous AS x", params)
+
+    // then
+    intercept[QueryExecutionException] {
+      result.columnAs[Int]("x").next()
+    }
+  }
+
+  test("should ignore erroneous parameters values if they are not used") {
+    // given
+    db = new TestGraphDatabaseFactory().newImpermanentDatabase()
+
+    // when
+    val params = new java.util.HashMap[String, AnyRef]()
+    params.put("valid", Integer.valueOf(42))
+    params.put("erroneous", ErroneousParameterValue())
+
+    val result = db.execute("RETURN $valid AS x", params)
+
+    // then
+    result.columnAs[Int]("x").next() should equal(42)
+  }
+
   private implicit class RichDb(db: GraphDatabaseCypherService) {
     def planDescriptionForQuery(query: String): ExecutionPlanDescription = {
       val res = db.execute(query)
@@ -210,4 +241,6 @@ class ExecutionEngineIT extends CypherFunSuite with GraphIcing {
     def execute(query: String, params: Map[String, Any]): Result =
       engine.execute(query, params, engine.queryService.transactionalContext(query = query -> params))
   }
+
+  private case class ErroneousParameterValue()
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacade.java

@@ -408,7 +408,7 @@ public Result execute( String query, Map<String,Object> parameters ) throws Quer
         InternalTransaction transaction =
                 beginTransaction( KernelTransaction.Type.implicit, AUTH_DISABLED );
 
-        return execute( transaction, query, ValueUtils.asMapValue( parameters ) );
+        return execute( transaction, query, ValueUtils.asParameterMapValue( parameters ) );
     }
 
     @Override
@@ -417,7 +417,7 @@ public Result execute( String query, Map<String,Object> parameters, long timeout
     {
         InternalTransaction transaction =
                 beginTransaction( KernelTransaction.Type.implicit, AUTH_DISABLED, timeout, unit );
-        return execute( transaction, query, ValueUtils.asMapValue( parameters ) );
+        return execute( transaction, query, ValueUtils.asParameterMapValue( parameters ) );
     }
 
     public Result execute( InternalTransaction transaction, String query, MapValue parameters )

community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java

@@ -233,18 +233,31 @@ public static ListValue asListOfEdges( Relationship[] rels )
 
     public static MapValue asMapValue( Map<String,Object> map )
     {
-        return map( mapValues( map ) );
+        HashMap<String,AnyValue> newMap = new HashMap<>( map.size() );
+        for ( Map.Entry<String,Object> entry : map.entrySet() )
+        {
+            newMap.put( entry.getKey(), of( entry.getValue() ) );
+        }
+
+        return map( newMap );
     }
 
-    private static Map<String,AnyValue> mapValues( Map<String,Object> map )
+    public static MapValue asParameterMapValue( Map<String,Object> map )
     {
         HashMap<String,AnyValue> newMap = new HashMap<>( map.size() );
         for ( Map.Entry<String,Object> entry : map.entrySet() )
         {
-            newMap.put( entry.getKey(), of( entry.getValue() ) );
+            try
+            {
+                newMap.put( entry.getKey(), of( entry.getValue() ) );
+            }
+            catch ( IllegalArgumentException e )
+            {
+                newMap.put( entry.getKey(), VirtualValues.error( e ) );
+            }
         }
 
-        return newMap;
+        return map( newMap );
     }
 
     public static NodeValue fromNodeProxy( Node node )

community/shell/src/main/java/org/neo4j/shell/kernel/apps/cypher/Start.java

@@ -205,7 +205,7 @@ private TransactionalContext createTransactionContext( String queryText, Map<Str
                 new ShellConnectionInfo( session.getId() ),
                 transaction,
                 queryText,
-                ValueUtils.asMapValue( queryParameters )
+                ValueUtils.asParameterMapValue( queryParameters )
         );
     }
 }

community/values/src/main/java/org/neo4j/values/virtual/ErrorValue.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2002-2018 "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.values.virtual;
+
+
+import java.util.Comparator;
+
+import org.neo4j.values.AnyValue;
+import org.neo4j.values.AnyValueWriter;
+import org.neo4j.values.ValueMapper;
+import org.neo4j.values.VirtualValue;
+import org.neo4j.values.utils.InvalidValuesArgumentException;
+
+/**
+ * The ErrorValue allow delaying errors in value creation until runtime, which is useful
+ * if it turns out that the value is never used.
+ */
+public final class ErrorValue extends VirtualValue
+{
+    private final InvalidValuesArgumentException e;
+
+    ErrorValue( Exception e )
+    {
+        this.e = new InvalidValuesArgumentException( e.getMessage() );
+    }
+
+    @Override
+    public boolean equals( VirtualValue other )
+    {
+        throw e;
+    }
+
+    @Override
+    public VirtualValueGroup valueGroup()
+    {
+        return VirtualValueGroup.ERROR;
+    }
+
+    @Override
+    public int compareTo( VirtualValue other, Comparator<AnyValue> comparator )
+    {
+        throw e;
+    }
+
+    @Override
+    protected int computeHash()
+    {
+        throw e;
+    }
+
+    @Override
+    public <E extends Exception> void writeTo( AnyValueWriter<E> writer )
+    {
+        throw e;
+    }
+
+    @Override
+    public <T> T map( ValueMapper<T> mapper )
+    {
+        throw e;
+    }
+}

community/values/src/main/java/org/neo4j/values/virtual/VirtualValueGroup.java

@@ -31,4 +31,5 @@ public enum VirtualValueGroup
     LIST,
     PATH,
     NO_VALUE,
+    ERROR
 }

community/values/src/main/java/org/neo4j/values/virtual/VirtualValues.java

@@ -164,6 +164,11 @@ public static MapValue map( Map<String,AnyValue> map )
         return new MapValue( map );
     }
 
+    public static ErrorValue error( Exception e )
+    {
+        return new ErrorValue( e );
+    }
+
     @SafeVarargs
     public static MapValue copy( MapValue map, Pair<String,AnyValue>... moreEntries )
     {

enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/ParameterValuesAcceptanceTest.scala

@@ -38,7 +38,7 @@ class ParameterValuesAcceptanceTest extends ExecutionEngineFunSuite with CypherC
 
   // Not TCK material below; sending graph types or characters as parameters is not supported
 
-  test("ANY should be able to use varibels from the horizon") {
+  test("ANY should be able to use variables from the horizon") {
 
     val query =
       """ WITH 1 AS node, [] AS nodes1
@@ -168,5 +168,4 @@ class ParameterValuesAcceptanceTest extends ExecutionEngineFunSuite with CypherC
     val config = Configs.Interpreted - Configs.Cost2_3
     executeWith(config, "EXPLAIN CREATE (n:Person) WITH n MATCH (n:Person {name:{name}}) RETURN n")
   }
-
 }