Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 2, 2025

Fixes #37275

Description
EF Core 10 added support for json SQL Server type for JSON column and it's used by default if the target SQL Server version supports it, but no migrations to convert the columns to the new json column type are created because the model snapshot doesn't capture the default store type for JSON columns.
For the complete fix #37294 is required for the user to create a migration in 9 that has the explicit type.

Customer impact
Customers upgrading from EF Core 9 to 10 who use ToJson() won't get migrations generated to update the column type from nvarchar(max) to json. The workaround is to manually specify .HasColumnType("json") on the JSON column.

How found
Customer reported on EF 10.0.0

Regression
No, new feature.

Testing
Test added

Risk
Low. Targeted fix to include the default column type in the model snapshot. Only affects design-time.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix model differ test for migrating JSON columns Add model differ tests for JSON column migration from nvarchar(max) to json Dec 2, 2025
Copilot AI requested a review from AndriySvyryd December 2, 2025 22:09
Copilot finished work on behalf of AndriySvyryd December 2, 2025 22:09
- Modified AnnotationCodeGenerator to generate column type for owned
  and complex types mapped to JSON even when not explicitly set
- Updated Owned_types_mapped_to_json_are_stored_in_snapshot to expect
  HasColumnType("nvarchar(max)") and verify via GetContainerColumnType()
- Renamed Complex_types_mapped_to_json_are_stored_in_snapshot to
  Complex_types_mapped_to_json_with_explicit_column_type_are_stored_in_snapshot
- Added new Complex_types_mapped_to_json_are_stored_in_snapshot test
  without explicit column type expecting nvarchar(max) in snapshot

Fixes #37275

Co-authored-by: AndriySvyryd <[email protected]>
Copilot AI changed the title Add model differ tests for JSON column migration from nvarchar(max) to json Fix snapshot generation to capture column type for JSON columns Dec 2, 2025
Copilot AI requested a review from AndriySvyryd December 2, 2025 22:42
Copilot finished work on behalf of AndriySvyryd December 2, 2025 22:42
… type test

Addresses PR feedback to not add a custom type for the test.

Co-authored-by: AndriySvyryd <[email protected]>
Copilot finished work on behalf of AndriySvyryd December 2, 2025 22:52
Moved the logic for adding ContainerColumnType annotation from
AnnotationCodeGenerator to CSharpSnapshotGenerator as requested.
Now GenerateEntityTypeAnnotations and GenerateComplexPropertyAnnotations
check if the entity/complex type is mapped to JSON and add the
ContainerColumnType annotation if it's missing.

Co-authored-by: AndriySvyryd <[email protected]>
Copilot AI requested a review from AndriySvyryd December 3, 2025 01:26
Copilot finished work on behalf of AndriySvyryd December 3, 2025 01:26
Per review feedback, updated CSharpSnapshotGenerator to first try
GetContainerColumnType() for both entity types and complex types,
then fall back to type mapping source if the method returns null.

Co-authored-by: AndriySvyryd <[email protected]>
Copilot AI requested a review from AndriySvyryd December 3, 2025 01:48
Copilot finished work on behalf of AndriySvyryd December 3, 2025 01:48
@AndriySvyryd AndriySvyryd changed the title Fix snapshot generation to capture column type for JSON columns [release/10.0] Fix snapshot generation to capture column type for JSON columns Dec 3, 2025
@AndriySvyryd AndriySvyryd marked this pull request as ready for review December 3, 2025 02:50
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner December 3, 2025 02:50
@AndriySvyryd
Copy link
Member

@copilot Add SqlServerSnapshotModelProcessor derived from SnapshotModelProcessor and add code to it to detect snapshots that are from version 9.*.* or older that don't have the container column type set (check for annotation presence) on the top-level owned type mapped and set it to nvarchar(max). In the test for this set the compatibility level to 170 to change the default to json (the older processed snapshot should still contain nvarchar(max)).

…older snapshots

- Created SqlServerSnapshotModelProcessor that detects snapshots from version 9 or older
- Processor sets ContainerColumnType to nvarchar(max) for JSON owned types without the annotation
- Registered the processor in SqlServerDesignTimeServices using TryAddProviderSpecificServices
- Added EFCore.Design project reference to EFCore.SqlServer for design-time types
- Added test to verify the processor correctly handles version 9 snapshots

Co-authored-by: AndriySvyryd <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 3, 2025

@copilot Add SqlServerSnapshotModelProcessor derived from SnapshotModelProcessor and add code to it to detect snapshots that are from version 9.*.* or older that don't have the container column ...

Fixed in efc876a. Added SqlServerSnapshotModelProcessor derived from SnapshotModelProcessor that detects snapshots from version 9.. or older with JSON owned types missing the ContainerColumnType annotation and sets it to nvarchar(max). Registered it in SqlServerDesignTimeServices using TryAddProviderSpecificServices. Added a test that verifies the processor works correctly.

Copilot finished work on behalf of AndriySvyryd December 3, 2025 22:08
@AndriySvyryd
Copy link
Member

@copilot Revert the last commit

Reverted the last commit that added:
- SqlServerSnapshotModelProcessor class
- EFCore.Design project reference in EFCore.SqlServer
- Related test

Co-authored-by: AndriySvyryd <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 4, 2025

@copilot Revert the last commit

Reverted in 6c0f828. Removed SqlServerSnapshotModelProcessor, the EFCore.Design project reference in EFCore.SqlServer, and the related test.

Copilot finished work on behalf of AndriySvyryd December 4, 2025 00:14
Copilot AI added a commit that referenced this pull request Dec 4, 2025
@AndriySvyryd AndriySvyryd requested a review from artl93 December 5, 2025 18:50
@AndriySvyryd
Copy link
Member

cc @artl93

@artl93
Copy link
Member

artl93 commented Dec 5, 2025

@AndriySvyryd - would it make sense to wait for the full fix?

@artl93
Copy link
Member

artl93 commented Dec 5, 2025

Never mind - I see we're ready to take both.

Copy link
Member

@artl93 artl93 left a comment

Choose a reason for hiding this comment

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

Regression, customer reported, blocked. Approved. Please take at same time as #37294.

AndriySvyryd added a commit that referenced this pull request Dec 5, 2025
@AndriySvyryd AndriySvyryd merged commit 9544f91 into release/10.0 Dec 5, 2025
8 checks passed
@AndriySvyryd AndriySvyryd deleted the copilot/fix-model-differ-test-json-columns branch December 5, 2025 22:12
@rbhanda rbhanda added this to the 10.0.2 milestone Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants