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

Query: API review changes #20772

Merged
merged 1 commit into from
Apr 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ protected virtual void ValidateDbFunctions(
var methodInfo = dbFunction.MethodInfo;

if (dbFunction.TypeMapping == null
&& dbFunction.QueryableEntityType == null)
&& dbFunction.ReturnEntityType == null)
{
throw new InvalidOperationException(
RelationalStrings.DbFunctionInvalidReturnType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,20 @@ public virtual void ProcessModelFinalizing(

foreach (var dbFunction in modelBuilder.Metadata.GetDbFunctions())
{
// TODO: This check needs to be updated to skip over enumerable parameter of aggregate.
foreach (var parameter in dbFunction.Parameters)
{
parameter.Builder.HasTypeMapping(!string.IsNullOrEmpty(parameter.StoreType)
? _relationalTypeMappingSource.FindMapping(parameter.StoreType)
: _relationalTypeMappingSource.FindMapping(parameter.ClrType));
}

if (dbFunction.IsQueryable)
if (dbFunction.IsScalar)
{
continue;
dbFunction.Builder.HasTypeMapping(!string.IsNullOrEmpty(dbFunction.StoreType)
? _relationalTypeMappingSource.FindMapping(dbFunction.StoreType)
: _relationalTypeMappingSource.FindMapping(dbFunction.ReturnType));
}

dbFunction.Builder.HasTypeMapping(!string.IsNullOrEmpty(dbFunction.StoreType)
? _relationalTypeMappingSource.FindMapping(dbFunction.StoreType)
: _relationalTypeMappingSource.FindMapping(dbFunction.ReturnType));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public override ConventionSet CreateConventionSet()
ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, valueGenerationConvention);

conventionSet.PropertyFieldChangedConventions.Add(relationalColumnAttributeConvention);
conventionSet.PropertyFieldChangedConventions.Add(relationalCommentAttributeConvention);
conventionSet.PropertyFieldChangedConventions.Add(relationalCommentAttributeConvention);

var storeGenerationConvention = new StoreGenerationConvention(Dependencies, RelationalDependencies);
conventionSet.PropertyAnnotationChangedConventions.Add(storeGenerationConvention);
Expand All @@ -98,10 +98,10 @@ public override ConventionSet CreateConventionSet()
conventionSet.ModelFinalizingConventions,
new TableSharingConcurrencyTokenConvention(Dependencies, RelationalDependencies),
typeof(TypeMappingConvention));
// ModelCleanupConvention would remove the entity types added by QueryableDbFunctionConvention #15898
// ModelCleanupConvention would remove the entity types added by TableValuedDbFunctionConvention #15898
ConventionSet.AddAfter(
conventionSet.ModelFinalizingConventions,
new QueryableDbFunctionConvention(Dependencies, RelationalDependencies),
new TableValuedDbFunctionConvention(Dependencies, RelationalDependencies),
typeof(ModelCleanupConvention));
conventionSet.ModelFinalizingConventions.Add(dbFunctionAttributeConvention);
conventionSet.ModelFinalizingConventions.Add(tableNameFromDbSetConvention);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
/// <summary>
/// A convention that configures the entity type to which a queryable function is mapped.
/// </summary>
public class QueryableDbFunctionConvention : IModelFinalizingConvention
public class TableValuedDbFunctionConvention : IModelFinalizingConvention
{
/// <summary>
/// Creates a new instance of <see cref="RelationalDbFunctionAttributeConvention" />.
/// Creates a new instance of <see cref="TableValuedDbFunctionConvention" />.
/// </summary>
/// <param name="dependencies"> Parameter object containing dependencies for this convention. </param>
/// <param name="relationalDependencies"> Parameter object containing relational dependencies for this convention. </param>
public QueryableDbFunctionConvention(
public TableValuedDbFunctionConvention(
[NotNull] ProviderConventionSetBuilderDependencies dependencies,
[NotNull] RelationalConventionSetBuilderDependencies relationalDependencies)
{
Expand Down Expand Up @@ -51,7 +51,7 @@ private void ProcessDbFunctionAdded(
[NotNull] IConventionDbFunctionBuilder dbFunctionBuilder, [NotNull] IConventionContext context)
{
var function = dbFunctionBuilder.Metadata;
if (!function.IsQueryable)
if (function.IsScalar)
{
return;
}
Expand Down
23 changes: 14 additions & 9 deletions src/EFCore.Relational/Metadata/IDbFunction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,24 @@ public interface IDbFunction : IAnnotatable
MethodInfo MethodInfo { get; }

/// <summary>
/// Gets the value indicating whether this method returns IQueryable
/// Gets the value indicating whether this function returns scalar value.
/// </summary>
bool IsQueryable { get; }
bool IsScalar { get; }

/// <summary>
/// Gets the entity type returned by this queryable function
/// Gets the value indicating whether this function is an aggregate function.
/// </summary>
IEntityType QueryableEntityType => IsQueryable
? Model.FindEntityType(ReturnType.GetGenericArguments()[0])
: null;
bool IsAggregate { get; }

/// <summary>
/// Gets the configured store type string
/// Gets the <see cref="IEntityType"/> returned by this table valued function.
/// </summary>
IEntityType ReturnEntityType => IsScalar
? null
: Model.FindEntityType(ReturnType.GetGenericArguments()[0]);

/// <summary>
/// Gets the configured store type string.
/// </summary>
string StoreType { get; }

Expand All @@ -63,12 +68,12 @@ public interface IDbFunction : IAnnotatable
Type ReturnType { get; }

/// <summary>
/// Gets the type mapping for the function's return type
/// Gets the type mapping for the function's return type.
/// </summary>
RelationalTypeMapping TypeMapping { get; }

/// <summary>
/// Gets the parameters for this function
/// Gets the parameters for this function.
/// </summary>
IReadOnlyList<IDbFunctionParameter> Parameters { get; }

Expand Down
35 changes: 18 additions & 17 deletions src/EFCore.Relational/Metadata/Internal/DbFunction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,9 @@ public DbFunction(
name, returnType.ShortDisplayName()));
}

if (returnType.IsGenericType
&& returnType.GetGenericTypeDefinition() == typeof(IQueryable<>))
{
IsQueryable = true;
}
IsScalar = !returnType.IsGenericType
|| returnType.GetGenericTypeDefinition() != typeof(IQueryable<>);
IsAggregate = false;

ModelName = name;
ReturnType = returnType;
Expand Down Expand Up @@ -257,19 +255,16 @@ public static DbFunction RemoveDbFunction(
public virtual Type ReturnType { get; }

/// <inheritdoc />
public virtual bool IsQueryable { get; }
public virtual bool IsScalar { get; }

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
IEntityType QueryableEntityType
/// <inheritdoc />
public virtual bool IsAggregate { get; }

private IEntityType ReturnEntityType
{
get
{
if (!IsQueryable)
if (IsScalar)
{
return null;
}
Expand Down Expand Up @@ -475,9 +470,15 @@ public virtual Func<IReadOnlyCollection<SqlExpression>, SqlExpression> SetTransl
ConfigurationSource configurationSource)
{
if (translation != null
&& IsQueryable)
&& !IsScalar)
{
throw new InvalidOperationException(RelationalStrings.DbFunctionTableValuedCustomTranslation(MethodInfo.DisplayName()));
}

if (translation != null
&& IsAggregate)
{
throw new InvalidOperationException(RelationalStrings.DbFunctionQueryableCustomTranslation(MethodInfo.DisplayName()));
throw new InvalidOperationException(RelationalStrings.DbFunctionAggregateCustomTranslation(MethodInfo.DisplayName()));
}

_translation = translation;
Expand Down Expand Up @@ -527,7 +528,7 @@ IConventionModel IConventionDbFunction.Model
}

/// <inheritdoc />
IEntityType IDbFunction.QueryableEntityType => QueryableEntityType;
IEntityType IDbFunction.ReturnEntityType => ReturnEntityType;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public virtual IConventionDbFunctionBuilder HasTranslation(
/// </summary>
public virtual bool CanSetTranslation(
[CanBeNull] Func<IReadOnlyCollection<SqlExpression>, SqlExpression> translation, ConfigurationSource configurationSource)
=> (!Metadata.IsQueryable || configurationSource == ConfigurationSource.Explicit)
=> ((Metadata.IsScalar && !Metadata.IsAggregate) || configurationSource == ConfigurationSource.Explicit)
&& (configurationSource.Overrides(Metadata.GetTranslationConfigurationSource())
|| Metadata.Translation == translation);

Expand Down
30 changes: 19 additions & 11 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,8 @@
<data name="DatabaseModelMissing" xml:space="preserve">
<value>The database model hasn't been initialized. The model needs to be finalized before the database model can be accessed.</value>
</data>
<data name="DbFunctionQueryableCustomTranslation" xml:space="preserve">
<value>Cannot set custom translation on the DbFunction '{function}' since it returns IQueryable type.</value>
<data name="DbFunctionTableValuedCustomTranslation" xml:space="preserve">
<value>Cannot set custom translation on the DbFunction '{function}' since it is a table valued function.</value>
</data>
<data name="DbFunctionInvalidIQueryableReturnType" xml:space="preserve">
<value>The DbFunction '{function}' has an invalid return type '{type}'. Only functions that return IQueryable of entity type are supported.</value>
Expand Down Expand Up @@ -581,4 +581,7 @@
<data name="UnsupportedDataOperationStoreType" xml:space="preserve">
<value>The store type '{type}' used for the column '{column}' in a migration data operation is not supported by the current provider.</value>
</data>
<data name="DbFunctionAggregateCustomTranslation" xml:space="preserve">
<value>Cannot set custom translation on the DbFunction '{function}' since it is an aggregate function.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@

namespace Microsoft.EntityFrameworkCore.Query.Internal
{
public class QueryableFunctionQueryRootExpression : QueryRootExpression
public class TableValuedFunctionQueryRootExpression : QueryRootExpression
{
//Since this is always generated while compiling there is no query provider associated
public QueryableFunctionQueryRootExpression(
public TableValuedFunctionQueryRootExpression(
[NotNull] IEntityType entityType, [NotNull] IDbFunction function, [NotNull] IReadOnlyCollection<Expression> arguments)
: base(entityType)
{
Expand All @@ -41,7 +41,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
}

return changed
? new QueryableFunctionQueryRootExpression(EntityType, Function, arguments)
? new TableValuedFunctionQueryRootExpression(EntityType, Function, arguments)
: this;
}

Expand All @@ -56,10 +56,10 @@ public override void Print(ExpressionPrinter expressionPrinter)
public override bool Equals(object obj)
=> obj != null
&& (ReferenceEquals(this, obj)
|| obj is QueryableFunctionQueryRootExpression queryRootExpression
|| obj is TableValuedFunctionQueryRootExpression queryRootExpression
&& Equals(queryRootExpression));

private bool Equals(QueryableFunctionQueryRootExpression queryRootExpression)
private bool Equals(TableValuedFunctionQueryRootExpression queryRootExpression)
=> base.Equals(queryRootExpression)
&& Equals(Function, queryRootExpression.Function)
&& Arguments.SequenceEqual(queryRootExpression.Arguments, ExpressionEqualityComparer.Instance);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

namespace Microsoft.EntityFrameworkCore.Query.Internal
{
public class QueryableFunctionToQueryRootConvertingExpressionVisitor : ExpressionVisitor
public class TableValuedFunctionToQueryRootConvertingExpressionVisitor : ExpressionVisitor
{
private readonly IModel _model;

public QueryableFunctionToQueryRootConvertingExpressionVisitor([NotNull] IModel model)
public TableValuedFunctionToQueryRootConvertingExpressionVisitor([NotNull] IModel model)
{
Check.NotNull(model, nameof(model));

Expand All @@ -24,13 +24,13 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
{
var function = _model.FindDbFunction(methodCallExpression.Method);

return function?.IsQueryable == true
? CreateQueryableFunctionQueryRootExpression(function, methodCallExpression.Arguments)
return function?.IsScalar == false
? CreateTableValuedFunctionQueryRootExpression(function, methodCallExpression.Arguments)
: base.VisitMethodCall(methodCallExpression);
}

private Expression CreateQueryableFunctionQueryRootExpression(
private Expression CreateTableValuedFunctionQueryRootExpression(
IDbFunction function, IReadOnlyCollection<Expression> arguments)
=> new QueryableFunctionQueryRootExpression(function.QueryableEntityType, function, arguments);
=> new TableValuedFunctionQueryRootExpression(function.ReturnEntityType, function, arguments);
}
}
Loading