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

CallProcedure deserialization is not working with Read-Only queries #25

Closed
Ivan-Gyoshev opened this issue Jun 6, 2022 · 8 comments · Fixed by #27
Closed

CallProcedure deserialization is not working with Read-Only queries #25

Ivan-Gyoshev opened this issue Jun 6, 2022 · 8 comments · Fixed by #27

Comments

@Ivan-Gyoshev
Copy link

We are using a Redis graph read-only replica and performing read-only queries. In standard scenarios when the result is a specific value like a string or long there aren't any problems, but in the case when we are expecting nodes, the deserialization is calling inside it a 'GRAPH.QUERY' and the read-only replica which is only accepting 'GRAPH_RO.QUERY' is throwing an exception.

The full path to the method is ResultSet>RecordIterator>DeserializeScalar>DeserializeNode>_graphCache.GetLabel>_labels.GetCachedData>GetProcedureInfo>CallProcedure

@tombatron
Copy link
Owner

Thanks for reporting this!

I'll take a look at the issue and your pull request very soon.

@tombatron
Copy link
Owner

@Ivan-Gyoshev It just occurred to me 🤦‍♂️, would you mind trying the method GraphReadOnlyQuery (or GraphReadOnlyQueryAsync)?

The implementation is VERY similar to the pull request that you opened.

@Ivan-Gyoshev
Copy link
Author

Ivan-Gyoshev commented Jun 9, 2022

Hey @tombatron ,

Do you mean using the already implemented method GraphReadOnlyQuery instead of creating the new one(ReadOnlyQuery) which I put in the pull request?

@tombatron
Copy link
Owner

Yes.

Also, could I get some sample data and a sample procedure to replicate for a test.

I'd also like to check Jedis's RedisGraph support for the same issue since my goal here is to essentially replicate the "official" support.

1 similar comment
@tombatron
Copy link
Owner

Yes.

Also, could I get some sample data and a sample procedure to replicate for a test.

I'd also like to check Jedis's RedisGraph support for the same issue since my goal here is to essentially replicate the "official" support.

@Ivan-Gyoshev
Copy link
Author

Ivan-Gyoshev commented Jun 9, 2022

Hello,
I updated my pull request.

Here is a little sample of how you can reproduce this issue:

Connect to your master Redis graph

ConnectionMultiplexer connectionMultiplexer = ConnectionMultiplexer.Connect("localhost:6379,abortConnect=false");
var db = connectionMultiplexer.GetDatabase();
var redisGraph = new RedisGraph(db);

    // Insert data
    string firstPersonQuery = "CREATE (:Person {username: 'John', age: 20})";
    string secondPersonQuery = "CREATE (:Person {username: 'Peter', age: 23})";
    string thirdPersonQuery = "CREATE (:Person {username: 'Steven', age: 30})";

    redisGraph.GraphQuery("demo", firstPersonQuery);
    redisGraph.GraphQuery("demo", secondPersonQuery);
    redisGraph.GraphQuery("demo", thirdPersonQuery);`

After you enter some sample data in your graph, you have to connect to your read-only replica and read this data.
Connect to your Redis graph replica

ConnectionMultiplexer replicaConnection = ConnectionMultiplexer.Connect("localhost:6378,abortConnect=false");
       var dbReplica = replicaConnection.GetDatabase();
       var redisGraphReplica = new RedisGraph(dbReplica);
    // Read from the read-only replica
    string readQuery = "MATCH x=(p:Person) RETURN nodes(x) as nodes";

    ResultSet records = redisGraphReplica.GraphReadOnlyQuery("demo", readQuery);

    List<Person> result = new List<Person>();

    foreach (Record record in records) // Here it throws the exception (On the iteration)
    {
        var nodes = record.GetValue<object[]>("nodes");

        foreach (var node in nodes)
        {
            if (node is Node castedNode)
            {
                Person person = GetPersonInfo(castedNode);
                result.Add(new Person(person.Name, person.Age));
            }
        }
    }

`

As I described above, when you execute the read-only query and have a result, in the process of deserializing the nodes in the result, the StackExchange.Redis driver is throwing an exception because the deserialization process internally is making GRAPH.QUERY call instead of GRAPH_RO.QUERY

tombatron added a commit that referenced this issue Jun 10, 2022
@tombatron
Copy link
Owner

@Ivan-Gyoshev Would you mind trying: https://www.nuget.org/packages/NRedisGraph/1.7.0-rc ?

If that works for you I'll finalize my changes and publish version 1.7.0.

@Ivan-Gyoshev
Copy link
Author

Yes, I tested it now and it seems to work fine. :)

tombatron added a commit that referenced this issue Jun 13, 2022
Expanded support for "read only" queries. 

Closes #25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants