Nodatime 'Instant' sorting issue

[luis-fss]

From what I'm seeing, there seems to be something wrong, please take a look at the image:

[garayx]

Hi
You can use Noda in the index, yes. See here for more details: https://github.com/ryanheath/RavenDB-NodaTime/wiki

Can you share the docs, query that reproduce the wrong results?

[ayende]

RavenDB on the server side has no awareness of NodaTime.
The behavior is controlled via the client side extension, which matches the NodeTime behavior with the RavenDB expectation.
https://codeofmatt.com/date-and-time-in-ravendb/

That said, can you create an export of the database with the screen shot above and attach it here?
That doesn't look right to me.

[luis-fss]

Thanks Ayende, I'll do that soon...

[luis-fss]

Hi @ayende, I wasn't able to return as soon as I would have liked, but I think there is still time, as I managed to create a minimal project that presents the problem.
This is an xunit project with RavenDB.TestDriver with just one test that proves the ordering problem discussed here.
https://github.com/luis-fss/RavenDbAndNodaTimeSortingIssue
I know your time is extremely valuable, but please try to see if any correction or workaround is possible, I will be very grateful.

[luis-fss]

I just set up a GitHub action to run the test: https://github.com/luis-fss/RavenDbAndNodaTimeSortingIssue/actions/runs/8035301531/job/21947780393#step:4:22

And for your convenience here is the complete test:

[Fact]
public async Task RavenDbAndNodaTimeSortingIssueTest()
{
    var pattern = InstantPattern.ExtendedIso;

    // I think the issue is related to milliseconds; note that if the milliseconds have
    // exactly the same number of digits in all Instants, the sorting will be corrected.
    const string instantFormat1 = "2024-10-10T10:10:10.3370257Z";
    const string instantFormat2 = "2024-10-10T10:10:11.554013Z";  // this Instant has less digits than the others and will cause the error!!!
    //const string instantFormat2 = "2024-10-10T10:10:11.5540131Z";  // this Instant has the same quantity of digits than the others and will NOT cause any error!!!
    //const string instantFormat2 = "2024-10-10T10:10:11.5540130Z";  // this Instant has the same quantity of digits than the others, but with a zero at the end, this will cause a parse error NOT related with RavenBD!!!
    const string instantFormat3 = "2024-10-10T10:10:12.9545526Z";

    var person1 = new Person("1", pattern.Parse(instantFormat1).Value);
    var person2 = new Person("2", pattern.Parse(instantFormat2).Value);
    var person3 = new Person("3", pattern.Parse(instantFormat3).Value);

    // linq sorting
    var personExpectedOrderList = new List<Person> { person1, person2, person3 }.OrderBy(person => person.CreatedOn).ToList();

    // Asserting linq sorting just to prove it
    Assert.Equal(instantFormat1, pattern.Format(personExpectedOrderList[0].CreatedOn));
    Assert.Equal(instantFormat2, pattern.Format(personExpectedOrderList[1].CreatedOn));
    Assert.Equal(instantFormat3, pattern.Format(personExpectedOrderList[2].CreatedOn));

    var documentStore = GetDocumentStore();

    using (var storesSession = documentStore.OpenAsyncSession())
    {
        await storesSession.StoreAsync(person1);
        await storesSession.StoreAsync(person2);
        await storesSession.StoreAsync(person3);

        storesSession.Advanced.WaitForIndexesAfterSaveChanges();
        await storesSession.SaveChangesAsync();
    }

    List<Person> ravenOrderedList;

    using (var querySession = documentStore.OpenAsyncSession())
    {
        ravenOrderedList = await querySession.Query<Person>().OrderBy(person => person.CreatedOn).ToListAsync();
    }

    WaitForUserToContinueTheTest(documentStore);

    Assert.Equal(instantFormat1, pattern.Format(ravenOrderedList[0].CreatedOn)); // fails here!!!
    Assert.Equal(instantFormat2, pattern.Format(ravenOrderedList[1].CreatedOn));
    Assert.Equal(instantFormat3, pattern.Format(ravenOrderedList[2].CreatedOn));
}

[arekpalinski]

@luis-fss thanks for the test.

@maciejaszyk can you please have a look here?

[maciejaszyk]

Hey, thanks for test.
I'm not very familiar with NodaTime API but your problematic date (2024-10-10T10:10:11.554013Z) is not ISO 8601 format. https://learn.microsoft.com/en-us/dotnet/standard/base-types/standard-date-and-time-format-strings#Roundtrip

As @ayende said before, RavenDB is not aware about NodaTime format, thus your date is changed into standard string/term, rest of them are ISO compliant so will be changed into Ticks by indexing engine and your query (since order by has implicit type; no as string / as long etc) and that means it will use Ticks since your field has them.

You can solve it by:

1. Transform your data from Instant into DateTime (or other) in index via AdditionalSources, see examples here: https://demo.ravendb.net/demos/csharp/static-indexes/additional-sources-index

2. Write your custom JsonConverter that will transform Instant to DateTime (and vice-versa) during serialization/deserialization. I've found some example in our tests base:
   https://github.com/ravendb/ravendb/blob/v6.0/test/SlowTests/Issues/RavenDB-11750.cs

[luis-fss]

Hello @maciejaszyk, first of all I want to thank you for the information, it was very useful and helped me find a solution to the problem, especially regarding the implementation of JsonConverter.
Therefore, I will mark this question as answered, but leave it open as I would also like to leave some considerations that may or may not help to resolve the problem in a more general way:

"your problematic date (2024-10-10T10:10:11.554013Z) is not ISO 8601 format"

Right, but see that this value was taken from a RavenDB database, that is, RavenDB itself converted an object of type NodaTime.Instant which, not by chance, had a value with all the necessary characteristics to be converted to a string in ISO 8601 format.

So what exactly is the API that RavenDB uses internally to convert a NodaTime.Instant into an ISO 8601 format before storing it in the database?

It is important to say that NodaTime.Text.InstantPattern.ExtendedIso presents the following documentation:

"Gets an invariant instant pattern which is ISO-8601 compatible, providing up to 9 decimal places of sub-second accuracy. (These digits are omitted when unnecessary.) This corresponds to the text pattern "uuuu'-'MM'-'dd'T'HH':'mm':'ss;FFFFFFFFF'Z'"."

The excerpt "These digits are omitted when unnecessary" caught my attention. This may be concerning if RavenDB is using the ExtendedIso.Format method of this pattern.

For reference, it was with this JsonConverter that I solved my problem:

private class InstantConverter : JsonConverter<Instant>
{
    private static readonly InstantPattern FormatPattern = InstantPattern.CreateWithInvariantCulture("yyyy-MM-ddTHH:mm:ss.fffffff'Z'");

    public override Instant ReadJson(JsonReader reader, Type type, Instant existingValue, bool hasExistingValue, JsonSerializer serializer)
    {
        var timestamp = serializer.Deserialize<string>(reader);
        // here the ExtendedIso Pattern can be used without problems
        return InstantPattern.ExtendedIso.Parse(timestamp!).Value;
    }

    public override void WriteJson(JsonWriter writer, Instant instant, JsonSerializer serializer)
    {
        var timestamp = FormatPattern.Format(instant);
        // here we need a stricter Pattern in order to guarantee the expected precision
        serializer.Serialize(writer, timestamp);
    }
}

[maciejaszyk]

Hey,
for C# types we've our custom converters provided for NewtonsoftJson. In your case, since we never provided any custom converters for NodaTime the lib will use default one, I debugged through the serialization process of NewtonsoftJson and I see that InstantPatternParser.LocalDateTimePatternAdapter was called during the serialization.