Skip to content

Commit c46500e

Browse files
rojiroji
committed
Remove unneeded Cosmos 3-value logic compensation for InExpression
Closes #31063
1 parent 8adc1a9 commit c46500e

7 files changed

Lines changed: 55 additions & 93 deletions

‎src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs‎

Lines changed: 0 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/EFCore.Cosmos/Properties/CosmosStrings.resx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,6 @@
243243
<data name="OneOfTwoValuesMustBeSet" xml:space="preserve">
244244
<value>Exactly one of '{param1}' or '{param2}' must be set.</value>
245245
</data>
246-
<data name="OnlyConstantsAndParametersAllowedInContains" xml:space="preserve">
247-
<value>Only constants or parameters are currently allowed in Contains.</value>
248-
</data>
249246
<data name="OrphanedNestedDocument" xml:space="preserve">
250247
<value>The entity of type '{entityType}' is mapped as a part of the document mapped to '{missingEntityType}', but there is no tracked entity of this type with the corresponding key value. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values.</value>
251248
</data>

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

Lines changed: 20 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -10,99 +10,47 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
1010

1111
public partial class CosmosShapedQueryCompilingExpressionVisitor
1212
{
13-
private sealed class InExpressionValuesExpandingExpressionVisitor : ExpressionVisitor
13+
private sealed class InExpressionValuesExpandingExpressionVisitor(
14+
ISqlExpressionFactory sqlExpressionFactory,
15+
IReadOnlyDictionary<string, object> parametersValues)
16+
: ExpressionVisitor
1417
{
15-
private readonly ISqlExpressionFactory _sqlExpressionFactory;
16-
private readonly IReadOnlyDictionary<string, object> _parametersValues;
17-
18-
public InExpressionValuesExpandingExpressionVisitor(
19-
ISqlExpressionFactory sqlExpressionFactory,
20-
IReadOnlyDictionary<string, object> parametersValues)
21-
{
22-
_sqlExpressionFactory = sqlExpressionFactory;
23-
_parametersValues = parametersValues;
24-
}
25-
26-
public override Expression Visit(Expression expression)
18+
protected override Expression VisitExtension(Expression expression)
2719
{
2820
if (expression is InExpression inExpression)
2921
{
30-
var inValues = new List<SqlExpression>();
31-
var hasNullValue = false;
22+
IReadOnlyList<SqlExpression> values;
3223

3324
switch (inExpression)
3425
{
35-
case { ValuesParameter: SqlParameterExpression valuesParameter }:
36-
{
37-
var typeMapping = valuesParameter.TypeMapping;
38-
39-
foreach (var value in (IEnumerable)_parametersValues[valuesParameter.Name])
40-
{
41-
if (value is null)
42-
{
43-
hasNullValue = true;
44-
continue;
45-
}
46-
47-
inValues.Add(_sqlExpressionFactory.Constant(value, typeMapping));
48-
}
49-
26+
case { Values: IReadOnlyList<SqlExpression> values2 }:
27+
values = values2;
5028
break;
51-
}
5229

53-
case { Values: IReadOnlyList<SqlExpression> values }:
30+
// TODO: IN with subquery (return immediately, nothing to do here)
31+
32+
case { ValuesParameter: SqlParameterExpression valuesParameter }:
5433
{
55-
foreach (var value in values)
34+
var typeMapping = valuesParameter.TypeMapping;
35+
var mutableValues = new List<SqlExpression>();
36+
foreach (var value in (IEnumerable)parametersValues[valuesParameter.Name])
5637
{
57-
if (value is not (SqlConstantExpression or SqlParameterExpression))
58-
{
59-
throw new InvalidOperationException(CosmosStrings.OnlyConstantsAndParametersAllowedInContains);
60-
}
61-
62-
if (IsNull(value))
63-
{
64-
hasNullValue = true;
65-
continue;
66-
}
67-
68-
inValues.Add(value);
38+
mutableValues.Add(sqlExpressionFactory.Constant(value, typeMapping));
6939
}
70-
40+
values = mutableValues;
7141
break;
7242
}
7343

7444
default:
7545
throw new UnreachableException();
7646
}
7747

78-
var updatedInExpression = inValues.Count > 0
79-
? _sqlExpressionFactory.In((SqlExpression)Visit(inExpression.Item), inValues)
80-
: null;
81-
82-
var nullCheckExpression = hasNullValue
83-
? _sqlExpressionFactory.IsNull(inExpression.Item)
84-
: null;
85-
86-
if (updatedInExpression != null
87-
&& nullCheckExpression != null)
88-
{
89-
return _sqlExpressionFactory.OrElse(updatedInExpression, nullCheckExpression);
90-
}
91-
92-
if (updatedInExpression == null
93-
&& nullCheckExpression == null)
94-
{
95-
return _sqlExpressionFactory.Equal(_sqlExpressionFactory.Constant(true), _sqlExpressionFactory.Constant(false));
96-
}
97-
98-
return (SqlExpression)updatedInExpression ?? nullCheckExpression;
48+
return values.Count == 0
49+
? sqlExpressionFactory.ApplyDefaultTypeMapping(sqlExpressionFactory.Constant(false))
50+
: sqlExpressionFactory.In((SqlExpression)Visit(inExpression.Item), values);
9951
}
10052

101-
return base.Visit(expression);
53+
return base.VisitExtension(expression);
10254
}
103-
104-
private bool IsNull(SqlExpression expression)
105-
=> expression is SqlConstantExpression { Value: null }
106-
|| expression is SqlParameterExpression { Name: string parameterName } && _parametersValues[parameterName] is null;
10755
}
10856
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1806,9 +1806,8 @@ public override Task Contains_with_local_ordered_read_only_collection_all_null(b
18061806
"""
18071807
SELECT c
18081808
FROM root c
1809-
WHERE ((c["Discriminator"] = "Customer") AND (c["CustomerID"] = null))
1810-
"""
1811-
);
1809+
WHERE ((c["Discriminator"] = "Customer") AND c["CustomerID"] IN (null, null))
1810+
""");
18121811
});
18131812

18141813
public override Task Contains_with_local_read_only_collection_inline(bool async)
@@ -1969,7 +1968,7 @@ public override Task Contains_with_local_collection_empty_inline(bool async)
19691968
"""
19701969
SELECT c
19711970
FROM root c
1972-
WHERE ((c["Discriminator"] = "Customer") AND NOT((true = false)))
1971+
WHERE ((c["Discriminator"] = "Customer") AND NOT(false))
19731972
""");
19741973
});
19751974

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4158,7 +4158,7 @@ public override Task Entity_equality_contains_with_list_of_null(bool async)
41584158
"""
41594159
SELECT c
41604160
FROM root c
4161-
WHERE ((c["Discriminator"] = "Customer") AND (c["CustomerID"] IN ("ALFKI") OR (c["CustomerID"] = null)))
4161+
WHERE ((c["Discriminator"] = "Customer") AND c["CustomerID"] IN (null, "ALFKI"))
41624162
""");
41634163
});
41644164

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

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public override Task Inline_collection_of_nullable_ints_Contains_null(bool async
5555
"""
5656
SELECT c
5757
FROM root c
58-
WHERE ((c["Discriminator"] = "PrimitiveCollectionsEntity") AND (c["NullableInt"] IN (999) OR (c["NullableInt"] = null)))
58+
WHERE ((c["Discriminator"] = "PrimitiveCollectionsEntity") AND c["NullableInt"] IN (null, 999))
5959
""");
6060
});
6161

@@ -137,7 +137,7 @@ public override Task Inline_collection_Contains_with_zero_values(bool async)
137137
"""
138138
SELECT c
139139
FROM root c
140-
WHERE ((c["Discriminator"] = "PrimitiveCollectionsEntity") AND (true = false))
140+
WHERE ((c["Discriminator"] = "PrimitiveCollectionsEntity") AND false)
141141
""");
142142
});
143143

@@ -230,13 +230,37 @@ FROM root c
230230
""");
231231
});
232232

