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.

Sign up for GitHub

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

Jump to bottom

Cosmos: Avoid reading default values from database #21821

Merged
1 commit merged into from
Jul 28, 2020
Merged

Cosmos: Avoid reading default values from database #21821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
378 changes: 261 additions & 117 deletions src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs
View file
Open in desktop

Large diffs are not rendered by default.

13 changes: 9 additions & 4 deletions src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
View file
Open in desktop
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
6 changes: 5 additions & 1 deletion ...edQueryCompilingExpressionVisitor.CosmosProjectionBindingRemovingExpressionVisitorBase.cs
View file
Open in desktop
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
33 changes: 32 additions & 1 deletion src/EFCore.Relational/Query/Internal/RelationalProjectionBindingExpressionVisitor.cs
View file
Open in desktop
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);
}
}
}
}
6 changes: 0 additions & 6 deletions test/EFCore.Cosmos.FunctionalTests/BuiltInDataTypesCosmosTest.cs
View file
Open in desktop
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
6 changes: 0 additions & 6 deletions test/EFCore.Cosmos.FunctionalTests/CustomConvertersCosmosTest.cs
View file
Open in desktop
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
38 changes: 12 additions & 26 deletions test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs
View file
Open in desktop
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
65 changes: 0 additions & 65 deletions test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSetOperationsQueryCosmosTest.cs
View file
Open in desktop

This file was deleted.