Skip to content

Commit

Permalink
Query: API review changes
Browse files Browse the repository at this point in the history
Part of #20409

Remove IDbFunction.IsQueryable
Add IDbFunction.IsScalar
Add IDbFunction.IsAggregate
Update TVF pipeline end to end to remove usage of Queryable in the name.
  • Loading branch information
smitpatel committed Apr 28, 2020
1 parent 818c93a commit 4e04362
Show file tree
Hide file tree
Showing 23 changed files with 143 additions and 175 deletions.
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 entity type 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
29 changes: 18 additions & 11 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,22 @@ public static DbFunction RemoveDbFunction(
public virtual Type ReturnType { get; }

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

/// <inheritdoc />
public virtual bool IsAggregate { 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
public virtual IEntityType ReturnEntityType
{
get
{
if (!IsQueryable)
if (IsScalar)
{
return null;
}
Expand Down Expand Up @@ -475,9 +476,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 +534,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

0 comments on commit 4e04362

Please sign in to comment.