Cosmos: Implement owned type null comparison

[JoasE]

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

[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).

[Copilot]

Pull request overview

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

[JoasE]

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

[JoasE]

@roji Thanks for the feedback! Definitely some food for thought on some things which I might have to come back on 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.

I want to add that even tho this pr is currently titled 'owned type null comparison', this will also work for complex types after the query pipeline pr for that is implemented. In that pr, EntityReferenceExpression will be renamed to StructuralTypeReferenceExpression and this method will be able to handle both.

I will have a look at how this is done in SQL and adjust accordingly where possible

[roji]

I want to add that even tho this pr is currently titled 'owned type null comparison', this will also work for complex types after the query pipeline pr for that is implemented. In that pr, EntityReferenceExpression will be renamed to StructuralTypeReferenceExpression and this method will be able to handle both.

OK, but note that what comparison actually means is quite different across entity and complex types. Comparing entity types is checking whether they represent the same instance/identity (i.e. have the same primary key), whereas complex types are compared by their contents, as they have no identity; so the code for each case is going to look quite different. Definitely look closely at the relational implementation.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[roji]

@JoasE I've looked at this again, and rather than hacking it via ScalarReferenceExpression as I suggested earlier, I decided to just make SqlBinaryExpression constructible over non-SqlExpression nodes; I've pushed a commit, take a look and tell me what you think. This is far less hacky and goes in the direction we eventually really need to go.

[JoasE]

@roji That works aswell! I thought that the direction we need to go was more of ObjectAccessExpression being a SqlExpression, but I don't fully understand the separation of concerns here yet, so I will trust your judgment. Either way it's going to need some work after this to represent cosmos query language better so maybe we shouldn't be too nitpicky here.
I've merged with main, and did readd the check in CosmosSqlTranslatingExpressionVisitor.cs‎ you removed, which optimizes !false to true so it can filtered out by CosmosQueryableMethodTranslatingExpressionVisitor.cs. As I am guessing that was by accident? Unless you think it should be done differently. I did move it down to the switch instead of top level if check.