Skip to content

[release/9.0-staging] Fix to #21006 - Support a default value for non-nullable properties #35782

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

Merged
merged 1 commit into from
Mar 14, 2025

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Mar 13, 2025

Port of #35746
Fixes #21006

Description
Problem was that we don't have a good story for schema evolution in Cosmos (e.g. adding a new non-nullable property). We start generating shaper code expecting the new schema, but the old documents are not being updated. Effectively, querying for those modified entities stops working because the document no longer matches the expected shape. This is a major pain point and adoption blocker for EF Core Cosmos users. While designing a compelling schema evolution story is a large task and out of scope for now, we believe we can solve majority of the problems with a targeted change - when generating shaper for Cosmos entities, if required value is not present, we generate the CLR default rather than throw.

Customer impact
EF Core customers using Cosmos experience exceptions when querying for an entity which had it's schema modified, e.g. by adding a non-nullable property. There is no good workaround, apart from manually adding missing data to the document or using nullable properties instead.

How found
Reported by multiple customers on 9.

Regression
No.

Testing
Added multiple tests for scalar and reference scenarios, all supported types, converters vs no. missing value vs explicit nulls.

Risk
Low - Targeted fix, when constructing non-nullable property of an entity type we produce default of that type, rather than null. This does not affect keys, just regular scalar properties. Potential negative side-effect is that we lose the validation for scenarios where legitimate required property is introduced, but the value hasn't been provided for it in the document by accident. Added a quirk as an escape hatch to revert to previous behavior.

Description
Problem was that we don't have a good story for schema evolution in Cosmos (e.g. adding a new non-nullable property). We start generating shaper code expecting the new schema, but the old documents are not being updated. Effectively, querying for those modified entities stops working because the document no longer matches the expected shape. This is a major pain point and adoption blocker for EF Core Cosmos users. While designing a compelling schema evolution story is a large task and out of scope for now, we believe we can solve majority of the problems with a targeted change - when generating shaper for Cosmos entities, if required value is not present, we generate the CLR default rather than throw.

Customer impact
EF Core customers using Cosmos experience exceptions when querying for an entity which had it's schema modified, e.g. by adding a non-nullable property. There is no good workaround, apart from manually adding missing data to the document or using nullable properties instead.

How found
Reported by multiple customers on 9.

Regression
No.

Testing
Added multiple tests for scalar and reference scenarios, all supported types, converters vs no. missing value vs explicit nulls.

Risk
Low - Targeted fix, when constructing non-nullable property of an entity type we produce default of that type, rather than null. This does not affect keys, just regular scalar properties.
Potential negative side-effect is that we lose the validation for scenarios where legitimate required property is introduced, but the value hasn't been provided for it in the document by accident.
Added a quirk as an escape hatch to revert to previous behavior.

Fixes #21006
@maumar maumar changed the title Fix to #21006 - Support a default value for non-nullable properties [release/9.0-staging] Fix to #21006 - Support a default value for non-nullable properties Mar 13, 2025
@maumar maumar requested review from Copilot and a team March 13, 2025 00:20
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses issue #21006 by changing the behavior for missing non‐nullable properties in Cosmos when materializing query results. Key changes include:

  • Modifying the Cosmos shaper to return CLR default values for missing non-nullable scalars (with a feature flag switch).
  • Updating helper and test classes across Cosmos, Sqlite, SQL Server, and InMemory providers to validate the new behavior.
  • Renaming test base classes (e.g. from AdHocJsonQueryTestBase to AdHocJsonQueryRelationalTestBase) to reflect their relational focus.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/EFCore.Cosmos.FunctionalTests/Query/AdHocCosmosTestHelpers.cs Introduces a new helper for Cosmos test document creation.
test/EFCore.Cosmos.FunctionalTests/Query/AdHocMiscellaneousQueryCosmosTest.cs Adds tests for missing scalar values and schema evolution scenarios.
test/EFCore.Sqlite.FunctionalTests/Query/AdHocJsonQuerySqliteTest.cs Updates test base class and seed data to incorporate the new behavior.
test/EFCore.SqlServer.FunctionalTests/Query/AdHocJsonQuerySqlServerTestBase.cs Updates tests with AssertSql validations and adjusts base class usage.
test/EFCore.Relational.Specification.Tests/Query/AdHocJsonQueryRelationalTestBase.cs Adjusts test base for relational JSON queries including new exception messages.
src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs Implements conditional conversion logic based on a feature flag.
test/EFCore.InMemory.FunctionalTests/InMemoryComplianceTest.cs Updates InMemory compliance tests to include the JSON query test base.
Comments suppressed due to low confidence (3)

src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs:801

  • Review the conversion logic at this line; if originalBodyType differs from the expected type, the conversion for non-nullable scalars may produce unexpected results. Consider adding targeted tests for this edge case.
replaceExpression = isNonNullableScalar && !UseOldBehavior21006 ? Expression.Convert(Default(originalBodyType), type) : Default(type);

src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs:24

  • Ensure additional test coverage exists for both branches of the UseOldBehavior21006 flag to validate the behavior of non-nullable scalar conversion under different scenarios.
private static readonly bool UseOldBehavior21006 = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue21006", out var enabled21006) && enabled21006;

test/EFCore.InMemory.FunctionalTests/InMemoryComplianceTest.cs:28

  • [nitpick] Consider using AdHocJsonQueryRelationalTestBase instead of AdHocJsonQueryTestBase for consistency with changes in other providers, if applicable.
typeof(AdHocJsonQueryTestBase),

@maumar maumar closed this Mar 13, 2025
@maumar maumar reopened this Mar 13, 2025
@maumar maumar marked this pull request as ready for review March 14, 2025 01:42
@maumar maumar added this to the 9.0.x milestone Mar 14, 2025
@maumar maumar merged commit 21cc2ab into release/9.0-staging Mar 14, 2025
8 checks passed
@maumar maumar deleted the fix21006_90 branch March 14, 2025 19:41
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.

3 participants