233-
// TODO: Remove incorrect null semantics compensation for Cosmos: #31063
234233
public override Task Inline_collection_Contains_with_mixed_value_types(bool async)
235-
=> Assert.ThrowsAsync<InvalidOperationException>(() => base.Inline_collection_Contains_with_mixed_value_types(async));
234+
=> CosmosTestHelpers.Instance.NoSyncTest(
235+
async, async a =>
236+
{
237+
await base.Inline_collection_Contains_with_mixed_value_types(a);
238+
239+
AssertSql(
240+
"""
241+
@__i_0='11'
242+
243+
SELECT c
244+
FROM root c
245+
WHERE ((c["Discriminator"] = "PrimitiveCollectionsEntity") AND c["Int"] IN (999, @__i_0, c["Id"], (c["Id"] + c["Int"])))
246+
""");
247+
});
236248

237-
// TODO: Remove incorrect null semantics compensation for Cosmos: #31063
238249
public override Task Inline_collection_List_Contains_with_mixed_value_types(bool async)
239-
=> Assert.ThrowsAsync<InvalidOperationException>(() => base.Inline_collection_List_Contains_with_mixed_value_types(async));
250+
=> CosmosTestHelpers.Instance.NoSyncTest(
251+
async, async a =>
252+
{
253+
await base.Inline_collection_List_Contains_with_mixed_value_types(a);
254+
255+
AssertSql(
256+
"""
257+
@__i_0='11'
258+
259+
SELECT c
260+
FROM root c
261+
WHERE ((c["Discriminator"] = "PrimitiveCollectionsEntity") AND c["Int"] IN (999, @__i_0, c["Id"], (c["Id"] + c["Int"])))
262+
""");
263+
});
240264

241265
public override Task Inline_collection_Contains_as_Any_with_predicate(bool async)
242266
=> CosmosTestHelpers.Instance.NoSyncTest(

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public virtual async Task Inline_collection_List_Contains_with_mixed_value_types
147147

148148
await AssertQuery(
149149
async,
150-
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => new List<int>() { 999, i, c.Id, c.Id + c.Int }.Contains(c.Int)));
150+
ss => ss.Set<PrimitiveCollectionsEntity>().Where(c => new List<int> { 999, i, c.Id, c.Id + c.Int }.Contains(c.Int)));
151151
}
152152

153153
[ConditionalTheory]

0 commit comments

Comments
 (0)