Skip to content

Commit ec42520

Browse files
rojiroji
authored
Some minor Cosmos cleanup (#34155)
* Simplify SqlConstantExpression and SqlParameterExpression, aligning to relationl * Clean up test failures in NonSharedPrimitiveCollectionsQueryCosmosTest
1 parent ac7380e commit ec42520

10 files changed

+79
-63
lines changed

‎src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ protected override ShapedQueryExpression CreateShapedQueryExpression(IEntityType
440440
selectExpression,
441441
_sqlExpressionFactory.Equal(
442442
(SqlExpression)discriminatorColumn,
443-
_sqlExpressionFactory.Constant(concreteEntityType.GetDiscriminatorValue())));
443+
_sqlExpressionFactory.Constant(concreteEntityType.GetDiscriminatorValue(), discriminatorColumn.Type)));
444444
Check.DebugAssert(success, "Couldn't apply predicate when creating a new ShapedQueryExpression");
445445
}
446446
}
@@ -455,7 +455,8 @@ protected override ShapedQueryExpression CreateShapedQueryExpression(IEntityType
455455
selectExpression,
456456
_sqlExpressionFactory.In(
457457
(SqlExpression)discriminatorColumn,
458-
concreteEntityTypes.Select(et => _sqlExpressionFactory.Constant(et.GetDiscriminatorValue())).ToArray()));
458+
concreteEntityTypes.Select(et => _sqlExpressionFactory.Constant(et.GetDiscriminatorValue(), discriminatorColumn.Type))
459+
.ToArray()));
459460
Check.DebugAssert(success, "Couldn't apply predicate when creating a new ShapedQueryExpression");
460461
}
461462

@@ -743,7 +744,7 @@ protected override ShapedQueryExpression TranslateCast(ShapedQueryExpression sou
743744
translation = new ObjectBinaryExpression(
744745
ExpressionType.Coalesce,
745746
translation,
746-
new SqlConstantExpression(Expression.Constant(null, typeof(object)), _typeMappingSource.FindMapping(typeof(int))),
747+
new SqlConstantExpression(null, typeof(object), _typeMappingSource.FindMapping(typeof(int))),
747748
translation.Type);
748749
}
749750

