florinpatrascu / bolt_sips

Neo4j driver for Elixir
Apache License 2.0
256 stars 49 forks source link

Parameters not working? #32

Closed cheerfulstoic closed 6 years ago

cheerfulstoic commented 6 years ago

I'm trying to use parameters to create nodes and not having much luck. I started out with:

Neo4j.query!(Neo4j.conn, "CREATE (report:Report) SET report = {props}", %{props: my_map})

That gives me:

** (Bolt.Sips.Exception) socket is not connected
    (bolt_sips) lib/bolt_sips/query.ex:57: Bolt.Sips.Query.query!/3
    lib/mix/tasks/test.load_db.ex:32: anonymous fn/2 in Mix.Tasks.Test.LoadDb.run/1
    (elixir) lib/enum.ex:1841: Enum.reduce_range_inc/4
    lib/mix/tasks/test.load_db.ex:18: Mix.Tasks.Test.LoadDb.run/1
    (mix) lib/mix/task.ex:301: Mix.Task.run_task/3
    (mix) lib/mix/cli.ex:75: Mix.CLI.run_task/2

To simplify I tried this:

Neo4j.query!(Neo4j.conn, "CREATE (report:Report) SET report.test = {props}", %{props: 'foo'})

That successfully queries, but then the test property has the value "102,111,111" in the database.

This works, however:

Neo4j.query!(Neo4j.conn, "CREATE (report:Report) SET report.test = 'foo'")

So it seems like maybe there's just something wrong with params (?)

cheerfulstoic commented 6 years ago

For background:

 $ elixir -v
Erlang/OTP 20 [erts-9.0] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:10] [hipe] [kernel-poll:false] [dtrace]

Elixir 1.5.1

And in my mix.lock, neo4j_sips is version 0.3.2 and boltex is 0.3.0

florinpatrascu commented 6 years ago

hi there - I'm pretty sure the params support is working. Are you trying to use the mix task? If yes, I am not sure it can be used for something else than simple queries, as it was scoped as a convenience only. I'll have a look though. And, regarding the query you show in the comment above, you should probably try using a String value instead? Above you have a charlist ie 'foo'? ;P Try this:

Neo4j.query!(Neo4j.conn, "CREATE (report:Report) SET report.test = {props}", %{props: "foo"})
florinpatrascu commented 6 years ago

Wait, are you referring to neo4j_sips? neo4j_sips is not using Bolt; it's using the REST API, over http.

florinpatrascu commented 6 years ago

plus socket is not connected, in your initial report, seems to be a network issue, i.e. neo4j server port configuration, mix config, etc?

dnesteryuk commented 6 years ago

@cheerfulstoic please, check that the Bolt.Sips is started before you try to query, example, https://github.com/sirko-io/engine/blob/master/lib/sirko.ex#L10. It should solve your first problem with the socket. The second problem should be solved by @florinpatrascu 's suggestion about strings.

cheerfulstoic commented 6 years ago

Ah, sorry for the confusion. This is actually Bolt.Sips, but I aliased that to Neo4j because I saw it in this task:

https://github.com/florinpatrascu/bolt_sips/blob/master/lib/mix/tasks/cypher.ex#L30

Though I'm not using the built-in task, I'm using my own custom mix task that I've created.

I don't think it's a network issue because I do a Neo4j.query!(Neo4j.conn, "MATCH (n) DETACH DELETE n") at the start of the task which works fine. And again, if I change it to a simple query without params it works fine.

Good catch about 'foo'. Absolutely changing it to "foo" fixed that, but I still can't set all of the properties at once by passing in a map as the parameter to Neo4j. Could it be something where the bolt driver doesn't yet support passing in maps as parameters?

florinpatrascu commented 6 years ago

We actually have tests covering this, please see link. Excerpt:

test "executing a Cpyher query, with map parameters", context do
    conn = context[:conn]

    cypher = """
      CREATE(n:User {props})
    """

    assert {:ok, _} = Bolt.Sips.query(conn, cypher, %{props: %{name: "Mep", bolt_sips: true}})
  end
