RedisGraph / redisgraph-go

A Golang client for redisgraph
https://redisgraph.io
BSD 3-Clause "New" or "Revised" License
132 stars 38 forks source link

No way to access query results #15

Closed ChrisSandison closed 4 years ago

ChrisSandison commented 5 years ago

There's no way to query a stored graph anymore. Constructing a new graph and committing it works but if I'm accessing a stored graph I can't get at the results of my queries.

From looking at it, they are private in query_results.go and not accessible outside of the package.

I'm happy to jump on this as a contributor and make a PR

swilly22 commented 5 years ago

@ChrisSandison can you please provide an example snippet?

ChrisSandison commented 5 years ago

Hey @swilly22 sorry for the delay.

We had been upgrading from from v1.0.0 version running on redisgraph v1.22 to the latest builds running on v1.99.2 and having some issues getting them to play.

I've made a derivative repo that just supports the querying interface and added some typing control to the results. Unfortunately I wasn't able to import this pkg and access any of the query results.

https://github.com/thinkdata-works/redisgraph-go-query

Please let me know if the correct license has been applied, I'm very interested in collaborating on this repo and getting any relevant changes merged.

swilly22 commented 5 years ago

@ChrisSandison,

having some issues getting them to play.

What's the issue? I've reviewed redisgraph-go-query the first thing which I find missing was the compact flag whenever issuing a query.

Can you elaborate on how redisgraph-go-query complements redisgraph-go?

ChrisSandison commented 4 years ago

Sure thing. The QueryResult and QueryResultHeader structs do not expose their fields outside of the package. Currently, the only thing available when pulling the lib into a project is getting at the stats and printing the results to stdout. I have a PR that will address these issues directly.

The primary changes involve:

type QueryResultHeader struct {
    ColumnNames []string
    ColumnTypes []ResultSetColumnTypes
}

// QueryResult represents the results of a query.
type QueryResult struct {
    Results    [][]interface{}
    Statistics map[string]float64
    Header     QueryResultHeader
    graph      *Graph
}

Thank you for the feedback on the repo, I'm looking at adding the --compact flag to the command right now. I was hoping to add some functionality to the results set that gets returned from the query, and providing type conversion over the interface type that gets returned from redigo.

swilly22 commented 4 years ago

@ChrisSandison, you're absolutely right! I wonder if exposing: ColumnNames, ColumnTypes, Results, Statistics and Header directly is the right way to go... as this will bind internals, I think it would be better if we introduce functions which will allow a client to consume the result-set. thoughts?

ChrisSandison commented 4 years ago

@swilly22 I agree. In my personal repo, I had a type wrapping the raw cells that allowed the user to convert them on the fly -- wondering if this is an approach that you folks would be in favour of.

In terms of exposing the struct, would the getter functions of these just be creating copies of the values so that they aren't tied to the original result set?

I would find a lot of functionality around retrieving cells by their header name useful, but that could be added in later.

swilly22 commented 4 years ago

I would get some inspiration from Neo4J golang client(s) generally I would expect an iterator based way of consuming a result-set.