Skip to content

Conversation

@rstam
Copy link
Contributor

@rstam rstam commented Nov 10, 2025

No description provided.

@rstam rstam requested a review from a team as a code owner November 10, 2025 20:19
@rstam rstam added the feature Adds new user-facing functionality. label Nov 10, 2025
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");
}
Copy link
Contributor Author

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");
}
Copy link
Contributor Author

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);
Copy link
Contributor Author

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.

@rstam rstam requested a review from BorisDog November 10, 2025 21:14
@adelinowona adelinowona requested a review from Copilot November 11, 2025 21:35
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Adds new user-facing functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant