Skip to content

Commit

Permalink
Generalize type mapping support for Unicode
Browse files Browse the repository at this point in the history 
In looking at #4425 and #4134 in became quickly apparent that the support for inferring Unicode was too specific to Unicode. What is really needed is that if the type mapping to be used can be inferred in some way, then this type mapping should be used for all relevant facets, not just Unicode. Only in cases where no inference is possible should the default mapping for the CLR type be used.
  • Loading branch information
@ajcvickers
ajcvickers committed May 9, 2016
1 parent 2a7c122 commit d7dce10
Show file tree
Hide file tree
Showing 16 changed files with 96 additions and 103 deletions.
66 changes: 35 additions & 31 deletions src/Microsoft.EntityFrameworkCore.Relational/Query/Sql/DefaultQuerySqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Linq.Expressions;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Query.Expressions;
using Microsoft.EntityFrameworkCore.Query.ExpressionVisitors.Internal;
using Microsoft.EntityFrameworkCore.Storage;
Expand All @@ -20,8 +21,6 @@ namespace Microsoft.EntityFrameworkCore.Query.Sql
{
public class DefaultQuerySqlGenerator : ThrowingExpressionVisitor, ISqlExpressionVisitor, IQuerySqlGenerator
{
private const bool DefaultUnicodeBehaviour = true;

private readonly IRelationalCommandBuilderFactory _relationalCommandBuilderFactory;
private readonly ISqlGenerationHelper _sqlGenerationHelper;
private readonly IParameterNameGeneratorFactory _parameterNameGeneratorFactory;
Expand All @@ -30,7 +29,7 @@ public class DefaultQuerySqlGenerator : ThrowingExpressionVisitor, ISqlExpressio
private IRelationalCommandBuilder _relationalCommandBuilder;
private IReadOnlyDictionary<string, object> _parametersValues;
private ParameterNameGenerator _parameterNameGenerator;
private bool _isUnicode;
private RelationalTypeMapping _typeMapping;

private static readonly Dictionary<ExpressionType, string> _binaryOperatorMap = new Dictionary<ExpressionType, string>
{
Expand Down Expand Up @@ -67,7 +66,6 @@ public DefaultQuerySqlGenerator(
_relationalTypeMapper = relationalTypeMapper;

SelectExpression = selectExpression;
_isUnicode = DefaultUnicodeBehaviour;
}

public virtual bool IsCacheable { get; private set; }
Expand Down Expand Up @@ -379,7 +377,8 @@ protected virtual void GenerateFromSql(

for (var i = 0; i < argumentValues.Length; i++)
{
substitutions[i] = SqlGenerator.GenerateLiteral(argumentValues[i], _isUnicode);
var value = argumentValues[i];
substitutions[i] = SqlGenerator.GenerateLiteral(value, GetTypeMapping(value));
}

break;
Expand All @@ -399,9 +398,10 @@ protected virtual void GenerateFromSql(
{
case ExpressionType.Constant:
{
var value = ((ConstantExpression)expression).Value;
substitutions[i]
= SqlGenerator
.GenerateLiteral(((ConstantExpression)expression).Value, _isUnicode);
.GenerateLiteral(value, GetTypeMapping(value));

break;
}
Expand Down Expand Up @@ -436,6 +436,9 @@ protected virtual void GenerateFromSql(
_relationalCommandBuilder.AppendLines(sql);
}

private RelationalTypeMapping GetTypeMapping(object value)
=> _typeMapping ?? _relationalTypeMapper.GetMappingForValue(value);

public virtual Expression VisitTable(TableExpression tableExpression)
{
Check.NotNull(tableExpression, nameof(tableExpression));
Expand Down Expand Up @@ -553,8 +556,8 @@ var relationalNullsInExpression

if (inValuesNotNull.Count > 0)
{
var parentIsUnicode = _isUnicode;
_isUnicode = InferUnicodeFromColumn(inExpression.Operand) ?? _isUnicode;
var parentTypeMapping = _typeMapping;
_typeMapping = InferTypeMappingFromColumn(inExpression.Operand) ?? parentTypeMapping;

Visit(inExpression.Operand);

Expand All @@ -564,7 +567,7 @@ var relationalNullsInExpression

_relationalCommandBuilder.Append(")");

_isUnicode = parentIsUnicode;
_typeMapping = parentTypeMapping;
}
else
{
Expand All @@ -573,16 +576,16 @@ var relationalNullsInExpression
}
else
{
var parentIsUnicode = _isUnicode;
_isUnicode = InferUnicodeFromColumn(inExpression.Operand) ?? _isUnicode;
var parentTypeMapping = _typeMapping;
_typeMapping = InferTypeMappingFromColumn(inExpression.Operand) ?? parentTypeMapping;

Visit(inExpression.Operand);

_relationalCommandBuilder.Append(" IN ");

Visit(inExpression.SubQuery);

_isUnicode = parentIsUnicode;
_typeMapping = parentTypeMapping;
}

return inExpression;
Expand Down Expand Up @@ -886,12 +889,15 @@ protected override Expression VisitBinary(BinaryExpression expression)
}
else
{
var parentUnicodeBehaviour = _isUnicode;
var parentTypeMapping = _typeMapping;

if (expression.IsComparisonOperation()
|| (expression.NodeType == ExpressionType.Add))
{
_isUnicode = InferUnicodeFromColumn(expression.Left) ?? InferUnicodeFromColumn(expression.Right) ?? _isUnicode;
_typeMapping
= InferTypeMappingFromColumn(expression.Left)
?? InferTypeMappingFromColumn(expression.Right)
?? parentTypeMapping;
}

var needParens = expression.Left is BinaryExpression;
Expand Down Expand Up @@ -955,7 +961,7 @@ protected override Expression VisitBinary(BinaryExpression expression)
_relationalCommandBuilder.Append(TrueLiteral);
}

_isUnicode = parentUnicodeBehaviour;
_typeMapping = parentTypeMapping;
}

return expression;
Expand Down Expand Up @@ -1020,16 +1026,16 @@ public virtual Expression VisitLike(LikeExpression likeExpression)
{
Check.NotNull(likeExpression, nameof(likeExpression));

var parentIsUnicode = _isUnicode;
_isUnicode = InferUnicodeFromColumn(likeExpression.Match) ?? _isUnicode;
var parentTypeMapping = _typeMapping;
_typeMapping = InferTypeMappingFromColumn(likeExpression.Match) ?? parentTypeMapping;

Visit(likeExpression.Match);

_relationalCommandBuilder.Append(" LIKE ");

Visit(likeExpression.Pattern);

_isUnicode = parentIsUnicode;
_typeMapping = parentTypeMapping;

return likeExpression;
}
Expand All @@ -1038,7 +1044,8 @@ public virtual Expression VisitLiteral(LiteralExpression literalExpression)
{
Check.NotNull(literalExpression, nameof(literalExpression));

_relationalCommandBuilder.Append(_sqlGenerationHelper.GenerateLiteral(literalExpression.Literal, _isUnicode));
var value = literalExpression.Literal;
_relationalCommandBuilder.Append(_sqlGenerationHelper.GenerateLiteral(value, GetTypeMapping(value)));

return literalExpression;
}
Expand Down Expand Up @@ -1144,9 +1151,10 @@ protected override Expression VisitConstant(ConstantExpression expression)
{
Check.NotNull(expression, nameof(expression));

_relationalCommandBuilder.Append(expression.Value == null
var value = expression.Value;
_relationalCommandBuilder.Append(value == null
? "NULL"
: _sqlGenerationHelper.GenerateLiteral(expression.Value, _isUnicode));
: _sqlGenerationHelper.GenerateLiteral(value, GetTypeMapping(value)));

return expression;
}
Expand All @@ -1163,8 +1171,8 @@ protected override Expression VisitParameter(ParameterExpression parameterExpres
_relationalCommandBuilder.AddParameter(
parameterExpression.Name,
parameterName,
parameterExpression.Type,
unicode: _isUnicode);
_typeMapping ?? _relationalTypeMapper.GetMapping(parameterExpression.Type),
parameterExpression.Type.IsNullableType());
}

_relationalCommandBuilder.Append(parameterName);
Expand Down Expand Up @@ -1192,16 +1200,12 @@ var parameterName
return propertyParameterExpression;
}

protected virtual bool? InferUnicodeFromColumn([NotNull] Expression expression)
protected virtual RelationalTypeMapping InferTypeMappingFromColumn([NotNull] Expression expression)
{
var column = expression.TryGetColumnExpression();
if (column?.Property != null)
{
var typeMapping = _relationalTypeMapper.FindMapping(column.Property);
return typeMapping.IsUnicode;
}

return null;
return column?.Property != null
? _relationalTypeMapper.FindMapping(column.Property)
: null;
}

protected virtual bool TryGenerateBinaryOperator(ExpressionType op, [NotNull] out string result)
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.EntityFrameworkCore.Relational/Storage/IRelationalParameterBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ void AddParameter(
void AddParameter(
[NotNull] string invariantName,
[NotNull] string name,
[NotNull] Type type,
bool unicode);
[NotNull] RelationalTypeMapping typeMapping,
bool nullable);

void AddParameter(
[NotNull] string invariantName,
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.EntityFrameworkCore.Relational/Storage/IRelationalTypeMapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.EntityFrameworkCore.Storage
public interface IRelationalTypeMapper
{
RelationalTypeMapping FindMapping([NotNull] IProperty property);
RelationalTypeMapping FindMapping([NotNull] Type clrType, bool unicode = true);
RelationalTypeMapping FindMapping([NotNull] Type clrType);

RelationalTypeMapping FindMapping([NotNull] string typeName);
void ValidateTypeName([NotNull] string typeName);
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.EntityFrameworkCore.Relational/Storage/ISqlGenerationHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ public interface ISqlGenerationHelper

void GenerateParameterName([NotNull] StringBuilder builder, [NotNull] string name);

string GenerateLiteral([CanBeNull] object value, bool unicode = true);
string GenerateLiteral([CanBeNull] object value, [CanBeNull] RelationalTypeMapping typeMapping = null);

void GenerateLiteral([NotNull] StringBuilder builder, [CanBeNull] object value, bool unicode = true);
void GenerateLiteral([NotNull] StringBuilder builder, [CanBeNull] object value, [CanBeNull] RelationalTypeMapping typeMapping = null);

string EscapeLiteral([NotNull] string literal);

Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.EntityFrameworkCore.Relational/Storage/Internal/RelationalParameterBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,18 @@ public virtual void AddParameter(string invariantName, string name)
Check.NotEmpty(name, nameof(name)),
TypeMapper));

public virtual void AddParameter(string invariantName, string name, Type type, bool unicode)
public virtual void AddParameter(string invariantName, string name, RelationalTypeMapping typeMapping, bool nullable)
{
Check.NotEmpty(invariantName, nameof(invariantName));
Check.NotEmpty(name, nameof(name));
Check.NotNull(type, nameof(type));
Check.NotNull(typeMapping, nameof(typeMapping));

_parameters.Add(
new TypeMappedRelationalParameter(
invariantName,
name,
TypeMapper.GetMapping(type, unicode),
type.IsNullableType()));
typeMapping,
nullable));
}

public virtual void AddParameter(string invariantName, string name, IProperty property)
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.EntityFrameworkCore.Relational/Storage/RelationalCommandBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,15 @@ public static IRelationalCommandBuilder AddParameter(
[NotNull] this IRelationalCommandBuilder commandBuilder,
[NotNull] string invariantName,
[NotNull] string name,
[NotNull] Type type,
bool unicode)
[NotNull] RelationalTypeMapping typeMapping,
bool nullable)
{
Check.NotNull(commandBuilder, nameof(commandBuilder));
Check.NotEmpty(invariantName, nameof(invariantName));
Check.NotEmpty(name, nameof(name));
Check.NotNull(type, nameof(type));
Check.NotNull(typeMapping, nameof(typeMapping));

commandBuilder.ParameterBuilder.AddParameter(invariantName, name, type, unicode);
commandBuilder.ParameterBuilder.AddParameter(invariantName, name, typeMapping, nullable);

return commandBuilder;
}
Expand Down
12 changes: 6 additions & 6 deletions src/Microsoft.EntityFrameworkCore.Relational/Storage/RelationalSqlGenerationHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,24 @@ public virtual string GenerateParameterName(string name)
public virtual void GenerateParameterName(StringBuilder builder, string name)
=> builder.Append("@").Append(name);

public virtual string GenerateLiteral(object value, bool unicode = true)
public virtual string GenerateLiteral(object value, RelationalTypeMapping typeMapping = null)
{
if (value != null)
{
var s = value as string;
return s != null ? GenerateLiteralValue(s, unicode) : GenerateLiteralValue((dynamic)value);
return s != null ? GenerateLiteralValue(s, typeMapping) : GenerateLiteralValue((dynamic)value);
}
return "NULL";
}

public virtual void GenerateLiteral(StringBuilder builder, object value, bool unicode = true)
public virtual void GenerateLiteral(StringBuilder builder, object value, RelationalTypeMapping typeMapping = null)
{
if (value != null)
{
var s = value as string;
if (s != null)
{
GenerateLiteralValue(builder, s, unicode);
GenerateLiteralValue(builder, s, typeMapping);
}
else
{
Expand Down Expand Up @@ -184,10 +184,10 @@ protected virtual string GenerateLiteralValue(char value)
protected virtual void GenerateLiteralValue([NotNull] StringBuilder builder, char value)
=> builder.Append("'").Append(value).Append("'");

protected virtual string GenerateLiteralValue([NotNull] string value, bool unicode = true)
protected virtual string GenerateLiteralValue([NotNull] string value, [CanBeNull] RelationalTypeMapping typeMapping)
=> $"'{EscapeLiteral(Check.NotNull(value, nameof(value)))}'";

protected virtual void GenerateLiteralValue([NotNull] StringBuilder builder, [NotNull] string value, bool unicode = true)
protected virtual void GenerateLiteralValue([NotNull] StringBuilder builder, [NotNull] string value, [CanBeNull] RelationalTypeMapping typeMapping)
{
builder.Append("'");
EscapeLiteral(builder, value);
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.EntityFrameworkCore.Relational/Storage/RelationalTypeMapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public virtual RelationalTypeMapping FindMapping(IProperty property)
?? FindMapping(property.ClrType);
}

public virtual RelationalTypeMapping FindMapping(Type clrType, bool unicode = true)
public virtual RelationalTypeMapping FindMapping(Type clrType)
{
Check.NotNull(clrType, nameof(clrType));

Expand All @@ -69,11 +69,11 @@ public virtual RelationalTypeMapping FindMapping(string typeName)
: null;
}

protected virtual RelationalTypeMapping GetCustomMapping([NotNull] IProperty property, bool unicode = true)
protected virtual RelationalTypeMapping GetCustomMapping([NotNull] IProperty property)
{
Check.NotNull(property, nameof(property));

var mapping = FindCustomMapping(property, unicode);
var mapping = FindCustomMapping(property);

if (mapping != null)
{
Expand All @@ -83,7 +83,7 @@ protected virtual RelationalTypeMapping GetCustomMapping([NotNull] IProperty pro
throw new InvalidOperationException(RelationalStrings.UnsupportedType(property.ClrType.Name));
}

protected virtual RelationalTypeMapping FindCustomMapping([NotNull] IProperty property, bool unicode = true) => null;
protected virtual RelationalTypeMapping FindCustomMapping([NotNull] IProperty property) => null;

protected virtual bool RequiresKeyMapping([NotNull] IProperty property)
=> property.IsKey() || property.IsForeignKey();
Expand Down
5 changes: 2 additions & 3 deletions src/Microsoft.EntityFrameworkCore.Relational/Storage/RelationalTypeMapperExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@ public static RelationalTypeMapping GetMapping(

public static RelationalTypeMapping GetMapping(
[NotNull] this IRelationalTypeMapper typeMapper,
[NotNull] Type clrType,
bool unicode = true)
[NotNull] Type clrType)
{
Check.NotNull(typeMapper, nameof(typeMapper));
Check.NotNull(clrType, nameof(clrType));

var mapping = typeMapper.FindMapping(clrType, unicode);
var mapping = typeMapper.FindMapping(clrType);
if (mapping != null)
{
return mapping;
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.EntityFrameworkCore.SqlServer/Storage/Internal/SqlServerSqlGenerationHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ protected override void GenerateLiteralValue(StringBuilder builder, byte[] value
}
}

protected override string GenerateLiteralValue(string value, bool unicode = true)
=> unicode
protected override string GenerateLiteralValue(string value, RelationalTypeMapping typeMapping = null)
=> typeMapping == null || typeMapping.IsUnicode
? $"N'{EscapeLiteral(Check.NotNull(value, nameof(value)))}'"
: $"'{EscapeLiteral(Check.NotNull(value, nameof(value)))}'";

protected override void GenerateLiteralValue(StringBuilder builder, string value, bool unicode = true)
protected override void GenerateLiteralValue(StringBuilder builder, string value, RelationalTypeMapping typeMapping = null)
{
builder.Append(unicode ? "N'" : "'");
builder.Append(typeMapping.IsUnicode ? "N'" : "'");
EscapeLiteral(builder, value);
builder.Append("'");
}
Expand Down
Loading

0 comments on commit d7dce10

Please sign in to comment.