@@ -1754,9 +1755,8 @@ when methodCallExpression.TryGetIndexerArguments(_queryCompilationContext.Model,
17541755
/// </summary>
17551756
protected override ShapedQueryExpression? TranslateParameterQueryRoot(ParameterQueryRootExpression parameterQueryRootExpression)
17561757
{
1757-
if (parameterQueryRootExpression.ParameterExpression.Name?.StartsWith(
1758-
QueryCompilationContext.QueryParameterPrefix, StringComparison.Ordinal)
1759-
!= true)
1758+
var parameter = parameterQueryRootExpression.ParameterExpression;
1759+
if (parameter.Name?.StartsWith(QueryCompilationContext.QueryParameterPrefix, StringComparison.Ordinal) != true)
17601760
{
17611761
return null;
17621762
}
@@ -1766,7 +1766,7 @@ when methodCallExpression.TryGetIndexerArguments(_queryCompilationContext.Model,
17661766
var elementClrType = parameterQueryRootExpression.ElementType;
17671767
var arrayTypeMapping = _typeMappingSource.FindMapping(typeof(IEnumerable<>).MakeGenericType(elementClrType));
17681768
var elementTypeMapping = _typeMappingSource.FindMapping(elementClrType)!;
1769-
var sqlParameterExpression = new SqlParameterExpression(parameterQueryRootExpression.ParameterExpression, arrayTypeMapping);
1769+
var sqlParameterExpression = new SqlParameterExpression(parameter.Name, parameter.Type, arrayTypeMapping);
17701770

17711771
var sourceAlias = _aliasManager.GenerateSourceAlias(sqlParameterExpression.Name.TrimStart('_'));
17721772
var select = SelectExpression.CreateForCollection(

‎src/EFCore.Cosmos/Query/Internal/CosmosShapedQueryCompilingExpressionVisitor.InExpressionValuesExpandingExpressionVisitor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ protected override Expression VisitExtension(Expression expression)
3535
var mutableValues = new List<SqlExpression>();
3636
foreach (var value in (IEnumerable)parametersValues[valuesParameter.Name])
3737
{
38-
mutableValues.Add(sqlExpressionFactory.Constant(value, typeMapping));
38+
mutableValues.Add(sqlExpressionFactory.Constant(value, value?.GetType() ?? typeof(object), typeMapping));
3939
}
4040
values = mutableValues;
4141
break;

‎src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,10 @@ Expression ProcessGetType(EntityReferenceExpression entityReferenceExpression, T
244244
return match
245245
? sqlExpressionFactory.Equal(
246246
discriminatorColumn,
247-
sqlExpressionFactory.Constant(derivedType.GetDiscriminatorValue()))
247+
sqlExpressionFactory.Constant(derivedType.GetDiscriminatorValue(), discriminatorColumn.Type))
248248
: sqlExpressionFactory.NotEqual(
249249
discriminatorColumn,
250-
sqlExpressionFactory.Constant(derivedType.GetDiscriminatorValue()));
250+
sqlExpressionFactory.Constant(derivedType.GetDiscriminatorValue(), discriminatorColumn.Type));
251251
}
252252
}
253253

@@ -323,7 +323,7 @@ protected override Expression VisitConditional(ConditionalExpression conditional
323323
/// doing so can result in application failures when updating to a new Entity Framework Core release.
324324
/// </summary>
325325
protected override Expression VisitConstant(ConstantExpression constantExpression)
326-
=> new SqlConstantExpression(constantExpression, null);
326+
=> new SqlConstantExpression(constantExpression.Value, constantExpression.Type, typeMapping: null);
327327

328328
/// <summary>
329329
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -785,7 +785,7 @@ protected override Expression VisitNewArray(NewArrayExpression newArrayExpressio
785785
/// </summary>
786786
protected override Expression VisitParameter(ParameterExpression parameterExpression)
787787
=> parameterExpression.Name?.StartsWith(QueryCompilationContext.QueryParameterPrefix, StringComparison.Ordinal) == true
788-
? new SqlParameterExpression(parameterExpression, null)
788+
? new SqlParameterExpression(parameterExpression.Name, parameterExpression.Type, null)
789789
: QueryCompilationContext.NotTranslatedExpression;
790790

791791
/// <summary>
@@ -859,10 +859,11 @@ protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExp
859859
return concreteEntityTypes.Count == 1
860860
? sqlExpressionFactory.Equal(
861861
discriminatorColumn,
862-
sqlExpressionFactory.Constant(concreteEntityTypes[0].GetDiscriminatorValue()))
862+
sqlExpressionFactory.Constant(concreteEntityTypes[0].GetDiscriminatorValue(), discriminatorColumn.Type))
863863
: sqlExpressionFactory.In(
864864
discriminatorColumn,
865-
concreteEntityTypes.Select(et => sqlExpressionFactory.Constant(et.GetDiscriminatorValue())).ToArray());
865+
concreteEntityTypes
866+
.Select(et => sqlExpressionFactory.Constant(et.GetDiscriminatorValue(), discriminatorColumn.Type)).ToArray());
866867
}
867868
}
868869

@@ -1196,11 +1197,10 @@ private static bool TryEvaluateToConstant(Expression expression, [NotNullWhen(tr
11961197
if (CanEvaluate(expression))
11971198
{
11981199
sqlConstantExpression = new SqlConstantExpression(
1199-
Expression.Constant(
1200-
Expression.Lambda<Func<object>>(Expression.Convert(expression, typeof(object)))
1201-
.Compile(preferInterpretation: true)
1202-
.Invoke(),
1203-
expression.Type),
1200+
Expression.Lambda<Func<object>>(Expression.Convert(expression, typeof(object)))
1201+
.Compile(preferInterpretation: true)
1202+
.Invoke(),
1203+
expression.Type,
12041204
null);
12051205
return true;
12061206
}

‎src/EFCore.Cosmos/Query/Internal/Expressions/SqlConstantExpression.cs

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,36 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
1515
/// any release. You should only use it directly in your code with extreme caution and knowing that
1616
/// doing so can result in application failures when updating to a new Entity Framework Core release.
1717
/// </summary>
18-
public class SqlConstantExpression(ConstantExpression constantExpression, CoreTypeMapping? typeMapping)
19-
: SqlExpression(constantExpression.Type, typeMapping)
18+
public class SqlConstantExpression : SqlExpression
2019
{
2120
/// <summary>
2221
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
2322
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
2423
/// any release. You should only use it directly in your code with extreme caution and knowing that
2524
/// doing so can result in application failures when updating to a new Entity Framework Core release.
2625
/// </summary>
27-
public virtual object? Value
28-
=> constantExpression.Value;
26+
public SqlConstantExpression(object? value, Type type, CoreTypeMapping? typeMapping)
27+
: base(type.UnwrapNullableType(), typeMapping)
28+
=> Value = value;
29+
30+
/// <summary>
31+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
32+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
33+
/// any release. You should only use it directly in your code with extreme caution and knowing that
34+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
35+
/// </summary>
36+
public SqlConstantExpression(object value, CoreTypeMapping? typeMapping)
37+
: this(value, value.GetType(), typeMapping)
38+
{
39+
}
40+
41+
/// <summary>
42+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
43+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
44+
/// any release. You should only use it directly in your code with extreme caution and knowing that
45+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
46+
/// </summary>
47+
public virtual object? Value { get; }
2948

3049
/// <summary>
3150
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -34,7 +53,7 @@ public virtual object? Value
3453
/// doing so can result in application failures when updating to a new Entity Framework Core release.
3554
/// </summary>
3655
public virtual SqlExpression ApplyTypeMapping(CoreTypeMapping? typeMapping)
37-
=> new SqlConstantExpression(constantExpression, typeMapping ?? TypeMapping);
56+
=> new SqlConstantExpression(Value, Type, typeMapping ?? TypeMapping);
3857

3958
/// <summary>
4059
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to

‎src/EFCore.Cosmos/Query/Internal/Expressions/SqlParameterExpression.cs

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,34 +10,16 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
1010
/// any release. You should only use it directly in your code with extreme caution and knowing that
1111
/// doing so can result in application failures when updating to a new Entity Framework Core release.
1212
/// </summary>
13-
public sealed class SqlParameterExpression : SqlExpression
13+
public sealed class SqlParameterExpression(string name, Type type, CoreTypeMapping? typeMapping)
14+
: SqlExpression(type, typeMapping)
1415
{
15-
private readonly ParameterExpression _parameterExpression;
16-
private readonly string _name;
17-
18-
/// <summary>
19-
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
20-
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
21-
/// any release. You should only use it directly in your code with extreme caution and knowing that
22-
/// doing so can result in application failures when updating to a new Entity Framework Core release.
23-
/// </summary>
24-
public SqlParameterExpression(ParameterExpression parameterExpression, CoreTypeMapping? typeMapping)
25-
: base(parameterExpression.Type, typeMapping)
26-
{
27-
Check.DebugAssert(parameterExpression.Name != null, "Parameter must have name.");
28-
29-
_parameterExpression = parameterExpression;
30-
_name = parameterExpression.Name;
31-
}
32-
3316
/// <summary>
3417
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
3518
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
3619
/// any release. You should only use it directly in your code with extreme caution and knowing that
3720
/// doing so can result in application failures when updating to a new Entity Framework Core release.
3821
/// </summary>
39-
public string Name
40-
=> _name;
22+
public string Name { get; } = name;
4123

4224
/// <summary>
4325
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -46,7 +28,7 @@ public string Name
4628
/// doing so can result in application failures when updating to a new Entity Framework Core release.
4729
/// </summary>
4830
public SqlExpression ApplyTypeMapping(CoreTypeMapping? typeMapping)
49-
=> new SqlParameterExpression(_parameterExpression, typeMapping ?? TypeMapping);
31+
=> new SqlParameterExpression(Name, Type, typeMapping ?? TypeMapping);
5032

5133
/// <summary>
5234
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -64,7 +46,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor)
6446
/// doing so can result in application failures when updating to a new Entity Framework Core release.
6547
/// </summary>
6648
protected override void Print(ExpressionPrinter expressionPrinter)
67-
=> expressionPrinter.Append("@" + _parameterExpression.Name);
49+
=> expressionPrinter.Append("@" + Name);
6850

6951
/// <summary>
7052
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to

‎src/EFCore.Cosmos/Query/Internal/ISqlExpressionFactory.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,5 +303,14 @@ SqlConditionalExpression Condition(
303303
/// any release. You should only use it directly in your code with extreme caution and knowing that
304304
/// doing so can result in application failures when updating to a new Entity Framework Core release.
305305
/// </summary>
306-
SqlConstantExpression Constant(object? value, CoreTypeMapping? typeMapping = null);
306+
SqlExpression Constant(object value, CoreTypeMapping? typeMapping = null);
307+
308+
/// <summary>
309+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
310+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
311+
/// any release. You should only use it directly in your code with extreme caution and knowing that
312+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
313+
/// </summary>
314+
SqlExpression Constant(object? value, Type type, CoreTypeMapping? typeMapping = null);
315+
307316
}

‎src/EFCore.Cosmos/Query/Internal/SqlExpressionFactory.cs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ public virtual SqlExpression CoalesceUndefined(SqlExpression left, SqlExpression
514514
/// doing so can result in application failures when updating to a new Entity Framework Core release.
515515
/// </summary>
516516
public virtual SqlBinaryExpression IsNull(SqlExpression operand)
517-
=> Equal(operand, Constant(null));
517+
=> Equal(operand, Constant(null, operand.Type));
518518

519519
/// <summary>
520520
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -523,7 +523,7 @@ public virtual SqlBinaryExpression IsNull(SqlExpression operand)
523523
/// doing so can result in application failures when updating to a new Entity Framework Core release.
524524
/// </summary>
525525
public virtual SqlBinaryExpression IsNotNull(SqlExpression operand)
526-
=> NotEqual(operand, Constant(null));
526+
=> NotEqual(operand, Constant(null, operand.Type));
527527

528528
/// <summary>
529529
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -627,6 +627,15 @@ public virtual InExpression In(SqlExpression item, SqlParameterExpression values
627627
/// any release. You should only use it directly in your code with extreme caution and knowing that
628628
/// doing so can result in application failures when updating to a new Entity Framework Core release.
629629
/// </summary>
630-
public virtual SqlConstantExpression Constant(object? value, CoreTypeMapping? typeMapping = null)
631-
=> new(Expression.Constant(value), typeMapping);
630+
public virtual SqlExpression Constant(object value, CoreTypeMapping? typeMapping = null)
631+
=> new SqlConstantExpression(value, typeMapping);
632+
633+
/// <summary>
634+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
635+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
636+
/// any release. You should only use it directly in your code with extreme caution and knowing that
637+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
638+
/// </summary>
639+
public virtual SqlExpression Constant(object? value, Type type, CoreTypeMapping? typeMapping = null)
640+
=> new SqlConstantExpression(value, type, typeMapping);
632641
}

‎src/EFCore/Query/EntityQueryRootExpression.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ namespace Microsoft.EntityFrameworkCore.Query;
1616
/// See <see href="https://aka.ms/efcore-docs-providers">Implementation of database providers and extensions</see>
1717
/// and <see href="https://aka.ms/efcore-docs-how-query-works">How EF Core queries work</see> for more information and examples.
1818
/// </remarks>
19+
[DebuggerDisplay("{Microsoft.EntityFrameworkCore.Query.ExpressionPrinter.Print(this), nq}")]
1920
public class EntityQueryRootExpression : QueryRootExpression, IPrintableExpression
2021
{
2122
/// <summary>

‎test/EFCore.Cosmos.FunctionalTests/Query/NonSharedPrimitiveCollectionsQueryCosmosTest.cs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,10 @@ OFFSET 0 LIMIT 2
7171
""");
7272
}
7373

74-
public override async Task Array_of_byte()
75-
{
76-
// TODO
77-
await Assert.ThrowsAsync<InvalidOperationException>(() => base.Array_of_byte());
78-
79-
AssertSql();
80-
}
74+
// byte[] gets mapped to base64, which isn't queryable as a regular primitive collection.
75+
[ConditionalFact]
76+
public override Task Array_of_byte()
77+
=> AssertTranslationFailed(() => TestArray((byte)1, (byte)2));
8178

8279
public override async Task Array_of_double()
8380
{
@@ -289,7 +286,7 @@ OFFSET 0 LIMIT 2
289286

290287
public override async Task Array_of_byte_array()
291288
{
292-
// TODO
289+
// TODO: primitive collection over value-converted element, #34153
293290
await Assert.ThrowsAsync<InvalidOperationException>(() => base.Array_of_byte_array());
294291

295292
AssertSql(
@@ -350,8 +347,8 @@ public override async Task Parameter_with_inferred_value_converter()
350347

351348
public override async Task Constant_with_inferred_value_converter()
352349
{
353-
// TODO
354-
await Assert.ThrowsAsync<InvalidOperationException>(() => base.Constant_with_inferred_value_converter());
350+
// TODO: advanced type mapping inference for inline scalar collection, #34026
351+
await AssertTranslationFailed(() => base.Constant_with_inferred_value_converter());
355352

356353
AssertSql();
357354
}

‎test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,6 @@ protected async Task TestArray<TElement>(
299299
Constant(2)),
300300
entityParam);
301301

302-
// context.Set<TestEntity>().SingleAsync(m => EF.Property<int[]>(m, "SomeArray").Count(a => a == <value1>) == 2)
303302
var result = await context.Set<TestEntity>().SingleAsync(predicate);
304303
Assert.Equal(1, result.Id);
305304
}

0 commit comments

Comments
 (0)