镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Support && and || null checks in conditional projection member access - #39636

Open
AhmedSadek10 wants to merge 1 commit into
dotnet:mainfrom
AhmedSadek10:issue-38565-and-or-null-checks
Open

AhmedSadek10 wants to merge 1 commit into
dotnet:mainfrom
AhmedSadek10:issue-38565-and-or-null-checks

Conversation

@AhmedSadek10

Copy link
Copy Markdown
Contributor

Fixes #38565

  • 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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A critical active finding shows the rewrite can change null-branch member access from a CLR NullReferenceException into null for all providers, so its eligibility condition is not semantics-preserving.

1 open issue
What changed in this PR

This PR fixes issue #38565 by supporting member access over conditional projections guarded by compound null checks (&& and ||). The generalized optimizer rewrite requires a null-safety restriction to avoid changing behavior for arbitrary conditionals with one null branch.

Changes:

  • Broadens conditional member-access optimization for compound null-check predicates.
  • Adds shared synchronous and asynchronous regression coverage for conjunctive and disjunctive null checks.
  • Adds SQL Server and SQLite query baselines.
File Description
test/​EFCore.SqlServer.FunctionalTests/​Query/​AdHocNavigationsQuerySqlServerTest.cs Adds SQL Server query baselines covering the conditional projection regression cases.
test/​EFCore.Sqlite.FunctionalTests/​Query/​AdHocNavigationsQuerySqliteTest.cs Adds SQLite query baselines covering the conditional projection regression cases.
test/​EFCore.Specification.Tests/​Query/​AdHocNavigationsQueryTestBase.cs Adds shared synchronous and asynchronous regression tests, seed data, and assertions for compound null checks.
src/​EFCore/​Query/​Internal/​QueryOptimizingExpressionVisitor.cs Generalizes conditional projection member-access optimization to handle null branches regardless of the conditional test expression.

Review effort: Lite

Comment on lines +368 to +372
if (conditional is { } conditionalExpression
&& !(conditionalExpression.Type.IsNullableValueType()
&& visitedMemberExpression.Member.Name is nameof(Nullable<>.HasValue) or nameof(Nullable<>.Value)))
{
var isLeftNullConstant = binaryTest.Left is ConstantExpression { Value: null };
var isRightNullConstant = binaryTest.Right is ConstantExpression { Value: null };

if (isLeftNullConstant != isRightNullConstant
&& ((binaryTest.NodeType == ExpressionType.Equal
&& conditionalExpression.IfTrue is ConstantExpression { Value: null })
|| (binaryTest.NodeType == ExpressionType.NotEqual
&& conditionalExpression.IfFalse is ConstantExpression { Value: null })))
var isTrueNullConstant = conditionalExpression.IfTrue is ConstantExpression { Value: null };
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.

Query translation fails when Where references a projection guarded by child != null && grandChild != null

2 participants