florinpatrascu commented 6 years ago

hmmm, I believe I understand what is going on. Boltex is throwing an exception for nested maps. Will try to look into it later as I am caught into a project for work, unless @mschae or @dnesteryuk have a quicker answer?! :) an anticipated thanks guys! Basically this:

# in boltex repo, run MIX_ENV=test iex -S mix
uri = Boltex.IntegrationCase.neo4j_uri
Boltex.test uri.host, uri.port, "CREATE (report:Report) SET report = {props}", %{props: %{foo: "bar", another_map: %{unu: 1, doi: 2}}}, uri.userinfo

%Boltex.Error{code: "Neo.ClientError.Statement.TypeError", connection_id: 43264,
 function: :run_statement,
 message: "Property values can only be of primitive types or arrays thereof",
 type: :cypher_error}

will also try to see if we can throw better errors from our end, but we pretty much echo back boltex' error messages.

Good catch!

florinpatrascu commented 6 years ago

for brevity, 1st test will pass, the 2nd one will fail (in Bolt.Sips)

defmodule NodesAndProperties.Test do
  use ExUnit.Case, async: true
  alias Bolt.Sips, as: Neo4j

  @simple_map %{foo: "bar", bolt_sips: true}
  @nested_map %{foo: "bar", bolt_sips: true, a_map: %{unu: 1, doi: 2, baz: "foo"}, a_list: [1, 2, 3.14]}

  test "create a node using SET properties and a simple map" do
   r =  Neo4j.query!(Neo4j.conn, "CREATE (report:Report) SET report = {props}", %{props: @simple_map})
   assert r = %{stats: %{"labels-added" => 1, "nodes-created" => 1, "properties-set" => 2}, type: "w"}
  end

  test "create a node using SET properties and a nested map" do
   r =  Neo4j.query!(Neo4j.conn, "CREATE (report:Report) SET report = {props}", %{props: @nested_map})
   assert r = %{stats: %{"labels-added" => 1, "nodes-created" => 1, "properties-set" => 2}, type: "w"}
  end
end

leaving this here as a bookmark, for later

mschae commented 6 years ago

Hey,

You're right @florinpatrascu that's a Neo4J error and not much we can do about it.

Neo4J doesn't allow for nested maps. Neither via bolt nor via REST. It's a limitation to Neo4J. In our projects I've resorted to encoding and decoding nested maps to json.

I quite like the error message actually. :)

florinpatrascu commented 6 years ago

Thank you, Michael!!!

I quite like the error message actually.

me too :)

And you also saved me for some pain, as I was planning later on to check the REST support for the nested maps. Thanks again!

@cheerfulstoic - is there something else we could help you with, for this issue? Thank you for your report though!

florinpatrascu commented 6 years ago

for reference: https://stackoverflow.com/questions/25750016/nested-maps-and-collections-in-neo4j-2#25754628

One comment:

There is a way to do this by converting nested maps to json string

exactly as you said, @mschae

cheerfulstoic commented 6 years ago

I don't actually have a nested map, actually. It's just a plain map. I can't share the example I'm using, but I'll try to put together a version which still breaks

florinpatrascu commented 6 years ago

understood, ty

cheerfulstoic commented 6 years ago

Oh, gosh. I figured it out and it's my Ruby habits again:

Neo4j.query!(Neo4j.conn, "CREATE (:Report {props})", %{props: %{'foo' => "test" }})

It's because of the single quotes for the key in the map being a charlist. So I'm able to fix my issue. I don't know if there's a way to validate values going in so that users get a better error message than "socket is not connected". I'll let you decide if you want to tackle that ;)

Thanks!

florinpatrascu commented 6 years ago

no worries, we all went through this :)

I'll close this one, and create a new issue for improving the error support. I have an idea about why the "socket is not connected". We're retrying the requests in case of errors, but there are some specific errors we shouldn't bother the server with, such as the one you encountered, encoding errors, and so on. I believe the "socket is not connected", is actually an artifact of trying a request with errors and for some reasons the connection is lost in "translation". I'll look into that this week-end. Thanks again!