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

SDK modifies values passed to custom JsonConverters in LINQ queries #3222

Closed
ccurrens opened this issue May 26, 2022 · 1 comment · Fixed by #3224
Closed

SDK modifies values passed to custom JsonConverters in LINQ queries #3222

ccurrens opened this issue May 26, 2022 · 1 comment · Fixed by #3224

Comments

@ccurrens
Copy link
Contributor

Describe the bug
A UTC DateTime is converted to a local DateTime before being passed to custom JsonConverters inside of ExpressionToSql.ApplyCustomConverters.

To Reproduce

using Microsoft.Azure.Cosmos;
using Newtonsoft.Json;
using Newtonsoft.Json.Converters;

const string account = "https://localhost:8081";
const string key = "C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==";

using var client = new CosmosClient(account, key);
var container = client.GetContainer("NotEven", "MakingARequest");

Console.WriteLine("******* Incorrect *******");
var now = DateTime.UtcNow.Date;
Console.WriteLine("Date: " + now);
Console.WriteLine("DateTimeKind: " + now.Kind);
Console.WriteLine("Query: " + container.GetItemLinqQueryable<MyDocument>() .Where(q => q.StartDate <= now));

Console.WriteLine();

Console.WriteLine("******* Correct *******");
now = DateTime.Now.Date;
Console.WriteLine("Date: " + now);
Console.WriteLine("DateTimeKind: " + now.Kind);
Console.WriteLine("Query: " + container.GetItemLinqQueryable<MyDocument>() .Where(q => q.StartDate <= now));

public class DateOnlyConverter : IsoDateTimeConverter
{
    public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
    {
        string text = null;
        if (value is DateTime dateTime)
        {
            text = dateTime.ToString("yyyy-MM-dd");
        }
        writer.WriteValue(text);
    }
}

class MyDocument
{
    [JsonProperty("id")] public string Id { get; set; }
    [JsonConverter(typeof(DateOnlyConverter))] public DateTime StartDate { get; set; }
}

Output:

******* Incorrect *******
Date: 5/26/2022 12:00:00 AM
DateTimeKind: Utc
Query: {"query":"SELECT VALUE root FROM root WHERE (root[\"StartDate\"] <= \"2022-05-25\")"}

******* Correct *******
Date: 5/26/2022 12:00:00 AM
DateTimeKind: Local
Query: {"query":"SELECT VALUE root FROM root WHERE (root[\"StartDate\"] <= \"2022-05-26\")"}

It works as expected for local DateTime, but fails for UTC.

Expected behavior
The same value passed into the query is passed into the custom JsonConverter.

Actual behavior
A UTC DateTime is converted to Local DateTime before being passed into the custom JsonConverter.

Environment summary
SDK Version: 3.27.1 (and earlier)
OS Version (e.g. Windows, Linux, MacOSX): All

Additional context
This seems to be a side-effect of ExpressionToSql.VisitConstant converting the value from it's .NET type to a string representation. Because of that, the string value must be parsed before passing to the JsonConverter. It seems that it would be better to pass the value directly to the JsonConverter inside of VisitConstant, but that seems difficult to do since it would require including additional context (e.g., what property it is being used with to determine whether or not a custom converter is specified).

image

@ccurrens
Copy link
Contributor Author

Actually, I think this could be solved if DateTime.Parse requested DateTimeStyles.RoundtripKind:

value = DateTime.Parse(serializedDateTime.Value, null, System.Globalization.DateTimeStyles.RoundtripKind)

Since DateTimeStyles.RoundtripKind is meant to preserve kind when it was serialized with "o" or "r" formatters. Presumably this always happens with JsonConvert.SerializeObject(someDate):

return CosmosElement.Parse(JsonConvert.SerializeObject(inputExpression.Value)).Accept(CosmosElementToSqlScalarExpressionVisitor.Singleton);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants