Skip to content

Commit

Permalink
Cosmos: Avoid reading default values from database (#21821)
Browse files Browse the repository at this point in the history
Resolves #21678
  • Loading branch information
smitpatel committed Jul 28, 2020
1 parent 9c8f116 commit e9fae4a
Show file tree
Hide file tree
Showing 8 changed files with 319 additions and 226 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,10 @@ protected override ShapedQueryExpression TranslateCount(ShapedQueryExpression so

selectExpression.ClearOrdering();
selectExpression.ReplaceProjectionMapping(projectionMapping);
return source.UpdateShaperExpression(new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), typeof(int)));
return source.UpdateShaperExpression(
Expression.Convert(
new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), typeof(int?)),
typeof(int)));
}

/// <summary>
Expand Down Expand Up @@ -631,7 +634,10 @@ protected override ShapedQueryExpression TranslateLongCount(ShapedQueryExpressio

selectExpression.ClearOrdering();
selectExpression.ReplaceProjectionMapping(projectionMapping);
return source.UpdateShaperExpression(new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), typeof(long)));
return source.UpdateShaperExpression(
Expression.Convert(
new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), typeof(long?)),
typeof(long)));
}

/// <summary>
Expand Down Expand Up @@ -1132,8 +1138,7 @@ private ShapedQueryExpression AggregateResultShaper(
selectExpression.ClearOrdering();

var nullableResultType = resultType.MakeNullable();
Expression shaper = new ProjectionBindingExpression(
source.QueryExpression, new ProjectionMember(), throwOnNullResult ? nullableResultType : projection.Type);
Expression shaper = new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), nullableResultType);

if (throwOnNullResult)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,9 @@ private Expression CreateGetValueExpression(
return Expression.Default(clrType);
}

return CreateGetValueExpression(jObjectExpression, storeName, clrType, property.GetTypeMapping());
return Expression.Convert(
CreateGetValueExpression(jObjectExpression, storeName, clrType.MakeNullable(), property.GetTypeMapping()),
clrType);
}

