Skip to content
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

Linq: Adds support constant evaluation of Nullable<T>.HasValue #3273

Merged
merged 11 commits into from
Jul 12, 2022
19 changes: 17 additions & 2 deletions Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.Azure.Cosmos.Linq
/// </summary>
internal sealed class SubtreeEvaluator : ExpressionVisitor
{
private HashSet<Expression> candidates;
private readonly HashSet<Expression> candidates;

public SubtreeEvaluator(HashSet<Expression> candidates)
{
Expand Down Expand Up @@ -72,12 +72,27 @@ private Expression EvaluateMemberAccess(Expression expression)
ConstantExpression targetConstant = targetExpression as ConstantExpression;

// If we have a target expression but it cannot be resolved to a constant, then we should skip
// using reflectoin here and instead rely on the fallback delegate compilation approach.
// using reflection here and instead rely on the fallback delegate compilation approach.
if (targetExpression is not null && targetConstant is null)
{
return expression;
}

// We need special handling for Nullable<T>.HasValue. This is because most reflection
// methods, including property and field accessors, pass instance arguments as object type.
// Nullable<T> has special runtime behavior that boxes the value of the nullable instead of the
// nullable struct itself. When Nullable<T>.HasValue is false, it is boxed as a null value when
// passed to the PropertyInfo/FieldInfo.GetValue. This causes a TargetException to be thrown since
// we are trying to evaluate an instance property with a null target.
if (targetConstant is { Value: null } &&
ealsur marked this conversation as resolved.
Show resolved Hide resolved
Nullable.GetUnderlyingType(targetConstant.Type) != null &&
memberExpression.Member is {Name: "HasValue"})
{
// So, if we're calling Nullable<T>.HasValue and targetConstant.Value is null, that means HasValue
// would return false. Do that here to work around reflection quirks
return Expression.Constant(false);
}

if (memberExpression.Member is FieldInfo fieldInfo)
{
return Expression.Constant(fieldInfo.GetValue(targetConstant?.Value), memberExpression.Type);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<Results>
<Result>
<Input>
<Description><![CDATA[Filter on null double?]]></Description>
<Expression><![CDATA[query.Where(doc => (DisplayClass.nullDouble.HasValue AndAlso (Convert(doc.NumericField, Nullable`1) > DisplayClass.nullDouble)))]]></Expression>
</Input>
<Output>
<SqlQuery><![CDATA[
SELECT VALUE root
FROM root
WHERE (false AND (root["NumericField"] > null))]]></SqlQuery>
</Output>
</Result>
<Result>
<Input>
<Description><![CDATA[Filter on false double?]]></Description>
<Expression><![CDATA[query.Where(doc => (DisplayClass.zeroDouble.HasValue AndAlso (Convert(doc.NumericField, Nullable`1) > DisplayClass.zeroDouble)))]]></Expression>
</Input>
<Output>
<SqlQuery><![CDATA[
SELECT VALUE root
FROM root
WHERE (true AND (root["NumericField"] > 0))]]></SqlQuery>
</Output>
</Result>
<Result>
<Input>
<Description><![CDATA[Filter on null bool?]]></Description>
<Expression><![CDATA[query.Where(doc => (DisplayClass.nullBool.HasValue AndAlso (Convert(doc.BooleanField, Nullable`1) == DisplayClass.nullBool)))]]></Expression>
</Input>
<Output>
<SqlQuery><![CDATA[
SELECT VALUE root
FROM root
WHERE (false AND (root["BooleanField"] = null))]]></SqlQuery>
</Output>
</Result>
<Result>
<Input>
<Description><![CDATA[Filter on false bool?]]></Description>
<Expression><![CDATA[query.Where(doc => (DisplayClass.zeroDouble.HasValue AndAlso (Convert(doc.BooleanField, Nullable`1) == DisplayClass.falseBool)))]]></Expression>
</Input>
<Output>
<SqlQuery><![CDATA[
SELECT VALUE root
FROM root
WHERE (true AND (root["BooleanField"] = false))]]></SqlQuery>
</Output>
</Result>
</Results>
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ internal class DataObject : LinqTestObject
public List<int> EnumerableField;
public Point Point;
public int? NullableField;
#pragma warning disable CS0649 // Field is never assigned to, and will always have its default value false
public bool BooleanField;
#pragma warning restore // Field is never assigned to, and will always have its default value false

[JsonConverter(typeof(StringEnumConverter))]
public TestEnum EnumField1;
Expand Down Expand Up @@ -588,7 +591,6 @@ public void TestMathFunctions()
this.ExecuteTestSuite(inputs);
}


[TestMethod]
public void TestMemberAccess()
{
Expand Down Expand Up @@ -619,6 +621,28 @@ public void TestMemberAccess()
this.ExecuteTestSuite(inputs);
}

[TestMethod]
public void TestMemberAccessWithNullableTypes()
{
List<DataObject> testData = new();
IOrderedQueryable<DataObject> constantQuery = testContainer.GetItemLinqQueryable<DataObject>(allowSynchronousQueryExecution: true);
Func<bool, IQueryable<DataObject>> getQuery = useQuery => useQuery ? constantQuery : testData.AsQueryable();

double? nullDouble = null;
double? zeroDouble = 0;
bool? nullBool = null;
bool? falseBool = false;

List<LinqTestInput> inputs = new()
{
new LinqTestInput("Filter on null double?", b => getQuery(b).Where(doc => nullDouble.HasValue && doc.NumericField > nullDouble)),
new LinqTestInput("Filter on false double?", b => getQuery(b).Where(doc => zeroDouble.HasValue && doc.NumericField > zeroDouble)),
new LinqTestInput("Filter on null bool?", b => getQuery(b).Where(doc => nullBool.HasValue && doc.BooleanField == nullBool)),
new LinqTestInput("Filter on false bool?", b => getQuery(b).Where(doc => zeroDouble.HasValue && doc.BooleanField == falseBool)),
};
this.ExecuteTestSuite(inputs);
}

private Func<bool, IQueryable<DataObject>> CreateDataTestStringFunctions()
{
const int Records = 100;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
<None Remove="BaselineTest\TestBaseline\EndToEndTraceWriterBaselineTests.StreamPointOperationsAsync.xml" />
<None Remove="BaselineTest\TestBaseline\EndToEndTraceWriterBaselineTests.TypedPointOperationsAsync.xml" />
<None Remove="BaselineTest\TestBaseline\LinqTranslationBaselineTests.TestDateTimeJsonConverterTimezones.xml" />
<None Remove="BaselineTest\TestBaseline\LinqTranslationBaselineTests.TestMemberAccessWithNullableTypes.xml" />
</ItemGroup>

<ItemGroup>
Expand Down Expand Up @@ -193,6 +194,9 @@
<Content Include="BaselineTest\TestBaseline\LinqTranslationBaselineTests.TestDateTimeJsonConverterTimezones.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="BaselineTest\TestBaseline\LinqTranslationBaselineTests.TestMemberAccessWithNullableTypes.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="BaselineTest\TestBaseline\LinqTranslationBaselineTests.TestSelectTop.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
Expand Down