Skip to content

Commit

Permalink
Infer type mapping suitable for concatenating two results (#32510)
Browse files Browse the repository at this point in the history
* Work on type mapping inference for string concatenation

Fixes #32325, see also #32333

* Tweaks, update baselines add more tests and cleanup

* Update baselines, add more tests, some cleanup.

---------

Co-authored-by: Shay Rojansky <roji@roji.org>
  • Loading branch information
ajcvickers and roji authored Dec 4, 2023
1 parent e1594af commit 1048f6a
Show file tree
Hide file tree
Showing 49 changed files with 546 additions and 298 deletions.
47 changes: 47 additions & 0 deletions src/EFCore.Relational/Query/SqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,53 @@ private SqlExpression ApplyTypeMappingOnSqlBinary(
}

case ExpressionType.Add:
inferredTypeMapping = typeMapping;

if (inferredTypeMapping is null)
{
var leftTypeMapping = left.TypeMapping;
var rightTypeMapping = right.TypeMapping;
if (leftTypeMapping != null || rightTypeMapping != null)
{
// Infer null size (nvarchar(max)) if either side has no size.
// Note that for constants, we could instead look at the value length; but that requires we know the type mappings
// which can have a size (string/byte[], maybe something else?).
var inferredSize = leftTypeMapping?.Size is { } leftSize && rightTypeMapping?.Size is { } rightSize
? leftSize + rightSize
: (int?)null;

// Unless both sides are fixed length, the result isn't fixed length.
var inferredFixedLength = leftTypeMapping?.IsFixedLength is true && rightTypeMapping?.IsFixedLength is true;

// Default to Unicode unless both sides are non-unicode.
var inferredUnicode = !(leftTypeMapping?.IsUnicode is false && rightTypeMapping?.IsUnicode is false);
var baseTypeMapping = leftTypeMapping ?? rightTypeMapping!;

inferredTypeMapping = leftTypeMapping?.Size == inferredSize
&& leftTypeMapping?.IsFixedLength == inferredFixedLength
&& leftTypeMapping?.IsUnicode == inferredUnicode
? leftTypeMapping
: rightTypeMapping?.Size == inferredSize
&& rightTypeMapping?.IsFixedLength == inferredFixedLength
&& rightTypeMapping?.IsUnicode == inferredUnicode
? rightTypeMapping
: _typeMappingSource.FindMapping(
baseTypeMapping.ClrType,
storeTypeName: null,
keyOrIndex: false,
inferredUnicode,
inferredSize,
rowVersion: false,
inferredFixedLength,
baseTypeMapping.Precision,
baseTypeMapping.Scale);
}
}

resultType = inferredTypeMapping?.ClrType ?? left.Type;
resultTypeMapping = inferredTypeMapping;
break;

case ExpressionType.Subtract:
case ExpressionType.Multiply:
case ExpressionType.Divide:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4622,6 +4622,66 @@ await AssertTranslationFailed(
AssertSql();
}

public override async Task Contains_over_concatenated_columns_with_different_sizes(bool async)
{
await base.Contains_over_concatenated_columns_with_different_sizes(async);

AssertSql(
"""
SELECT c
FROM root c
WHERE ((c["Discriminator"] = "Customer") AND (c["CustomerID"] || c["CompanyName"]) IN ("ALFKIAlfreds Futterkiste", "ANATRAna Trujillo Emparedados y helados"))
""");
}

public override async Task Contains_over_concatenated_column_and_constant(bool async)
{
await base.Contains_over_concatenated_column_and_constant(async);

AssertSql(
"""
SELECT c
FROM root c
WHERE ((c["Discriminator"] = "Customer") AND (c["CustomerID"] || "SomeConstant") IN ("ALFKISomeConstant", "ANATRSomeConstant", "ALFKIX"))
""");
}

public override async Task Contains_over_concatenated_columns_both_fixed_length(bool async)
{
await AssertTranslationFailed(
() => base.Contains_over_concatenated_columns_both_fixed_length(async));

AssertSql();
}

public override async Task Contains_over_concatenated_column_and_parameter(bool async)
{
await base.Contains_over_concatenated_column_and_parameter(async);

AssertSql(
"""
@__someVariable_1='SomeVariable'
SELECT c
FROM root c
WHERE ((c["Discriminator"] = "Customer") AND (c["CustomerID"] || @__someVariable_1) IN ("ALFKISomeVariable", "ANATRSomeVariable", "ALFKIX"))
""");
}

public override async Task Contains_over_concatenated_parameter_and_constant(bool async)
{
await base.Contains_over_concatenated_parameter_and_constant(async);

AssertSql(
"""
@__Contains_0='true'
SELECT c
FROM root c
WHERE ((c["Discriminator"] = "Customer") AND @__Contains_0)
""");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ void RewriteSourceWithNewBaseline(string fileName, int lineNumber)
indentBuilder.Append(" ");
var indent = indentBuilder.ToString();
var newBaseLine = $@"Assert{(forUpdate ? "ExecuteUpdate" : "")}Sql(
{string.Join("," + Environment.NewLine + indent + "//" + Environment.NewLine, SqlStatements.Skip(offset).Take(count).Select(sql => "\"\"\"" + Environment.NewLine + sql + Environment.NewLine + "\"\"\""))})";
{string.Join("," + Environment.NewLine + indent + "//" + Environment.NewLine, SqlStatements.Skip(offset).Take(count).Select(sql => indent + "\"\"\"" + Environment.NewLine + sql + Environment.NewLine + "\"\"\""))})";
var numNewlinesInRewritten = newBaseLine.Count(c => c is '\n' or '\r');

writer.Write(newBaseLine);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1603,9 +1603,9 @@ public virtual Task Non_unicode_string_literals_is_used_for_non_unicode_column_w
where c.Location.Contains("Jacinto")
select c);

[ConditionalTheory]
[ConditionalTheory] // Issue #32325
[MemberData(nameof(IsAsyncData))]
public virtual Task Non_unicode_string_literals_is_used_for_non_unicode_column_with_concat(bool async)
public virtual Task Unicode_string_literals_is_used_for_non_unicode_column_with_concat(bool async)
=> AssertQuery(
async,
ss => from c in ss.Set<City>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5674,5 +5674,62 @@ await AssertQuery(
.Select(g => new { g.Key, MaxTimestamp = g.Select(e => e.Order.OrderDate).Max() })
.OrderBy(x => x.MaxTimestamp)
.Select(x => x));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Contains_over_concatenated_columns_with_different_sizes(bool async)
{
var data = new[] { "ALFKI" + "Alfreds Futterkiste", "ANATR" + "Ana Trujillo Emparedados y helados" };

return AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => data.Contains(c.CustomerID + c.CompanyName)));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Contains_over_concatenated_column_and_constant(bool async)
{
var data = new[] { "ALFKI" + "SomeConstant", "ANATR" + "SomeConstant", "ALFKI" + "X"};

return AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => data.Contains(c.CustomerID + "SomeConstant")));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Contains_over_concatenated_column_and_parameter(bool async)
{
var data = new[] { "ALFKI" + "SomeVariable", "ANATR" + "SomeVariable", "ALFKI" + "X" };
var someVariable = "SomeVariable";

return AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => data.Contains(c.CustomerID + someVariable)));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Contains_over_concatenated_parameter_and_constant(bool async)
{
var data = new[] { "ALFKI" + "SomeConstant", "ANATR" + "SomeConstant" };
var someVariable = "ALFKI";

return AssertQuery(
async,
ss => ss.Set<Customer>().Where(c => data.Contains(someVariable + "SomeConstant")));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Contains_over_concatenated_columns_both_fixed_length(bool async)
{
var data = new[] { "ALFKIALFKI", "ALFKI", "ANATR" + "Ana Trujillo Emparedados y helados", "ANATR" + "ANATR"};

return AssertQuery(
async,
ss => ss.Set<Order>().Where(o => data.Contains(o.CustomerID + o.Customer.CustomerID)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1038,8 +1038,8 @@ public override async Task Update_Where_set_property_plus_parameter(bool async)
await base.Update_Where_set_property_plus_parameter(async);

AssertExecuteUpdateSql(
"""
@__value_0='Abc' (Size = 30)
"""
@__value_0='Abc' (Size = 4000)

UPDATE [c]
SET [c].[ContactName] = COALESCE([c].[ContactName], N'') + @__value_0
Expand Down
Loading

0 comments on commit 1048f6a

Please sign in to comment.