private Expression CreateGetValueExpression(
Expand All @@ -649,6 +651,8 @@ private Expression CreateGetValueExpression(
Type clrType,
CoreTypeMapping typeMapping = null)
{
Check.DebugAssert(clrType.IsNullableType(), "Must read nullable type from JObject.");

var innerExpression = jObjectExpression;
if (_projectionBindings.TryGetValue(jObjectExpression, out var innerVariable))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ private static readonly MethodInfo _getParameterValueMethodInfo

private readonly RelationalQueryableMethodTranslatingExpressionVisitor _queryableMethodTranslatingExpressionVisitor;
private readonly RelationalSqlTranslatingExpressionVisitor _sqlTranslator;
private readonly IncludeFindingExpressionVisitor _includeFindingExpressionVisitor;

private SelectExpression _selectExpression;
private SqlExpression[] _existingProjections;
Expand All @@ -52,6 +53,7 @@ public RelationalProjectionBindingExpressionVisitor(
{
_queryableMethodTranslatingExpressionVisitor = queryableMethodTranslatingExpressionVisitor;
_sqlTranslator = sqlTranslatingExpressionVisitor;
_includeFindingExpressionVisitor = new IncludeFindingExpressionVisitor();
}

/// <summary>
Expand Down Expand Up @@ -367,7 +369,8 @@ protected override Expression VisitMember(MemberExpression memberExpression)
Expression updatedMemberExpression = memberExpression.Update(
expression != null ? MatchTypes(expression, memberExpression.Expression.Type) : expression);

if (expression?.Type.IsNullableValueType() == true)
if (expression?.Type.IsNullableType() == true
&& !_includeFindingExpressionVisitor.ContainsInclude(expression))
{
var nullableReturnType = memberExpression.Type.MakeNullable();
if (!memberExpression.Type.IsNullableType())
Expand Down Expand Up @@ -591,5 +594,33 @@ private static Expression MatchTypes(Expression expression, Type targetType)
private static T GetParameterValue<T>(QueryContext queryContext, string parameterName)
#pragma warning restore IDE0052 // Remove unread private members
=> (T)queryContext.ParameterValues[parameterName];

private sealed class IncludeFindingExpressionVisitor : ExpressionVisitor
{
private bool _containsInclude;

public bool ContainsInclude(Expression expression)
{
_containsInclude = false;

Visit(expression);

return _containsInclude;
}

public override Expression Visit(Expression expression) => _containsInclude ? expression : base.Visit(expression);

protected override Expression VisitExtension(Expression extensionExpression)
{
if (extensionExpression is IncludeExpression)
{
_containsInclude = true;

return extensionExpression;
}

return base.VisitExtension(extensionExpression);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,6 @@ FROM root c
WHERE ((c[""Discriminator""] = ""BuiltInDataTypes"") AND (c[""Id""] = 13))");
}

[ConditionalFact(Skip = "Issue#21678")]
public override void Optional_datetime_reading_null_from_database()
{
base.Optional_datetime_reading_null_from_database();
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,6 @@ public override void Optional_owned_with_converter_reading_non_nullable_column()
base.Optional_owned_with_converter_reading_non_nullable_column();
}

[ConditionalFact(Skip = "Issue#21678")]
public override void Optional_datetime_reading_null_from_database()
{
base.Optional_datetime_reading_null_from_database();
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2308,12 +2308,10 @@ FROM root c
WHERE (c[""Discriminator""] = ""Customer"")");
}

[ConditionalTheory(Skip = "Issue#13168")]
public override Task Where_bitwise_or_with_logical_or(bool async)
{
// #13168
//await base.Where_bitwise_or_with_logical_or(async);

return Task.CompletedTask;
return base.Where_bitwise_or_with_logical_or(async);
}

public override async Task Where_bitwise_and_with_logical_and(bool async)
Expand All @@ -2326,12 +2324,10 @@ FROM root c
WHERE ((c[""Discriminator""] = ""Customer"") AND (((c[""CustomerID""] = ""ALFKI"") & (c[""CustomerID""] = ""ANATR"")) AND (c[""CustomerID""] = ""ANTON"")))");
}

[ConditionalTheory(Skip = "Issue#13168")]
public override Task Where_bitwise_or_with_logical_and(bool async)
{
// #13168
//await base.Where_bitwise_or_with_logical_and(async);

return Task.CompletedTask;
return base.Where_bitwise_or_with_logical_and(async);
}

public override async Task Where_bitwise_and_with_logical_or(bool async)
Expand Down Expand Up @@ -2409,12 +2405,10 @@ FROM root c
WHERE (c[""Discriminator""] = ""Customer"")");
}

[ConditionalTheory(Skip = "Issue#13159")]
public override Task Parameter_extraction_short_circuits_1(bool async)
{
// #13159
//await base.Parameter_extraction_short_circuits_1(async);

return Task.CompletedTask;
return base.Parameter_extraction_short_circuits_1(async);
}

[ConditionalTheory(Skip = "Issue #17246")]
Expand All @@ -2428,12 +2422,10 @@ FROM root c
WHERE (c[""Discriminator""] = ""Order"")");
}

[ConditionalTheory(Skip = "Issue#13159")]
public override Task Parameter_extraction_short_circuits_3(bool async)
{
// #13159
//await base.Parameter_extraction_short_circuits_3(async);

return Task.CompletedTask;
return base.Parameter_extraction_short_circuits_3(async);
}

[ConditionalTheory(Skip = "Issue #17246")]
Expand Down Expand Up @@ -4151,24 +4143,18 @@ public override Task Select_distinct_Select_with_client_bindings(bool async)
return base.Select_distinct_Select_with_client_bindings(async);
}

[ConditionalTheory(Skip = "Issue#21678")]
public override Task Non_nullable_property_through_optional_navigation(bool async)
[ConditionalTheory(Skip = "Non embedded collection subquery Issue#17246")]
public override Task Pending_selector_in_cardinality_reducing_method_is_applied_before_expanding_collection_navigation_member(bool async)
{
return base.Non_nullable_property_through_optional_navigation(async);
return base.Pending_selector_in_cardinality_reducing_method_is_applied_before_expanding_collection_navigation_member(async);
}

[ConditionalTheory(Skip = "Issue#21678")]
[ConditionalTheory(Skip = "Non embedded collection subquery Issue#17246")]
public override Task Max_on_empty_sequence_throws(bool async)
{
return base.Max_on_empty_sequence_throws(async);
}

[ConditionalTheory(Skip = "Non embedded collection subquery Issue#17246")]
public override Task Pending_selector_in_cardinality_reducing_method_is_applied_before_expanding_collection_navigation_member(bool async)
{
return base.Pending_selector_in_cardinality_reducing_method_is_applied_before_expanding_collection_navigation_member(async);
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down

This file was deleted.

0 comments on commit e9fae4a

Please sign in to comment.