Skip to content

Commit 05d4071

Browse files
cincuranetcincuranet
committed
Round of feedback.
1 parent 6eff338 commit 05d4071

26 files changed

+136
-93
lines changed

Diff for: ‎src/EFCore.Relational/Query/ISqlExpressionFactory.cs

+21-4
Original file line numberDiff line numberDiff line change
@@ -418,20 +418,37 @@ SqlExpression NiladicFunction(
418418
/// Creates a new <see cref="SqlExpression" /> which represents a constant in a SQL tree.
419419
/// </summary>
420420
/// <param name="value">A value.</param>
421-
/// <param name="originallyParameter"><see langword="true" /> if the expression was originally a parameter; otherwise, <see langword="false" />.</param>
422421
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
423422
/// <returns>An expression representing a constant in a SQL tree.</returns>
424-
SqlExpression Constant(object value, bool originallyParameter = false, RelationalTypeMapping? typeMapping = null);
423+
SqlExpression Constant(object value, RelationalTypeMapping? typeMapping = null);
425424

426425
/// <summary>
427426
/// Creates a new <see cref="SqlExpression" /> which represents a constant in a SQL tree.
428427
/// </summary>
429428
/// <param name="value">A value.</param>
430429
/// <param name="type">The type for the constant. Useful when value is null.</param>
431-
/// <param name="originallyParameter"><see langword="true" /> if the expression was originally a parameter; otherwise, <see langword="false" />.</param>
432430
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
433431
/// <returns>An expression representing a constant in a SQL tree.</returns>
434-
SqlExpression Constant(object? value, Type type, bool originallyParameter = false, RelationalTypeMapping? typeMapping = null);
432+
SqlExpression Constant(object? value, Type type, RelationalTypeMapping? typeMapping = null);
433+
434+
/// <summary>
435+
/// Creates a new <see cref="SqlExpression" /> which represents a constant in a SQL tree.
436+
/// </summary>
437+
/// <param name="value">A value.</param>
438+
/// <param name="isSensitive"><see langword="true" /> if the expression contains sensitive values; otherwise, <see langword="false" />.</param>
439+
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
440+
/// <returns>An expression representing a constant in a SQL tree.</returns>
441+
SqlExpression Constant(object value, bool isSensitive, RelationalTypeMapping? typeMapping = null);
442+
443+
/// <summary>
444+
/// Creates a new <see cref="SqlExpression" /> which represents a constant in a SQL tree.
445+
/// </summary>
446+
/// <param name="value">A value.</param>
447+
/// <param name="type">The type for the constant. Useful when value is null.</param>
448+
/// <param name="isSensitive"><see langword="true" /> if the expression contains sensitive values; otherwise, <see langword="false" />.</param>
449+
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
450+
/// <returns>An expression representing a constant in a SQL tree.</returns>
451+
SqlExpression Constant(object? value, Type type, bool isSensitive, RelationalTypeMapping? typeMapping = null);
435452

436453
/// <summary>
437454
/// Creates a new <see cref="SqlExpression" /> which represents a SQL token.

Diff for: ‎src/EFCore.Relational/Query/Internal/RelationalParameterProcessor.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ object ProcessConstantValue(object? existingConstantValue)
216216
return _sqlExpressionFactory.Constant(
217217
existingConstantValue,
218218
existingConstantValue?.GetType() ?? typeof(object),
219-
typeMapping: _typeMappingSource.GetMappingForValue(existingConstantValue));
219+
_typeMappingSource.GetMappingForValue(existingConstantValue));
220220
}
221221

222222
void ProcessDbParameter(DbParameter dbParameter)

Diff for: ‎src/EFCore.Relational/Query/Internal/RelationalValueConverterCompensatingExpressionVisitor.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ private Expression VisitJoin(PredicateJoinExpressionBase joinExpression)
103103
{
104104
return _sqlExpressionFactory.Equal(
105105
sqlExpression,
106-
_sqlExpressionFactory.Constant(true, typeMapping: sqlExpression.TypeMapping));
106+
_sqlExpressionFactory.Constant(true, sqlExpression.TypeMapping));
107107
}
108108

109109
if (sqlExpression is SqlUnaryExpression sqlUnaryExpression)

Diff for: ‎src/EFCore.Relational/Query/Internal/SqlExpressionSimplifyingExpressionVisitor.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ private SqlExpression OptimizeCompareTo(
161161
intValue switch
162162
{
163163
0 => _sqlExpressionFactory.GreaterThan(testLeft, testRight),
164-
1 => _sqlExpressionFactory.Constant(false, typeMapping: sqlBinaryExpression.TypeMapping),
164+
1 => _sqlExpressionFactory.Constant(false, sqlBinaryExpression.TypeMapping),
165165
_ => _sqlExpressionFactory.GreaterThanOrEqual(testLeft, testRight)
166166
}),
167167
// CompareTo(a, b) >= 0 -> a >= b
@@ -172,7 +172,7 @@ private SqlExpression OptimizeCompareTo(
172172
{
173173
0 => _sqlExpressionFactory.GreaterThanOrEqual(testLeft, testRight),
174174
1 => _sqlExpressionFactory.GreaterThan(testLeft, testRight),
175-
_ => _sqlExpressionFactory.Constant(true, typeMapping: sqlBinaryExpression.TypeMapping)
175+
_ => _sqlExpressionFactory.Constant(true, sqlBinaryExpression.TypeMapping)
176176
}),
177177
// CompareTo(a, b) < 0 -> a < b
178178
// CompareTo(a, b) < 1 -> a <= b
@@ -182,14 +182,14 @@ private SqlExpression OptimizeCompareTo(
182182
{
183183
0 => _sqlExpressionFactory.LessThan(testLeft, testRight),
184184
1 => _sqlExpressionFactory.LessThanOrEqual(testLeft, testRight),
185-
_ => _sqlExpressionFactory.Constant(false, typeMapping: sqlBinaryExpression.TypeMapping)
185+
_ => _sqlExpressionFactory.Constant(false, sqlBinaryExpression.TypeMapping)
186186
}),
187187

188188
_ => (SqlExpression)Visit(
189189
intValue switch
190190
{
191191
0 => _sqlExpressionFactory.LessThanOrEqual(testLeft, testRight),
192-
1 => _sqlExpressionFactory.Constant(true, typeMapping: sqlBinaryExpression.TypeMapping),
192+
1 => _sqlExpressionFactory.Constant(true, sqlBinaryExpression.TypeMapping),
193193
_ => _sqlExpressionFactory.LessThan(testLeft, testRight)
194194
})
195195
};

Diff for: ‎src/EFCore.Relational/Query/Internal/Translators/GetValueOrDefaultTranslator.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public GetValueOrDefaultTranslator(ISqlExpressionFactory sqlExpressionFactory)
4444
return _sqlExpressionFactory.Coalesce(
4545
instance,
4646
arguments.Count == 0
47-
? new SqlConstantExpression(method.ReturnType.GetDefaultValue(), method.ReturnType, originallyParameter: false, typeMapping: null)
47+
? new SqlConstantExpression(method.ReturnType.GetDefaultValue(), method.ReturnType, typeMapping: null)
4848
: arguments[0],
4949
instance.TypeMapping);
5050
}

Diff for: ‎src/EFCore.Relational/Query/QuerySqlGenerator.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -627,7 +627,7 @@ protected override Expression VisitSqlBinary(SqlBinaryExpression sqlBinaryExpres
627627
/// <param name="sqlConstantExpression">The <see cref="SqlConstantExpression" /> for which to generate SQL.</param>
628628
protected override Expression VisitSqlConstant(SqlConstantExpression sqlConstantExpression)
629629
{
630-
if (sqlConstantExpression.OriginallyParameter)
630+
if (sqlConstantExpression.IsSensitive)
631631
{
632632
_relationalCommandBuilder
633633
.Append(sqlConstantExpression.TypeMapping!.GenerateSqlLiteral(sqlConstantExpression.Value), "?");

Diff for: ‎src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.CreateSelect.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ [new TableExpression(alias, table)],
204204
var projection = declaringEntityType.IsAssignableFrom(concreteEntityType)
205205
? CreateColumnExpression(property, table, tableAlias, declaringEntityType != entityType)
206206
: _sqlExpressionFactory.Constant(
207-
null, property.ClrType.MakeNullable(), typeMapping: property.GetRelationalTypeMapping());
207+
null, property.ClrType.MakeNullable(), property.GetRelationalTypeMapping());
208208
projections.Add(new ProjectionExpression(projection, propertyNames[j]));
209209
}
210210

Diff for: ‎src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ JsonScalarExpression jsonScalar
404404
new[]
405405
{
406406
// Since VALUES may not guarantee row ordering, we add an _ord value by which we'll order.
407-
_sqlExpressionFactory.Constant(i, typeMapping : intTypeMapping),
407+
_sqlExpressionFactory.Constant(i, intTypeMapping),
408408
// If no type mapping was inferred (i.e. no column in the inline collection), it's left null, to allow it to get
409409
// inferred later based on usage. Note that for the element in the VALUES expression, we'll also apply an explicit
410410
// CONVERT to make sure the database gets the right type (see

Diff for: ‎src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ protected override Expression VisitConditional(ConditionalExpression conditional
489489

490490
/// <inheritdoc />
491491
protected override Expression VisitConstant(ConstantExpression constantExpression)
492-
=> new SqlConstantExpression(constantExpression.Value, constantExpression.Type, originallyParameter: false, typeMapping: null);
492+
=> new SqlConstantExpression(constantExpression.Value, constantExpression.Type, typeMapping: null);
493493

494494
/// <inheritdoc />
495495
protected override Expression VisitExtension(Expression extensionExpression)
@@ -1639,7 +1639,6 @@ private static bool TryEvaluateToConstant(Expression expression, [NotNullWhen(tr
16391639
.Compile(preferInterpretation: true)
16401640
.Invoke(),
16411641
expression.Type,
1642-
originallyParameter: false,
16431642
typeMapping: null);
16441643
return true;
16451644
}

Diff for: ‎src/EFCore.Relational/Query/SqlExpressionFactory.cs

+14-6
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ private SqlExpression Not(SqlExpression operand, SqlExpression? existingExpressi
640640

641641
// !(true) -> false
642642
// !(false) -> true
643-
SqlConstantExpression { Value: bool boolValue } => Constant(!boolValue, operand.Type, typeMapping: operand.TypeMapping),
643+
SqlConstantExpression { Value: bool boolValue } => Constant(!boolValue, operand.Type, operand.TypeMapping),
644644

645645
// !(!a) -> a
646646
// ~(~a) -> a (bitwise negation)
@@ -797,7 +797,7 @@ public virtual SqlExpression Case(
797797
typeMappedWhenClauses.RemoveAt(typeMappedWhenClauses.Count - 1);
798798
}
799799

800-
var nullResult = Constant(null, elseResult?.Type ?? whenClauses[0].Result.Type, typeMapping: resultTypeMapping);
800+
var nullResult = Constant(null, elseResult?.Type ?? whenClauses[0].Result.Type, resultTypeMapping);
801801

802802
// if there are no whenClauses left (e.g. their tests evaluated to false):
803803
// - if there is Else block, return it
@@ -970,10 +970,18 @@ public virtual SqlExpression Fragment(string sql, Type? type = null, RelationalT
970970
=> new SqlFragmentExpression(sql, type, typeMapping);
971971

972972
/// <inheritdoc />
973-
public virtual SqlExpression Constant(object value, bool originallyParameter = false, RelationalTypeMapping? typeMapping = null)
974-
=> new SqlConstantExpression(value, originallyParameter, typeMapping);
973+
public virtual SqlExpression Constant(object value, RelationalTypeMapping? typeMapping = null)
974+
=> new SqlConstantExpression(value, typeMapping);
975975

976976
/// <inheritdoc />
977-
public virtual SqlExpression Constant(object? value, Type type, bool originallyParameter = false, RelationalTypeMapping? typeMapping = null)
978-
=> new SqlConstantExpression(value, type, originallyParameter, typeMapping);
977+
public virtual SqlExpression Constant(object? value, Type type, RelationalTypeMapping? typeMapping = null)
978+
=> new SqlConstantExpression(value, type, typeMapping);
979+
980+
/// <inheritdoc />
981+
public virtual SqlExpression Constant(object value, bool isSensitive, RelationalTypeMapping? typeMapping = null)
982+
=> new SqlConstantExpression(value, isSensitive, typeMapping);
983+
984+
/// <inheritdoc />
985+
public virtual SqlExpression Constant(object? value, Type type, bool isSensitive, RelationalTypeMapping? typeMapping = null)
986+
=> new SqlConstantExpression(value, type, isSensitive, typeMapping);
979987
}

Diff for: ‎src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs

+3-4
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ static void UpdateLimit(SelectExpression selectExpression)
771771
{
772772
if (selectExpression.Limit is SqlConstantExpression { Value: 2 } limitConstantExpression)
773773
{
774-
selectExpression.Limit = new SqlConstantExpression(1, originallyParameter: false, typeMapping: limitConstantExpression.TypeMapping);
774+
selectExpression.Limit = new SqlConstantExpression(1, limitConstantExpression.TypeMapping);
775775
}
776776
}
777777
}
@@ -2353,7 +2353,7 @@ static bool IsNullableProjection(ProjectionExpression projectionExpression)
23532353
public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory)
23542354
{
23552355
var nullSqlExpression = sqlExpressionFactory.ApplyDefaultTypeMapping(
2356-
new SqlConstantExpression(null, typeof(string), originallyParameter: false, typeMapping: null));
2356+
new SqlConstantExpression(null, typeof(string), null));
23572357

23582358
var dummySelectExpression = CreateImmutable(
23592359
_sqlAliasManager.GenerateTableAlias("empty"),
@@ -2968,8 +2968,7 @@ private void AddJoin(
29682968
&& limit is SqlConstantExpression limitConstant
29692969
? new SqlConstantExpression(
29702970
(int)offsetConstant.Value! + (int)limitConstant.Value!,
2971-
originallyParameter: false,
2972-
typeMapping: limit.TypeMapping)
2971+
limit.TypeMapping)
29732972
: new SqlBinaryExpression(ExpressionType.Add, offset, limit, limit.Type, limit.TypeMapping);
29742973
}
29752974

Diff for: ‎src/EFCore.Relational/Query/SqlExpressions/SqlConstantExpression.cs

+33-13
Original file line numberDiff line numberDiff line change
@@ -23,35 +23,55 @@ public class SqlConstantExpression : SqlExpression
2323
/// </summary>
2424
/// <param name="value">An <see cref="Object" /> to set the <see cref="Value" /> property equal to.</param>
2525
/// <param name="type">The <see cref="System.Type" /> of the expression.</param>
26-
/// <param name="originallyParameter"><see langword="true" /> if the expression was originally a parameter; otherwise, <see langword="false" />.</param>
2726
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
28-
public SqlConstantExpression(object? value, Type type, bool originallyParameter, RelationalTypeMapping? typeMapping)
27+
public SqlConstantExpression(object? value, Type type, RelationalTypeMapping? typeMapping)
28+
: this(value, type, false, typeMapping)
29+
{
30+
}
31+
32+
/// <summary>
33+
/// Creates a new instance of the <see cref="SqlConstantExpression" /> class.
34+
/// </summary>
35+
/// <param name="value">An <see cref="Object" /> to set the <see cref="Value" /> property equal to.</param>
36+
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
37+
public SqlConstantExpression(object value, RelationalTypeMapping? typeMapping)
38+
: this(value, false, typeMapping)
39+
{
40+
}
41+
42+
/// <summary>
43+
/// Creates a new instance of the <see cref="SqlConstantExpression" /> class.
44+
/// </summary>
45+
/// <param name="value">An <see cref="Object" /> to set the <see cref="Value" /> property equal to.</param>
46+
/// <param name="type">The <see cref="System.Type" /> of the expression.</param>
47+
/// <param name="isSensitive"><see langword="true" /> if the expression contains sensitive values; otherwise, <see langword="false" />.</param>
48+
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
49+
public SqlConstantExpression(object? value, Type type, bool isSensitive, RelationalTypeMapping? typeMapping)
2950
: base(type.UnwrapNullableType(), typeMapping)
3051
{
3152
Value = value;
32-
OriginallyParameter = originallyParameter;
53+
IsSensitive = isSensitive;
3354
}
3455

3556
/// <summary>
3657
/// Creates a new instance of the <see cref="SqlConstantExpression" /> class.
3758
/// </summary>
3859
/// <param name="value">An <see cref="Object" /> to set the <see cref="Value" /> property equal to.</param>
39-
/// <param name="originallyParameter"><see langword="true" /> if the expression was originally a parameter; otherwise, <see langword="false" />.</param>
60+
/// <param name="isSensitive"><see langword="true" /> if the expression contains sensitive values; otherwise, <see langword="false" />.</param>
4061
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
41-
public SqlConstantExpression(object value, bool originallyParameter, RelationalTypeMapping? typeMapping)
42-
: this(value, value.GetType(), originallyParameter, typeMapping)
62+
public SqlConstantExpression(object value, bool isSensitive, RelationalTypeMapping? typeMapping)
63+
: this(value, value.GetType(), isSensitive, typeMapping)
4364
{
4465
}
4566

4667
/// <summary>
4768
/// Creates a new instance of the <see cref="SqlConstantExpression" /> class.
4869
/// </summary>
4970
/// <param name="constantExpression">A <see cref="ConstantExpression" />.</param>
50-
/// <param name="originallyParameter"><see langword="true" /> if the expression was originally a parameter; otherwise, <see langword="false" />.</param>
5171
/// <param name="typeMapping">The <see cref="RelationalTypeMapping" /> associated with the expression.</param>
5272
[Obsolete("Call the constructor accepting a value (and possibly a Type) instead")]
53-
public SqlConstantExpression(ConstantExpression constantExpression, bool originallyParameter, RelationalTypeMapping? typeMapping)
54-
: this(constantExpression.Value, constantExpression.Type, originallyParameter, typeMapping)
73+
public SqlConstantExpression(ConstantExpression constantExpression, RelationalTypeMapping? typeMapping)
74+
: this(constantExpression.Value, constantExpression.Type, false, typeMapping)
5575
{
5676
}
5777

@@ -61,17 +81,17 @@ public SqlConstantExpression(ConstantExpression constantExpression, bool origina
6181
public virtual object? Value { get; }
6282

6383
/// <summary>
64-
/// Whether the expression was originally a parameter.
84+
/// Whether the expression contains sensitive values.
6585
/// </summary>
66-
public virtual bool OriginallyParameter { get; }
86+
public virtual bool IsSensitive { get; }
6787

6888
/// <summary>
6989
/// Applies supplied type mapping to this expression.
7090
/// </summary>
7191
/// <param name="typeMapping">A relational type mapping to apply.</param>
7292
/// <returns>A new expression which has supplied type mapping.</returns>
7393
public virtual SqlExpression ApplyTypeMapping(RelationalTypeMapping? typeMapping)
74-
=> new SqlConstantExpression(Value, Type, OriginallyParameter, typeMapping);
94+
=> new SqlConstantExpression(Value, Type, IsSensitive, typeMapping);
7595

7696
/// <inheritdoc />
7797
protected override Expression VisitChildren(ExpressionVisitor visitor)
@@ -87,7 +107,7 @@ public override Expression Quote()
87107
Constant(Value, Type), typeof(object))
88108
: Constant(Value, Type),
89109
Constant(Type),
90-
Constant(OriginallyParameter),
110+
Constant(IsSensitive),
91111
RelationalExpressionQuotingUtilities.QuoteTypeMapping(TypeMapping));
92112

93113
/// <inheritdoc />

0 commit comments

Comments
 (0)