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

HasValue LINQ translated as IS_DEFINED instead of IS_DEFINED AND NOT IS_NULL #2248

Closed
Ettores88 opened this issue Feb 23, 2021 · 10 comments
Closed

Comments

@Ettores88
Copy link

// convert .HasValue to IS_DEFINED expression
if (memberName == "HasValue")
{
return SqlFunctionCallScalarExpression.CreateBuiltin("IS_DEFINED", memberExpression);
}

Shouldn't be translated as "IS_DEFINED() AND NOT IS_NULL()" ?
The behaviour in code change if I use the "NullValueHandling = NullValueHandling.Ignore" or the "NullValueHandling = NullValueHandling.Include" but from an object point of view in the end it is still null and doesn't have a value.

@j82w
Copy link
Contributor

j82w commented Feb 23, 2021

@timsander1 or @sboshra can you take a look?

@timsander1
Copy link
Contributor

This is the correct behavior. Null is a valid JSON value.

@Ettores88
Copy link
Author

Ettores88 commented Feb 23, 2021

Sorry @timsander1 but I haven't understood.

Let's make an example. Given the following Class for documents stored in Cosmos:

    public class Foo
    {
        [JsonProperty("id")]
        public string Id { get; set; }

        [JsonProperty("partitionKey")]
        public string PartitionKey { get; set; }

        [JsonProperty("fooData", NullValueHandling = NullValueHandling.Ignore)]
        public Guid? FooData { get; set; }
    }

In this case the following LINQ query will get all the data where FooData is null:

_container.GetItemLinqQueryable<Foo>()
                .Where(foo => !foo.FooData.HasValue)
                .ToFeedIterator().ReadNextAsync();

This LINQ will be translated as "SELECT * FROM c WHERE NOT IS_DEFINED(c.fooData)" and this will match all my documents because them are translated as:

{
    "id" : "fooId",
    "partitionKey": "fooPartition"
}

But if I changed the serialization mode into:

    public class Foo
    {
        [JsonProperty("id")]
        public string Id { get; set; }

        [JsonProperty("partitionKey")]
        public string PartitionKey { get; set; }

        [JsonProperty("fooData", NullValueHandling = NullValueHandling.Include)]
        public Guid? FooData { get; set; }
    }

the same LINQ query will not work because the JSON document will be serialiez as:

{
    "id" : "fooId",
    "partitionKey": "fooPartition",
    "fooData" : null
}

Is this the desired behavior? Should I change the LINQ query based on how I serialize the data?

_container.GetItemLinqQueryable<Foo>()
                .Where(foo => foo.FooData == null)
                .ToFeedIterator().ReadNextAsync();

@timsander1
Copy link
Contributor

That makes sense and this is the desired behavior (not a bug). If you want null values to be omitted from the item, can you set NullValueHandling.Ignore, like you mentioned? Is that reasonable?

@Ettores88
Copy link
Author

My point is different.

My app created a series of documents (in production) with a serialized property that in some cases could have NULL value. In a new version of the same application I decide to change the serialization of that property and add the decorator NullValueHandling.Ignore. Am I now responsible to remember that the meaning of HasValue for the Cosmos SDK doesn't manage the defined null property?.

So from now on I should write a query like the following one to manage both cases:

_container.GetItemLinqQueryable<Foo>()
                .Where(foo => !foo.FooData.HasValue || foo.FooData == null)
                .ToFeedIterator().ReadNextAsync();

That where condition is a no-sense in C# code.

@timsander1
Copy link
Contributor

Null and undefined are different in JSON. This is an attribute of JSON, more than Cosmos DB specifically. Like you mentioned, this is important to remember. Here's a doc that walks through the differences between null and undefined in queries: https://devblogs.microsoft.com/cosmosdb/difference-between-null-and-undefined/

A property can be defined and have a null value. In this case, it has a value (the value is null). LINQ in Cosmos DB is consistent with this definition. If you want null values to be ignored, setting NullValueHandling.Ignore is a good solution.

@Ettores88
Copy link
Author

At Cosmos level it's fine, I thinked about this possibility. But at SDK level it is a strange behaviour.

@j82w
Copy link
Contributor

j82w commented Feb 24, 2021

The issue is c# doesn't have a concept of undefined like JSON does. Using the linq extension might help with readability.

var iter = _container.GetItemLinqQueryable<Foo>()
                .Where(foo => !foo.FooData.IsNull() && !foo.FooData.IsDefined())
                .ToFeedIterator();
while(iter.HasMoreResults)
{
    var result = await iter.ReadNextAsync();
}

@Ettores88
Copy link
Author

Yes, this will definitely help. But 'HasValue' is supported from the SDK and its translation is misleading in this specific case.

@timsander1
Copy link
Contributor

Hi @Ettores88, I agree with that! The mapping of C# -> JSON concepts is confusing here. This great feedback to have when we plan the next major SDK release and can make significant changes.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants