Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for ValueType VALUE_POINT. #45

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mitchsw
Copy link

@mitchsw mitchsw commented May 11, 2021

Currently, returning a point fails to parse. RedisGraph returns a point as a 2-element double array (see _ResultSet_CompactReplyWithPoint). In this client implementation, I parse those float strings into a 2-element map[string]float64.

Currently, returning a point fails to parse. RedisGraph implements this value type as a 2-element array (see _ResultSet_CompactReplyWithPoint). In this client implementation, I parse that into a 2-element map[string]float64.
@mitchsw
Copy link
Author

mitchsw commented May 28, 2021

Please take a look @filipecosta90

@filipecosta90 filipecosta90 added the enhancement New feature or request label Jun 11, 2021
func (qr *QueryResult) parsePoint(cell interface{}) map[string]float64 {
point := make(map[string]float64)
var array = cell.([]interface{})
if len(array) == 2 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition should always be true, but there is no harm in checking for it.

jeffreylovitz
jeffreylovitz previously approved these changes Jun 11, 2021
Copy link
Contributor

@jeffreylovitz jeffreylovitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thank you for the contribution!

@@ -45,6 +45,7 @@ const (
VALUE_NODE
VALUE_PATH
VALUE_MAP
VALUE_POINT
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use ALL_CAPS in Go names; use CamelCase

@jeffreylovitz
Copy link
Contributor

@mitchsw Sorry for the delay, we had a hiccup with our CI testing!

Would you mind making a new commit through a command like git amend and pushing?

That should re-trigger our CI and allow us to merge this feature.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants