Cosmos: Implement owned type null comparison #37321

JoasE · 2025-12-08T14:08:56Z

Comparison used to compare by checking on primary key properties, but this doesn't work for cosmos as it will filter out any document that doesn't contain a property used in a query condition. Compare by c["Prop1"]["Prop2"] = null instead. Don't compare document roots as those can't be null. Fixes: #24087

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

Comparison used to compare by checking on primary key properties, but this doesn't work for cosmos as it will filter out any document that doesn't contain a property used in a query condition. Compare by c["Prop"] = null instead Fixes: dotnet#24087

…et#24087

Copilot

Pull request overview

Implements correct null/not-null comparison translation for owned types in the Cosmos provider by using IS_NULL(...) OR NOT IS_DEFINED(...) semantics (and avoiding null checks on document roots), and updates/extends query tests accordingly.

Changes:

Update Cosmos structural/entity equality translation to generate IS_NULL/IS_DEFINED checks for null comparisons (and simplify always-true/always-false document-root comparisons).
Add new structural-equality test coverage for optional-nested-owned null/not-null comparisons across providers.
Update Cosmos functional test baselines to reflect the new translation behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs	Changes null equality translation to use `IS_NULL`/`IS_DEFINED` and avoids null checks on document roots.
src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs	Skips applying predicates that are effectively constant-true (`NOT false`) to avoid redundant filters.
test/EFCore.Specification.Tests/Query/Associations/AssociationsStructuralEqualityTestBase.cs	Adds base tests for optional-associate nested null/not-null scenarios.
test/EFCore.Cosmos.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualityCosmosTest.cs	Updates Cosmos assertions to validate new null/not-null SQL translation for owned navigations.
test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs	Updates Cosmos baselines for entity null/not-null comparisons to match new constant folding.
test/EFCore.Sqlite.FunctionalTests/Query/Associations/OwnedTableSplitting/OwnedTableSplittingStructuralEqualitySqliteTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests.
test/EFCore.Sqlite.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualitySqliteTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests.
test/EFCore.Sqlite.FunctionalTests/Query/Associations/OwnedJson/OwnedJsonStructuralEqualitySqliteTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests (JSON).
test/EFCore.SqlServer.FunctionalTests/Query/Associations/OwnedTableSplitting/OwnedTableSplittingStructuralEqualitySqlServerTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests.
test/EFCore.SqlServer.FunctionalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualitySqlServerTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests.
test/EFCore.SqlServer.FunctionalTests/Query/Associations/OwnedJson/OwnedJsonStructuralEqualitySqlServerTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests (JSON).
test/EFCore.SqlServer.FunctionalTests/Query/Associations/Navigations/NavigationsStructuralEqualitySqlServerTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests (non-owned navigations).
test/EFCore.SqlServer.FunctionalTests/Query/Associations/ComplexTableSplitting/ComplexTableSplittingStructuralEqualitySqlServerTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests (complex/table-splitting).
test/EFCore.SqlServer.FunctionalTests/Query/Associations/ComplexJson/ComplexJsonStructuralEqualitySqlServerTest.cs	Adds provider-specific SQL baselines for the new optional-nested null/not-null tests (complex/JSON).

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

Copilot · 2026-01-29T10:26:17Z

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

+        // Null equality
+        if (IsNullSqlConstantExpression(compareReference))
        {
-            var nonNullEntityReference = (IsNullSqlConstantExpression(left) ? rightEntityReference : leftEntityReference)!;
-            var entityType1 = nonNullEntityReference.EntityType;
-            var primaryKeyProperties1 = entityType1.FindPrimaryKey()?.Properties;
-            if (primaryKeyProperties1 == null)
+            if (entityType.IsDocumentRoot() && entityReference.Subquery == null)
            {
-                throw new InvalidOperationException(
-                    CoreStrings.EntityEqualityOnKeylessEntityNotSupported(
-                        nodeType == ExpressionType.Equal
-                            ? equalsMethod ? nameof(object.Equals) : "=="
-                            : equalsMethod
-                                ? "!" + nameof(object.Equals)
-                                : "!=",
-                        entityType1.DisplayName()));
+                // Document root can never be be null
+                result = Visit(Expression.Constant(nodeType != ExpressionType.Equal));
+                return true;
            }

-            result = Visit(
-                primaryKeyProperties1.Select(p =>
-                        Expression.MakeBinary(
-                            nodeType, CreatePropertyAccessExpression(nonNullEntityReference, p),
-                            Expression.Constant(null, p.ClrType.MakeNullable())))
-                    .Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r)));
+            var isNull = sqlExpressionFactory.Function("IS_NULL", [entityReference.Object], typeof(bool));
+            var isDefined = sqlExpressionFactory.Function("IS_DEFINED", [entityReference.Object], typeof(bool));
+            var notDefined = sqlExpressionFactory.Not(isDefined);
+            var check = sqlExpressionFactory.MakeBinary(ExpressionType.OrElse, isNull, notDefined, typeMappingSource.FindMapping(typeof(bool))) ?? throw new UnreachableException();

+            if (nodeType == ExpressionType.NotEqual)
+            {
+                check = sqlExpressionFactory.Not(check);
+            }
+
+            result = check;
            return true;
        }


Entity equality/null comparison no longer throws for keyless entity types: the keyless check is skipped in the null-comparison branch and only enforced for non-null comparisons. Other providers still throw for entity == null when the entity type is keyless (e.g. RelationalSqlTranslatingExpressionVisitor.StructuralEquality.cs:146-157). If this isn’t intentionally Cosmos-specific, consider preserving the keyless exception behavior here as well to avoid inconsistent semantics across providers.

I thought it was ok to not throw in this case as we are able to easily compare to null the same way we do with keyfull entities. @roji What do you think?

I agree... I think the previous check was more of a carryover from relational, where you cannot compare the entire (JSON) structural type directly, but rather always have to compare key properties. In a JSON database, comparison to null can be easily translated to object = null without referencing any key properties, so I think this makes sense.

src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs

...nalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualityCosmosTest.cs

…ld fail" This reverts commit c294e3b.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

src/EFCore.Cosmos/Query/Internal/Expressions/SqlObjectAccessExpression.cs

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

...nalTests/Query/Associations/OwnedNavigations/OwnedNavigationsStructuralEqualityCosmosTest.cs

src/EFCore.Cosmos/Query/Internal/Expressions/SqlObjectAccessExpression.cs

JoasE · 2026-01-31T15:46:34Z

If you want me to reopen to clear out the noise lmk

roji

One general note... I certainly haven't thought this through (or confirmed with @AndriySvyryd), but assuming we implement full complex type support for Cosmos in 11 (on par with the current owned mapping support), I'm not quite sure what the status of owned entity mapping would then be. Unlike with relational JSON, Cosmos users don't explicitly opt into either owned or complex mapping - they currently just get owned JSON as the only option. I believe it would make sense to simply switch the default over from owned to complex, and at that point I'm also not sure what possible reasons someone might have to prefer owned - everything that was possible with owned should be possible with complex, plus some other basic capabilities that are currently lacking (such as comparing JSON objects).

