Cosmos datetime fixed width by mohammedwed · Pull Request #39131 · dotnet/efcore

mohammedwed · 2026-09-30T10:06:46Z

I've read the guidelines for contributing and seen the walkthrough
I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
The code builds and tests pass locally (also verified by our automated build checks)
Commit messages follow this format:

        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber

Tests for the changes have been added (for bug fixes / features)
Code follows the same patterns and style as existing code in this repo

Fixes #39113

- Fall back to Cosmos SDK serialization when parameter values do not match the mapping type - Preserve default SDK handling for parameter values incompatible with configured type mappings Fixes dotnet#39113

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

mohammedwed · 2026-09-30T10:12:35Z

@dotnet-policy-service agree company="veraxity.dev"

Copilot

Copilot review overview

🟡 Changes recommended

Legacy Cosmos documents remain incompatible with new parameters, and several shared JSON assertions are currently broken.

Review effort: Balanced
Findings: 3

Open (3)

Handle legacy DateTime JSON values in queries and ordering · New
Assess cross-provider impact of fixed-width JSON DateTime serialization · New
Remove extra opening brace from expected JSON · New

    public override void ToJsonTyped(Utf8JsonWriter writer, DateTime value)
-        => writer.WriteStringValue(value);
-
+        => writer.WriteStringValue(value.ToString("yyyy-MM-dd'T'HH:mm:ss.fffffffK", System.Globalization.CultureInfo.InvariantCulture));


Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

🔵 Needs a closer look

The shared writer changes unrelated providers and breaks existing SQL Server JSON baselines, while legacy-format compatibility remains untested.

1 open finding

Assess cross-provider impact of fixed-width JSON DateTime serialization

1 resolved since last review

Handle legacy DateTime JSON values in queries and ordering

Previously missed (1)

In code that hasn't changed since last review

Add regression coverage for reading legacy short-form dates

test/EFCore.Cosmos.FunctionalTests/Query/AdHocMiscellaneousQueryCosmosTest.cs:197

This regression test only inserts documents through the new writer, so it does not verify that legacy documents containing the short form (for example, "2026-01-01T00:00:00Z") remain readable. The provider-independent JSON expectations were changed away from that form, leaving the compatibility claim from the review thread uncovered. Seed one document as raw legacy JSON (the existing AdHocCosmosTestHelpers.CreateCustomEntityHelperAsync supports this) and assert it materializes correctly.

🧠 Review effort: Balanced

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

AndriySvyryd · 2026-10-08T20:08:45Z

+        #region 39113
+
+    [Fact]
+    public virtual async Task DateTime_with_and_without_fractional_seconds_orders_and_filters_correctly()


Add a test that inserts datetime using the old format (by using the SDK directly) and then asserts that it's still materialized correctly both in entity instances and in projections

AndriySvyryd · 2026-10-08T20:10:29Z

    }

-    [Theory, InlineData("0001-01-01T00:00:00.0000000", """{"Prop":"0001-01-01T00:00:00"}"""),
+    [Theory, InlineData("0001-01-01T00:00:00.0000000", """{"Prop":"0001-01-01T00:00:00.0000000"}"""),


Also update all the affected SQL Server baselines

AndriySvyryd · 2026-10-08T21:19:17Z

+
+

Extra whitespace

AndriySvyryd · 2026-10-08T22:58:56Z

    public override void ToJsonTyped(Utf8JsonWriter writer, DateTime value)
-        => writer.WriteStringValue(value);
-
+        => writer.WriteStringValue(value.ToString("yyyy-MM-dd'T'HH:mm:ss.fffffffK", System.Globalization.CultureInfo.InvariantCulture));


Allow the format to be specified in a public constructor. This would allow to use previous behavior if necessary.

mohammedwed added 3 commits September 30, 2026 10:52

Cosmos: write DateTime in fixed-width format (dotnet#39113)

62ff983

Add Cosmos test for DateTime ordering and filtering (dotnet#39113)

304e4cd

Cosmos: Fall back to SDK serialization for mismatched parameter types

7439816

- Fall back to Cosmos SDK serialization when parameter values do not match the mapping type - Preserve default SDK handling for parameter values incompatible with configured type mappings Fixes dotnet#39113

Copilot AI balanced review requested due to automatic review settings September 30, 2026 10:06

mohammedwed requested a review from a team as a code owner September 30, 2026 10:06

Copilot AI reviewed Sep 30, 2026

github-actions Bot added the community-contribution label Sep 30, 2026

AndriySvyryd requested a balanced review from Copilot October 3, 2026 02:01

Copilot started reviewing on behalf of AndriySvyryd October 3, 2026 02:02 View session

Copilot AI reviewed Oct 3, 2026

View reviewed changes

Fix InlineData formatting in JsonTypesTestBase

97c634c

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI balanced review requested due to automatic review settings October 4, 2026 16:38

Copilot AI reviewed Oct 4, 2026

AndriySvyryd requested a balanced review from Copilot October 7, 2026 23:41

Copilot started reviewing on behalf of AndriySvyryd October 7, 2026 23:42 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

AndriySvyryd reviewed Oct 8, 2026

View reviewed changes

Comment thread src/EFCore/Storage/Json/JsonDateTimeReaderWriter.cs

Comment on lines +27 to +28

AndriySvyryd Oct 8, 2026

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra whitespace

AndriySvyryd reviewed Oct 8, 2026

View reviewed changes

Navigation Menu

Repository navigation

Cosmos datetime fixed width - #39131

Cosmos datetime fixed width#39131
mohammedwed wants to merge 4 commits into
dotnet:mainfrom
mohammedwed:cosmos-datetime-fixed-width

mohammedwed commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mohammedwed commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

AndriySvyryd Oct 8, 2026

Uh oh!

AndriySvyryd Oct 8, 2026

Uh oh!

AndriySvyryd Oct 8, 2026

Uh oh!

AndriySvyryd Oct 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mohammedwed commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

mohammedwed commented Sep 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

AndriySvyryd Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

AndriySvyryd Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

AndriySvyryd Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

AndriySvyryd Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants