Skip to content

Commit

Permalink
Query: Add builtIn flag to SqlFunction and use it to quoting logic
Browse files Browse the repository at this point in the history 
Fuctions without arguments passed considered niladic.
Non-niladic functions without arguments must pass empty array of args.
Functions with instance are considered built in.
Functions without schema are considered built in.
Functions with schema are non-built-in (Schema can be null)
To map a DbFunction to built-in function use HasTranslation API

Resolves #12757
Resolves #14882
Resolves #15501
  • Loading branch information
@smitpatel
smitpatel committed Jun 19, 2019
1 parent 674a991 commit 1fc7152
Show file tree
Hide file tree
Showing 20 changed files with 77 additions and 187 deletions.
1 change: 0 additions & 1 deletion src/EFCore.Cosmos/Query/Pipeline/ISqlExpressionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public interface ISqlExpressionFactory
{
SqlExpression ApplyTypeMapping(SqlExpression sqlExpression, CoreTypeMapping typeMapping);
SqlExpression ApplyDefaultTypeMapping(SqlExpression sqlExpression);
//CoreTypeMapping GetTypeMappingForValue(object value);
CoreTypeMapping FindMapping(Type type);

SqlBinaryExpression MakeBinary(ExpressionType operatorType, SqlExpression left, SqlExpression right, CoreTypeMapping typeMapping);
Expand Down
10 changes: 1 addition & 9 deletions src/EFCore.Relational/Metadata/Internal/DbFunction.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,6 @@ public virtual ConfigurationSource GetConfigurationSource()
public virtual void UpdateConfigurationSource(ConfigurationSource configurationSource)
=> ((Model)_model).FindAnnotation(_annotationName).UpdateConfigurationSource(configurationSource);

/// <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>
public virtual string DefaultSchema { get; [param: CanBeNull] set; }

/// <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
Expand All @@ -144,7 +136,7 @@ public virtual void UpdateConfigurationSource(ConfigurationSource configurationS
/// </summary>
public virtual string Schema
{
get => _schema ?? _model.GetDefaultSchema() ?? DefaultSchema;
get => _schema ?? _model.GetDefaultSchema();
set => SetSchema(value, ConfigurationSource.Explicit);
}

Expand Down
6 changes: 3 additions & 3 deletions src/EFCore.Relational/Query/Pipeline/ISqlExpressionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ public interface ISqlExpressionFactory
SqlFunctionExpression Function(
SqlExpression instance, string functionName, IEnumerable<SqlExpression> arguments, Type returnType, RelationalTypeMapping typeMapping = null);
SqlFunctionExpression Function(
string functionName, bool niladic, Type returnType, RelationalTypeMapping typeMapping = null);
string functionName, Type returnType, RelationalTypeMapping typeMapping = null);
SqlFunctionExpression Function(
string schema, string functionName, bool niladic, Type returnType, RelationalTypeMapping typeMapping = null);
string schema, string functionName, Type returnType, RelationalTypeMapping typeMapping = null);
SqlFunctionExpression Function(
SqlExpression instance, string functionName, bool niladic, Type returnType, RelationalTypeMapping typeMapping = null);
SqlExpression instance, string functionName, Type returnType, RelationalTypeMapping typeMapping = null);

