Better error message for calling procedures

When the static types are wrong we should provide a nice error message.
neo4j · Feb 11, 2016 · 9543296 · 9543296
1 parent 7ef4d25
commit 9543296
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 8 deletions.
diff --git a/...internal/compiler/v3_0/executionplan/procs/DelegatingProcedureExecutablePlanBuilder.scala b/...internal/compiler/v3_0/executionplan/procs/DelegatingProcedureExecutablePlanBuilder.scala
@@ -20,7 +20,7 @@
 package org.neo4j.cypher.internal.compiler.v3_0.executionplan.procs
 
 import org.neo4j.cypher.internal.compiler.v3_0.executionplan.{ExecutablePlanBuilder, ExecutionPlan, PlanFingerprint, PlanFingerprintReference, SCHEMA_WRITE}
-import org.neo4j.cypher.internal.compiler.v3_0.spi.{FieldSignature, PlanContext, ProcedureName, QueryContext}
+import org.neo4j.cypher.internal.compiler.v3_0.spi.{FieldSignature, PlanContext, ProcedureName, ProcedureSignature, QueryContext}
 import org.neo4j.cypher.internal.compiler.v3_0.{CompilationPhaseTracer, PreparedQuery}
 import org.neo4j.cypher.internal.frontend.v3_0.ast._
 import org.neo4j.cypher.internal.frontend.v3_0.symbols.TypeSpec
@@ -51,7 +51,7 @@ case class DelegatingProcedureExecutablePlanBuilder(delegate: ExecutablePlanBuil
           }
           //type check arguments
           args.zip(signature.inputSignature).foreach {
-            case (arg, field) => typeCheck(inputQuery.semanticTable)(arg, field)
+            case (arg, field) => typeCheck(inputQuery.semanticTable)(arg, field, signature)
           }
         }
 
@@ -115,13 +115,18 @@ case class DelegatingProcedureExecutablePlanBuilder(delegate: ExecutablePlanBuil
   private def typeProp(ctx: QueryContext)(relType: RelTypeName, prop: PropertyKeyName) =
     (ctx.getOrCreateRelTypeId(relType.name), ctx.getOrCreatePropertyKeyId(prop.name))
 
-  private def typeCheck(semanticTable: SemanticTable)(exp: Expression, field: FieldSignature) = {
+  private def typeCheck(semanticTable: SemanticTable)(exp: Expression, field: FieldSignature, proc: ProcedureSignature) = {
     val actual = semanticTable.types(exp).actual
     val expected = field.typ
     val intersected = actual intersectOrCoerce expected.covariant
     if (intersected == TypeSpec.none)
       throw new CypherTypeException(
-        s"""${field.name} expects $expected, but got ${actual.mkString(",", "or")}""")
+        s"""Parameter `${field.name}` for procedure `${proc.name}`
+            |expects value of type ${semanticTable.types(exp).actual.toShortString} but got value of type ${field.typ}.
+            |
+        |Usage: CALL ${proc.name}(${proc.inputSignature.map(s => s"<${s.name}>").mkString(", ")})
+            |${proc.inputSignature.map(s => s"    ${s.name} (type ${s.typ})").mkString("Parameters:\n", "\n","")}
+        """.stripMargin)
   }
 }
 
diff --git a/...r-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/spi/ProcedureSignature.scala b/...r-3.0/src/main/scala/org/neo4j/cypher/internal/compiler/v3_0/spi/ProcedureSignature.scala
@@ -58,6 +58,8 @@ case class ProcedureSignature(name: ProcedureName,
                               outputSignature: Seq[FieldSignature],
                               mode: ProcedureCallMode = LazyReadOnlyCallMode)
 
-case class ProcedureName(namespace: Seq[String], name: String)
+case class ProcedureName(namespace: Seq[String], name: String) {
+  override def toString = s"""${namespace.mkString(".")}.$name"""
+}
 
 case class FieldSignature(name: String, typ: CypherType)
diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ProceduresTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ProceduresTest.java
@@ -90,7 +90,7 @@ public void shouldCallRegisteredProcedure() throws Throwable
     }
 
     @Test
-    public void shouldNotAllowCallingNonExistantProcedure() throws Throwable
+    public void shouldNotAllowCallingNonExistingProcedure() throws Throwable
     {
         // Expect
         exception.expect( ProcedureException.class );
@@ -105,7 +105,7 @@ public void shouldNotAllowCallingNonExistantProcedure() throws Throwable
     }
 
     @Test
-    public void shouldNotAllowRegisteringConflicingName() throws Throwable
+    public void shouldNotAllowRegisteringConflictingName() throws Throwable
     {
         // Given
         procs.register( procedure );
@@ -145,7 +145,7 @@ public void shouldNotAllowDuplicateFieldNamesInOutput() throws Throwable
     }
 
     @Test
-    public void shouldSignalNonExistantProcedure() throws Throwable
+    public void shouldSignalNonExistingProcedure() throws Throwable
     {
         // Expect
         exception.expect( ProcedureException.class );

diff --git a/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java b/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java
@@ -80,6 +80,24 @@ public void shouldCallProcedureWithParameterMap() throws Throwable
         }
     }
 
+    @Test
+    public void shouldGiveNiceErrorMessageOnWrongStaticType() throws Throwable
+    {
+        //Expect
+        exception.expect( QueryExecutionException.class );
+        exception.expectMessage(
+                "Parameter `name` for procedure `org.neo4j.procedure.simpleArgument`\n" +
+                "expects value of type String but got value of type Integer.\n\n" +
+                "Usage: CALL org.neo4j.procedure.simpleArgument(<name>)\n" +
+                "Parameters:\n" +
+                "    name (type Integer)"  );
+        // When
+        try ( Transaction ignore = db.beginTx() )
+        {
+            Result res = db.execute( "CALL org.neo4j.procedure.simpleArgument('42')");
+        }
+    }
+
     @Test
     public void shouldCallProcedureWithGenericArgument() throws Throwable
     {