Azure / azure-cosmos-dotnet-v3

.NET SDK for Azure Cosmos DB for the core SQL API
MIT License
739 stars 494 forks source link

LINQ with Contains and Enum does not consider converterType #1253

Closed SeriousJul closed 9 months ago

SeriousJul commented 4 years ago

Describe the bug azure-cosmos-dotnet-v3 supports LINQ query generation. When working with enum serialized as string in simple where clause, the expression is properly translated in SQL using the string value of the enum. But when using Contains operator to generate "IN" clause. This does not work with array of enum, as the SQL translated from it always contains ordinal values of enum, regardless of the model definition.

To Reproduce

public class Foo
{
        [JsonProperty("status")]
        [JsonConverter(typeof(StringEnumConverter))]
        public FooStatus Status { get; set; }
}

public enum FooStatus
{
        StatusA,
        StatusB
}
var query = container.GetItemLinqQueryable<Foo>();
query = query.Where(foo => new[]{FooStatus.StatusA}.Contains(foo.Status));
Console.WriteLine(query.ToQueryDefinition().QueryText);

Expected behavior

SELECT VALUE root FROM root WHERE (root["status"] IN ("StatusA"))

Actual behavior

SELECT VALUE root FROM root WHERE (root["status"] IN (1))

Environment summary SDK Version: 3.6.0 OS Version (e.g. Windows, Linux, MacOSX):

Operating System: Ubuntu 19.10
Kernel: Linux 5.3.0-40-generic
Architecture: x86-64

Additional context Simple where clause works just as expected

var query = container.GetItemLinqQueryable<Foo>();
query = query.Where(foo => foo.Status == FooStatus.StatusA);
Console.WriteLine(query.ToQueryDefinition().QueryText);
SELECT VALUE root FROM root WHERE (root["status"] = "StatusA")
NeilWBruce commented 3 years ago

Any updates on this one. Also having this issue while trying to trying to filter on the Enums.

j82w commented 3 years ago

@timsander1 can you take a look?

timsander1 commented 3 years ago

Thanks for flagging this! I can confirm that this is a bug. For now, could you try this workaround?

original code: query = query.Where(foo => new[]{​​​​​​FooStatus.StatusA}​​​​​​.Contains(foo.Status)); workaround: query = query.Where(foo => new[]{​​​​​​FooStatus.StatusA.ToString()}​​​​​​.Contains(foo.Status));

NeilWBruce commented 3 years ago

Yep that works. Was trying something similar myself yesterday but was just testing out the performance. All looks good. Thanks!

bhugot commented 3 years ago

We met similar problem with query like query = query.Where(foo => foo.ADictionary[FooStatus.StatusA] == "a value"); Where foo.ADictionary is Dictionary<FooStatus, string>

The query would be o.ADictionary[0] == "a value" instead of o.ADictionary["StatusA"] == "a value"

bhugot commented 3 years ago

I found a workaround but I don't like it it's by using JsonConvert.DefaultSettings, please allow to use CosmosJsonDotNetSerializer so we can share the JsonSerializerSettings

onionhammer commented 2 years ago

This is still an issue. Any update?

@bhugot can you give more detail if you were able to resolve this with the default settings?

ekjuanrejon commented 2 years ago

any update on this bug?

onionhammer commented 2 years ago

@sboshra @j82w I'd be happy to look into this issue and contribute a PR if this is within reach for a new contributor?

onionhammer commented 2 years ago

Looking over the code it looks like this needs to be enhanced to support the serializer settings

https://github.com/Azure/azure-cosmos-dotnet-v3/blob/11b774459a894657dcca8cbf89b945b42e008a86/Microsoft.Azure.Cosmos/src/Linq/ExpressionToSQL.cs#L529

pmieten commented 1 year ago

Are you going to fix it?

jlocans commented 1 year ago

Any updates?

onionhammer commented 1 year ago

@jlocans they reverted the regression at least, but you still have to specify newtonsoft attributes all over.

alice-ep commented 11 months ago

Hi, are there any updates about this?

Maya-Painter commented 9 months ago

Hi all, we've merged what we hope will be a solution to this issue by allowing the custom serializer to be used for LINQ translations. This also unblocks having System.Text.Json attributes applied in LINQ queries.

We plan to release this as part of the next preview package, but I also wanted to take this time to confirm these changes meet your needs before finalizing them as part of our public API. If you try the changes out, please follow up in the thread if this update addresses your concerns. If it doesn't, please let us know why.

To use your own custom serializer for LINQ translations you must implement CosmosLinqSerializer and set it as a custom serializer on the CosmosClient. There is a sample of this for serialization with System.Text.Json in CosmosLinqSerializer.cs.

lukasz-pyrzyk commented 9 months ago

@Maya-Painter thank you for merging the fix. We have tested our code with the nighly build and it fixes our problem with quering againts array of enums marked with JSonConverter(XYZ).

Do you know when we can expect next preview package available on the nuget feed?

Maya-Painter commented 9 months ago

@lukasz-pyrzyk We do not have a release scheduled yet but are hoping to get it out by the end of the month.

cultpodcasts commented 2 months ago

Leaving a link to source mentioned in comment above https://github.com/Azure/azure-cosmos-dotnet-v3/blob/master/Microsoft.Azure.Cosmos.Samples/Usage/SystemTextJson/CosmosSystemTextJsonSerializer.cs