-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5691: Non-deterministic functions should not be evaluated client-side #1808
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
base: main
Are you sure you want to change the base?
Conversation
| if (expression.IsNonDeterministic()) | ||
| { | ||
| throw new ExpressionNotSupportedException(expression, because: $"non-deterministic field or property '{declaringType.Name}.{memberName}' should not be evaluated client-side and is not currently supported server-side"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For non-deterministic expressions give a better error message saying why it isn't supported.
| if (expression.IsNonDeterministic()) | ||
| { | ||
| throw new ExpressionNotSupportedException(expression, because: $"non-deterministic method '{declaringType.Name}.{method.Name}' should not be evaluated client-side and is not currently supported server-side"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For non-deterministic expressions give a better error message saying why it isn't supported.
| var collection = Fixture.Collection; | ||
|
|
||
| var queryable = collection.AsQueryable() | ||
| .Select(x => DateTime.Now); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually these will be translated into server-side expressions, but that's a separate ticket.
There was a problem hiding this 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 CSHARP-5691 by preventing non-deterministic functions from being evaluated client-side in LINQ queries. The changes ensure that operations like DateTime.Now, Guid.NewGuid(), and Random.Next() throw clear exceptions rather than being silently evaluated on the client with potentially incorrect query results.
Key changes:
- Added detection of non-deterministic methods and properties during expression translation
- Modified partial evaluation to skip non-deterministic expressions
- Added comprehensive test coverage for all non-deterministic operations
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
CSharp5691Tests.cs |
New test file verifying that non-deterministic operations throw appropriate exceptions |
MethodCallExpressionToAggregationExpressionTranslator.cs |
Added check to detect and reject non-deterministic method calls |
MemberExpressionToAggregationExpressionTranslator.cs |
Added check to detect and reject non-deterministic property accesses |
ReflectionInfo.cs |
Added parameterless overloads for Method and Property to support static member reflection |
RandomMethod.cs |
New reflection helper for Random.Next() method |
GuidMethod.cs |
New reflection helper for Guid.NewGuid() method |
DateTimeProperty.cs |
New reflection helper for DateTime.Now, DateTime.Today, and DateTime.UtcNow |
DateTimeOffsetProperty.cs |
New reflection helper for DateTimeOffset.Now and DateTimeOffset.UtcNow |
PartialEvaluator.cs |
Updated visitor to mark non-deterministic expressions as non-evaluable |
ExpressionExtensions.cs |
Added IsNonDeterministic() method and arrays of non-deterministic methods/properties |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.