So in one extreme scenario we'd simply drop Cosmos owned JSON altogether in 11; in another, at the very least in order to be cautious, we'd switch over to complex by default but allow opting back into owned (preferably via an API marked with [Obsolete] that we'd remove later).

More concretely about this PR though... This PR fixes owned JSON equality; similar to relational JSON, I wouldn't want us to spend too much time fixing/improving that in general, as it's no longer going to be the way to do things going forward. So in general I'd recommend focusing efforts on complex mapping.

/cc @AndriySvyryd

roji · 2026-01-31T18:16:09Z

src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs

        {
-            if (translation is not SqlConstantExpression { Value: true })
+            if (translation is not SqlConstantExpression { Value: true } &&
+                translation is not SqlUnaryExpression {


Rather than pattern matching !false, it would be better if we ensured that TranslateLambdaExpression itself already performs the reduction of !false and returns true. This would simplify the tree for many more query scenarios without us having to check for !false everywhere.

roji · 2026-01-31T18:19:05Z

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

-
-        if (leftEntityReference == null
-            && rightEntityReference == null)
+        var entityReference = left as EntityReferenceExpression ?? right as EntityReferenceExpression;


Just noting that ideally we'd align across Cosmos and relational as far as it makes sense. So I'd suggest either adopting the relational code pattern here, or if you think you have something better, feel free to propose cleaning up the relational one (but the goal at the end is to have the two flows be as close as possible).

BTW maybe also this implementation out to CosmosSqlTranslatingExpressionVisitor.StructuralEquality.cs, as in relational?

roji · 2026-01-31T18:22:13Z

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

-        if (IsNullSqlConstantExpression(left)
-            || IsNullSqlConstantExpression(right))
+        var entityType = entityReference.EntityType;
+        var compareReference = entityReference == left ? right : left;


nit: you can have a switch expression that returns both entityReference and compareReference above.

roji · 2026-01-31T18:27:33Z

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

-            var entityType1 = nonNullEntityReference.EntityType;
-            var primaryKeyProperties1 = entityType1.FindPrimaryKey()?.Properties;
-            if (primaryKeyProperties1 == null)
+            if (entityType.IsDocumentRoot() && entityReference.Subquery == null)


While it's true that document root can never be null, why distinguish between document root non-roots which are also non-nullable, rather than having a single code path handling both? Concretely, you could simply check entityReference.Parameter.IsNullable - this should be false for both roots and non-nullable non-roots.

roji · 2026-01-31T18:30:58Z

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

-                    .Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r)));
-
+            // Treat type as object for null comparison
+            var access = new SqlObjectAccessExpression(entityReference.Object);


Why do we need a SqlObjectAccessExpression expression here? Isn't it enough to simply pass whatever entityReference is wrapping?

roji · 2026-01-31T18:34:07Z

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

+        // Null equality
+        if (IsNullSqlConstantExpression(compareReference))
        {
-            var nonNullEntityReference = (IsNullSqlConstantExpression(left) ? rightEntityReference : leftEntityReference)!;
-            var entityType1 = nonNullEntityReference.EntityType;
-            var primaryKeyProperties1 = entityType1.FindPrimaryKey()?.Properties;
-            if (primaryKeyProperties1 == null)
+            if (entityType.IsDocumentRoot() && entityReference.Subquery == null)
            {
-                throw new InvalidOperationException(
-                    CoreStrings.EntityEqualityOnKeylessEntityNotSupported(
-                        nodeType == ExpressionType.Equal
-                            ? equalsMethod ? nameof(object.Equals) : "=="
-                            : equalsMethod
-                                ? "!" + nameof(object.Equals)
-                                : "!=",
-                        entityType1.DisplayName()));
+                // Document root can never be be null
+                result = Visit(Expression.Constant(nodeType != ExpressionType.Equal));
+                return true;
            }

-            result = Visit(
-                primaryKeyProperties1.Select(p =>
-                        Expression.MakeBinary(
-                            nodeType, CreatePropertyAccessExpression(nonNullEntityReference, p),
-                            Expression.Constant(null, p.ClrType.MakeNullable())))
-                    .Aggregate((l, r) => nodeType == ExpressionType.Equal ? Expression.OrElse(l, r) : Expression.AndAlso(l, r)));
+            var isNull = sqlExpressionFactory.Function("IS_NULL", [entityReference.Object], typeof(bool));
+            var isDefined = sqlExpressionFactory.Function("IS_DEFINED", [entityReference.Object], typeof(bool));
+            var notDefined = sqlExpressionFactory.Not(isDefined);
+            var check = sqlExpressionFactory.MakeBinary(ExpressionType.OrElse, isNull, notDefined, typeMappingSource.FindMapping(typeof(bool))) ?? throw new UnreachableException();

+            if (nodeType == ExpressionType.NotEqual)
+            {
+                check = sqlExpressionFactory.Not(check);
+            }
+
+            result = check;
            return true;
        }


I agree... I think the previous check was more of a carryover from relational, where you cannot compare the entire (JSON) structural type directly, but rather always have to compare key properties. In a JSON database, comparison to null can be easily translated to object = null without referencing any key properties, so I think this makes sense.

roji · 2026-01-31T18:48:00Z

src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

+        if (compareReference is EntityReferenceExpression compareEntityReference)
+        {
+            // Comparing of 2 different entity types is always false.
+            if (entityType.GetRootType() != compareEntityReference.EntityType.GetRootType())


I'm slightly confused here... Unless I'm mistaken, when comparing two different entity types, the result is always false, regardless of whether they have the same root or not. In other words, if I want all customers where BillingAddress == ShippingAddress, I will always get nothing back. This sort of odd/unintuitive/non-useful behavior is exactly one of the reasons we're moving from entity type modeling for JSON to complex type, where comparison is structural (compare the contents, not the "identity").

If that's all accurate, then I don't think you need to call GetRootType() above - the moment you have an entity reference on both sides and the IEntityType isn't the same, we can immediately return false. Makes sense?

Minimal Cosmos test comparing two entity types with the same root

await using var context = new BlogContext(); await context.Database.EnsureDeletedAsync(); await context.Database.EnsureCreatedAsync(); _ = await context.Blogs.Where(b => b.ShippingAddress == b.BillingAddress).ToListAsync(); public class BlogContext : DbContext { public DbSet<Blog> Blogs { get; set; } protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder .UseCosmos( Environment.GetEnvironmentVariable("CosmosNoSql__ConnectionString")!, databaseName: "test", o => o .ConnectionMode(ConnectionMode.Gateway) .HttpClientFactory(() => new HttpClient( new HttpClientHandler { ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator }))) .LogTo(Console.WriteLine, LogLevel.Information) .EnableSensitiveDataLogging(); protected override void OnModelCreating(ModelBuilder modelBuilder) { modelBuilder.Entity<Blog>().OwnsOne(b => b.ShippingAddress, b => b.ToJson()); modelBuilder.Entity<Blog>().OwnsOne(b => b.BillingAddress, b => b.ToJson()); } } public class Blog { public int Id { get; set; } public string Name { get; set; } public Address ShippingAddress { get; set; } public Address BillingAddress { get; set; } } public class Address { public string Street { get; set; } public string City { get; set; } }

Generated SQL:

SELECT VALUE c FROM root c WHERE false

roji · 2026-01-31T18:48:19Z

src/EFCore.Cosmos/Query/Internal/Expressions/SqlObjectAccessExpression.cs

+///     doing so can result in application failures when updating to a new Entity Framework Core release.
+/// </remarks>
+[DebuggerDisplay("{Microsoft.EntityFrameworkCore.Query.ExpressionPrinter.Print(this), nq}")]
+public class SqlObjectAccessExpression(Expression @object)


I'm not sure this is required - see comment below.

JoasE added 4 commits December 8, 2025 15:07

Cleanup

fe40eac

Regenerate NorthwindMiscellaneousQueryCosmosTest and optimize NOT(false)

b3af934

Small cleanup

27fdc25

JoasE marked this pull request as ready for review December 8, 2025 19:25

JoasE requested a review from a team as a code owner December 8, 2025 19:25

JoasE mentioned this pull request Dec 8, 2025

Question on Cosmos DB database provider owned object querying null #24087

Open

AndriySvyryd added the community-contribution label Dec 10, 2025

AndriySvyryd assigned roji Dec 22, 2025

roji force-pushed the main branch 2 times, most recently from 249ae47 to 6b86657 Compare January 13, 2026 17:46

This comment was marked as off-topic.

Sign in to view

Merge branch 'main' of https://github.com/dotnet/efcore into fix/dotn…

a2dba87

…et#24087

This comment was marked as resolved.

Sign in to view

Move to IS_NULL OR NOT IS_DEFINED and add test cases that would fail

c294e3b

Copilot AI review requested due to automatic review settings January 29, 2026 10:15

Copilot started reviewing on behalf of JoasE January 29, 2026 10:16 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

JoasE mentioned this pull request Jan 29, 2026

Cosmos: Complex properties Query #37577

Draft

6 tasks

Revert "Move to IS_NULL OR NOT IS_DEFINED and add test cases that wou…

0583a45

…ld fail" This reverts commit c294e3b.

JoasE requested a review from Copilot January 31, 2026 15:27

Copilot started reviewing on behalf of JoasE January 31, 2026 15:27 View session

Fix typo and code style

9bc1e29

Copilot AI reviewed Jan 31, 2026

View reviewed changes

Update styling and small changes

2bcb90f

roji requested changes Jan 31, 2026

View reviewed changes

roji changed the title ~~Cosmos db provider: Implement owned type null comparison~~ Cosmos: Implement owned type null comparison Jan 31, 2026

Cosmos: Implement owned type null comparison #37321

Are you sure you want to change the base?

Cosmos: Implement owned type null comparison #37321

Conversation

JoasE commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as off-topic.

This comment was marked as resolved.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoasE commented Jan 31, 2026

Uh oh!

roji left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JoasE commented Dec 8, 2025 •

edited

Loading