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

Incorrect query when using P.Within() #41

Open
GerbenGaastra opened this issue Nov 12, 2018 · 6 comments
Open

Incorrect query when using P.Within() #41

GerbenGaastra opened this issue Nov 12, 2018 · 6 comments
Labels
bug Something isn't working

Comments

@GerbenGaastra
Copy link

When querying using the method P.Within() there never seem to yield any results. When inspecting the produced query I noticed it has some additional quotes around the array. When removing these quotes I get the expected behavior which are the requested vertexes.

Example code:

var idArray = new int[] {5, 13, 27};
var query = g.V<Vertex>().Has(p => p.IdProperty, P.Within(idArray));


var queryWithTooManyQuotes = query.ToGremlinQuery();
// -> "g.V().hasLabel(\"label"\).has(\"IdProperty\",within(\"[5,13,27]\"))"

var fixedQuery = query
      .ToGremlinQuery()
      .Replace("\"[", "[", StringComparison.InvariantCulture)
      .Replace("]\"", "]", StringComparison.InvariantCulture);
// -> "g.V().hasLabel(\"label"\).has(\"IdProperty\",within([5,13,27]))"

var incorrectlyEmptyResult = await gremlinClient.QueryAsync<Vertex>(queryWithTooManyQuotes);
var correctResult = await gremlinClient.QueryAsync<Vertex>(fixedQuery);
@evo-terren evo-terren added the bug Something isn't working label Nov 13, 2018
@evo-terren
Copy link
Owner

Yep. That's definitely a bug in the serialization logic. I'm guessing it's being treated as an object and is therefore getting the quotes. The code linked by @BenjaBobs would be a good place to start on a fix. I won't be able to get to this any time soon due to the season, unfortunately, but would be happy to look at any pull requests you may have for a fix. Otherwise, I will address this when I can. Thank you for your patience.

@BenjaBobs
Copy link
Collaborator

BenjaBobs commented Nov 15, 2018

I investigated a bit, and this turned out to be more complex than I initially thought.

P.Within(params object[]) behaves in the following way:

[Fact]
public void Within_serializes_properly()
{
    var g = new GraphTraversalSource();

    var priceObjectArray = new object[] { 5, 13, 27 };
    var priceIntArray = new int[] { 5, 13, 27 };

    var queryObject = g.V<Product>().Has(p => p.Price, P.Within(priceObjectArray));
    var queryInt = g.V<Product>().Has(p => p.Price, P.Within(priceIntArray));

    var queryObjectString = queryObject.ToGremlinQuery();
    var queryIntString = queryInt.ToGremlinQuery();

    queryObjectString.Should().Be("g.V().hasLabel(\"product\").has(\"Price\",within(5, 13, 27))");
    queryIntString.Should().Be("g.V().hasLabel(\"product\").has(\"Price\",within(\"[5, 13, 27]\"))");
}

Because of the way the type is inferred and the way params object[] works I'm not exactly sure how to fix this and maintain a nice syntax.

@GerbenGaastra
Copy link
Author

Might be here:
https://github.com/evo-terren/Gremlin.Net.CosmosDb/blob/master/src/Gremlin.Net.CosmosDb/Serialization/GremlinQuerySerializer.cs#L184-L202

When I add an additional overload to the class GremlinQuerySerializer you mentioned the query is parsed correctly.

private void Serialize(IEnumerable<int> array)
{
    var serializedObj = JsonConvert.SerializeObject(array, Formatting.None, _serializerSettings);
    _writer.Write(serializedObj);
}

The replace functionality in private void Serialize(object obj) seem to be not needed for a simple IEnumerable<int>, so this just might work.

@BenjaBobs
Copy link
Collaborator

I'm still not sure this is the best method of solving this, as the underlying data is still formatted wrong and might lead to errors in the future.
The problem is that P.Within(priceObjectArray) produces this data structure:

[
	5,
	13,
	27
]

While P.Within(priceIntArray) produces

[
	[
		5,
		13,
		27
	]
]

Due to the signature of P.Within(params object[]), the int array is interpreted as a single object instead of an array of objects. I worry this might lead to unexpected behavior.

@SamHard
Copy link

SamHard commented Jun 22, 2019

I believe this issue was previously documented as this issue:
#32

Soon after, it was addressed by this commit:
ae3b63f

This test scenario is passing, and it seems to be almost the same as the one from the original post.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants