go-cq / cq

neo4j cypher library for database/sql in go
MIT License
133 stars 11 forks source link

Interface conversion error when scanning node with only string properties #23

Closed shiftyp closed 8 years ago

shiftyp commented 8 years ago

I have a query that returns three nodes as values in a single row, and in scanning the nodes I get this error:

2015/12/05 04:55:22 http: panic serving 127.0.0.1:54129: interface conversion: interface {} is map[string]string, not map[string]types.CypherValue
goroutine 11 [running]:
net/http.(*conn).serve.func1(0xc8200ac370, 0x7f1eeebdb810, 0xc8200300f0)
    /usr/local/go/src/net/http/server.go:1287 +0xb5
gopkg.in/cq.v1/types.(*Node).Scan(0xc8200d2d80, 0x6ede60, 0xc8200d2870, 0x0, 0x0)
    /mnt/storage/code/go/src/gopkg.in/cq.v1/types/node.go:35 +0x754
database/sql.convertAssign(0x77fee0, 0xc8200d2d80, 0x6ede60, 0xc8200d2870, 0x0, 0x0)
    /usr/local/go/src/database/sql/convert.go:190 +0x4b1
database/sql.(*Rows).Scan(0xc8200ec420, 0xc820119690, 0x3, 0x3, 0x0, 0x0)
    /usr/local/go/src/database/sql/sql.go:1692 +0x47a
database/sql.(*Row).Scan(0xc820120540, 0xc820119690, 0x3, 0x3, 0x0, 0x0)
    /usr/local/go/src/database/sql/sql.go:1763 +0x382

The query looks a bit like this

MATCH (b:One)-[r1:rel]-(a:Two)-[r2:rel]-(c:One)
WHERE a.id = 'some_id' AND r1.type = 1 AND r2.type = 2
RETURN a, b, c

It returns a single row, but when I run it as a single row query I get the above error when scanning the nodes. So something like this:

var a types.Node
var b types.Node
var c types.Node

row := stmt.QueryRow(id)

if err := row.Scan(&a, &b, &c); err != nil {
    log.Fatal(err)
}

If I execute the statement as a multi row query and scan the first row I don't get the error.

shiftyp commented 8 years ago

I'm also getting the same error when running a multi-row query when more than one row is returned.

freeeve commented 8 years ago

Thanks for the report!

shiftyp commented 8 years ago

So I was able to reproduce the bug in the tests by removing the integer property from the node in TestQueryNode. The issue occurs when the node has only string properties, where the properties end up as a map[string]string instead of a map[string]CypherValue.

shiftyp commented 8 years ago

It's sort of a tricky issue since you can't really change the way the CypherValue unmarshalls. I was able to implement a fix in the node's Scan function by trying to convert the properties to a map[string]CypherValue, and if that fails convert it to a map[string]string and create the map of CypherValue's manually. Like so:

properties, ok := inner.Val.(map[string]CypherValue)
if ok {
    n.Properties = properties
} else {
    n.Properties = map[string]CypherValue{}

    properties := inner.Val.(map[string]string)

    for k, v := range properties {
        n.Properties[k] = CypherValue{CypherString, v}
    }
}

If that sounds like a good solution to you I can put in a pr for your review.

freeeve commented 8 years ago

Yeah, PR is great. I started doing this last night but fell asleep on the sofa. :P

freeeve commented 8 years ago

The good news is the typed-ness of bolt is going to solve a lot of this in v2.

freeeve commented 8 years ago

I don't often return nodes/rels as full entities, usually pull data into structs in go so I list out the fields. Wasn't sufficient testing for nodes/rels. (I imagine rels have the same problem.)

shiftyp commented 8 years ago

I'll check out the rels then as well and include a fix in the PR if they have the same issue.

shiftyp commented 8 years ago

For my application I'm returning the nodes and creating structs that can populate themselves from Nodes. Some of the nodes have arbitrary properties, so I can't easily list out all of them individually in the query.

freeeve commented 8 years ago

Merged PR.