ExistsExpression Exists(SelectExpression subquery, bool negated);
InExpression In(SqlExpression item, SqlExpression values, bool negated);
Expand Down
21 changes: 13 additions & 8 deletions src/EFCore.Relational/Query/Pipeline/QuerySqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,7 @@ protected override Expression VisitProjection(ProjectionExpression projectionExp

protected override Expression VisitSqlFunction(SqlFunctionExpression sqlFunctionExpression)
{
if (!string.IsNullOrEmpty(sqlFunctionExpression.Schema))
{
_relationalCommandBuilder
.Append(_sqlGenerationHelper.DelimitIdentifier(sqlFunctionExpression.Schema))
.Append(".")
.Append(_sqlGenerationHelper.DelimitIdentifier(sqlFunctionExpression.FunctionName));
}
else
if (sqlFunctionExpression.IsBuiltIn)
{
if (sqlFunctionExpression.Instance != null)
{
Expand All @@ -180,6 +173,18 @@ protected override Expression VisitSqlFunction(SqlFunctionExpression sqlFunction

_relationalCommandBuilder.Append(sqlFunctionExpression.FunctionName);
}
else
{
if (!string.IsNullOrEmpty(sqlFunctionExpression.Schema))
{
_relationalCommandBuilder
.Append(_sqlGenerationHelper.DelimitIdentifier(sqlFunctionExpression.Schema))
.Append(".");
}

_relationalCommandBuilder
.Append(_sqlGenerationHelper.DelimitIdentifier(sqlFunctionExpression.FunctionName));
}

if (!sqlFunctionExpression.IsNiladic)
{
Expand Down
13 changes: 6 additions & 7 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressionFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,6 @@ public CaseExpression Case(IReadOnlyList<CaseWhenClause> whenClauses, SqlExpress
string schema, string functionName, IEnumerable<SqlExpression> arguments, Type returnType, RelationalTypeMapping typeMapping = null)
{
var typeMappedArguments = new List<SqlExpression>();

foreach (var argument in arguments)
{
typeMappedArguments.Add(ApplyDefaultTypeMapping(argument));
Expand Down Expand Up @@ -439,22 +438,22 @@ public CaseExpression Case(IReadOnlyList<CaseWhenClause> whenClauses, SqlExpress
}

public SqlFunctionExpression Function(
string functionName, bool niladic, Type returnType, RelationalTypeMapping typeMapping = null)
string functionName, Type returnType, RelationalTypeMapping typeMapping = null)
{
return new SqlFunctionExpression(functionName, niladic, returnType, typeMapping);
return new SqlFunctionExpression(functionName, returnType, typeMapping);
}

public SqlFunctionExpression Function(
string schema, string functionName, bool niladic, Type returnType, RelationalTypeMapping typeMapping = null)
string schema, string functionName, Type returnType, RelationalTypeMapping typeMapping = null)
{
return new SqlFunctionExpression(schema, functionName, niladic, returnType, typeMapping);
return new SqlFunctionExpression(schema, functionName, returnType, typeMapping);
}

public SqlFunctionExpression Function(
SqlExpression instance, string functionName, bool niladic, Type returnType, RelationalTypeMapping typeMapping = null)
SqlExpression instance, string functionName, Type returnType, RelationalTypeMapping typeMapping = null)
{
instance = ApplyDefaultTypeMapping(instance);
return new SqlFunctionExpression(instance, functionName, niladic, returnType, typeMapping);
return new SqlFunctionExpression(instance, functionName, returnType, typeMapping);
}

public ExistsExpression Exists(SelectExpression subquery, bool negated)
Expand Down
29 changes: 17 additions & 12 deletions src/EFCore.Relational/Query/Pipeline/SqlExpressions/SqlFunctionExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,61 +12,61 @@ namespace Microsoft.EntityFrameworkCore.Relational.Query.Pipeline.SqlExpressions
{
public class SqlFunctionExpression : SqlExpression
{
// niladic
public SqlFunctionExpression(
string functionName,
bool niladic,
Type type,
RelationalTypeMapping typeMapping)
: this(null, null, functionName, niladic, null, type, typeMapping)
: this(instance: null, schema: null, functionName, niladic: true, arguments: null, builtIn: true, type, typeMapping)
{
}

// niladic
public SqlFunctionExpression(
string schema,
string functionName,
bool niladic,
Type type,
RelationalTypeMapping typeMapping)
: this(null, schema, functionName, niladic, null, type, typeMapping)
: this(instance: null, schema, functionName, niladic: true, arguments: null, builtIn: true, type, typeMapping)
{
}

// niladic
public SqlFunctionExpression(
SqlExpression instance,
string functionName,
bool niladic,
Type type,
RelationalTypeMapping typeMapping)
: this(instance, null, functionName, niladic, null, type, typeMapping)
: this(instance, schema: null, functionName, niladic: true, arguments: null, builtIn: true, type, typeMapping)
{
}

public SqlFunctionExpression(
SqlExpression instance,
string functionName,
IEnumerable<SqlExpression> arguments,
Type type,
RelationalTypeMapping typeMapping)
: this(null, null, functionName, false, arguments, type, typeMapping)
: this(instance, schema: null, functionName, niladic: false, arguments, builtIn: true, type, typeMapping)
{
}

public SqlFunctionExpression(
string schema,
string functionName,
IEnumerable<SqlExpression> arguments,
Type type,
RelationalTypeMapping typeMapping)
: this(null, schema, functionName, false, arguments, type, typeMapping)
: this(instance: null, schema: null, functionName, niladic: false, arguments, builtIn: true, type, typeMapping)
{
}

public SqlFunctionExpression(
SqlExpression instance,
string schema,
string functionName,
IEnumerable<SqlExpression> arguments,
Type type,
RelationalTypeMapping typeMapping)
: this(instance, null, functionName, false, arguments, type, typeMapping)
: this(instance: null, schema, functionName, niladic: false, arguments, builtIn: false, type, typeMapping)
{
}

Expand All @@ -76,6 +76,7 @@ public class SqlFunctionExpression : SqlExpression
string functionName,
bool niladic,
IEnumerable<SqlExpression> arguments,
bool builtIn,
Type type,
RelationalTypeMapping typeMapping)
: base(type, typeMapping)
Expand All @@ -84,12 +85,14 @@ public class SqlFunctionExpression : SqlExpression
FunctionName = functionName;
Schema = schema;
IsNiladic = niladic;
IsBuiltIn = builtIn;
Arguments = (arguments ?? Array.Empty<SqlExpression>()).ToList();
}

public string FunctionName { get; }
public string Schema { get; }
public bool IsNiladic { get; }
public bool IsBuiltIn { get; }
public IReadOnlyList<SqlExpression> Arguments { get; }
public Expression Instance { get; }

Expand All @@ -112,6 +115,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
FunctionName,
IsNiladic,
arguments,
IsBuiltIn,
Type,
TypeMapping)
: this;
Expand All @@ -124,12 +128,13 @@ public SqlFunctionExpression ApplyTypeMapping(RelationalTypeMapping typeMapping)
FunctionName,
IsNiladic,
Arguments,
IsBuiltIn,
Type,
typeMapping ?? TypeMapping);

public SqlFunctionExpression Update(SqlExpression instance, IReadOnlyList<SqlExpression> arguments)
=> instance != Instance || !arguments.SequenceEqual(Arguments)
? new SqlFunctionExpression(instance, Schema, FunctionName, IsNiladic, arguments, Type, TypeMapping)
? new SqlFunctionExpression(instance, Schema, FunctionName, IsNiladic, arguments, IsBuiltIn, Type, TypeMapping)
: this;

public override void Print(ExpressionPrinter expressionPrinter)
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.SqlServer.NTS/Query/Pipeline/SqlServerGeometryCollectionMemberTranslator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
return _sqlExpressionFactory.Function(
instance,
"STNumGeometries",
false,
Array.Empty<SqlExpression>(),
returnType);
}

Expand Down
5 changes: 2 additions & 3 deletions src/EFCore.SqlServer.NTS/Query/Pipeline/SqlServerGeometryMemberTranslator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
return _sqlExpressionFactory.Function(
instance,
functionName,
false,
Array.Empty<SqlExpression>(),
returnType,
resultTypeMapping);
}
Expand Down Expand Up @@ -98,7 +98,7 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
_sqlExpressionFactory.Function(
instance,
"STGeometryType",
false,
Array.Empty<SqlExpression>(),
typeof(string)),
whenClauses.ToArray());
}
Expand All @@ -108,7 +108,6 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
return _sqlExpressionFactory.Function(
instance,
"STSrid",
true,
returnType);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.SqlServer.NTS/Query/Pipeline/SqlServerLineStringMemberTranslator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
return _sqlExpressionFactory.Function(
instance,
"STNumPoints",
false,
Array.Empty<SqlExpression>(),
returnType);
}

Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.SqlServer.NTS/Query/Pipeline/SqlServerMultiCurveMemberTranslator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
return _sqlExpressionFactory.Function(
instance,
"STIsClosed",
false,
Array.Empty<SqlExpression>(),
returnType);
}

Expand Down
1 change: 0 additions & 1 deletion src/EFCore.SqlServer.NTS/Query/Pipeline/SqlServerPointMemberTranslator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
return _sqlExpressionFactory.Function(
instance,
propertyName,
true,
returnType);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.SqlServer.NTS/Query/Pipeline/SqlServerPolygonMemberTranslator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
_sqlExpressionFactory.Function(
instance,
"NumRings",
false,
Array.Empty<SqlExpression>(),
returnType),
_sqlExpressionFactory.Constant(1));
}
Expand All @@ -75,7 +75,7 @@ public SqlExpression Translate(SqlExpression instance, MemberInfo member, Type r
return _sqlExpressionFactory.Function(
instance,
functionName,
false,
Array.Empty<SqlExpression>(),
returnType,
resultTypeMapping);
}
Expand Down
4 changes: 0 additions & 4 deletions src/EFCore.SqlServer/Metadata/Conventions/SqlServerConventionSetBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,6 @@ public override ConventionSet CreateConventionSet()
ReplaceConvention(
conventionSet.PropertyAnnotationChangedConventions, (RelationalValueGenerationConvention)valueGenerationConvention);

ReplaceConvention(
conventionSet.ModelAnnotationChangedConventions,
(RelationalDbFunctionAttributeConvention)new SqlServerDbFunctionAttributeConvention(Dependencies, RelationalDependencies));

ReplaceConvention(conventionSet.ModelFinalizedConventions, storeGenerationConvention);

return conventionSet;
Expand Down
43 changes: 0 additions & 43 deletions src/EFCore.SqlServer/Metadata/Conventions/SqlServerDbFunctionAttributeConvention.cs
View file

This file was deleted.

2 changes: 1 addition & 1 deletion src/EFCore.SqlServer/Metadata/Conventions/SqlServerIndexConvention.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class SqlServerIndexConvention :
private readonly ISqlGenerationHelper _sqlGenerationHelper;

/// <summary>
/// Creates a new instance of <see cref="SqlServerDbFunctionAttributeConvention" />.
/// Creates a new instance of <see cref="SqlServerIndexConvention" />.
/// </summary>
/// <param name="dependencies"> Parameter object containing dependencies for this convention. </param>
/// <param name="relationalDependencies"> Parameter object containing relational dependencies for this convention. </param>
Expand Down
Loading

0 comments on commit 1fc7152

Please sign in to comment.