Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Contains on byte arrays on SqlServer #19071

Merged
merged 1 commit into from
Nov 26, 2019
Merged

Support Contains on byte arrays on SqlServer #19071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/EFCore.SqlServer/Query/Internal/SqlServerByteArrayMethodTranslator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Text;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
using Microsoft.EntityFrameworkCore.Storage;

namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal
{
public class SqlServerByteArrayMethodTranslator : IMethodCallTranslator
{
private readonly ISqlExpressionFactory _sqlExpressionFactory;

public SqlServerByteArrayMethodTranslator(ISqlExpressionFactory sqlExpressionFactory)
{
_sqlExpressionFactory = sqlExpressionFactory;
}

public virtual SqlExpression Translate(SqlExpression instance, MethodInfo method, IReadOnlyList<SqlExpression> arguments)
{
if (method.IsGenericMethod
&& method.GetGenericMethodDefinition().Equals(EnumerableMethods.Contains)
&& arguments[0].Type == typeof(byte[]))
{
instance = arguments[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't assign to instance. Using parameters of a function to store local variable causes confusion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be even more confusing to have a two instance variables, one "bad" and one "good".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't call it instance. Just call it firstArgument.

var typeMapping = instance.TypeMapping;

var pattern = arguments[1] is SqlConstantExpression constantExpression
? (SqlExpression)_sqlExpressionFactory.Constant(new[] { (byte)constantExpression.Value }, typeMapping)
: _sqlExpressionFactory.Convert(arguments[1], typeof(byte[]), typeMapping);

return _sqlExpressionFactory.GreaterThan(
_sqlExpressionFactory.Function(
"CHARINDEX",
new[] { pattern, instance },
typeof(int)),
_sqlExpressionFactory.Constant(0));
}

return null;
}
}
}
3 changes: 2 additions & 1 deletion src/EFCore.SqlServer/Query/Internal/SqlServerMethodCallTranslatorProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public SqlServerMethodCallTranslatorProvider([NotNull] RelationalMethodCallTrans
new SqlServerMathTranslator(sqlExpressionFactory),
new SqlServerNewGuidTranslator(sqlExpressionFactory),
new SqlServerObjectToStringTranslator(sqlExpressionFactory),
new SqlServerStringMethodTranslator(sqlExpressionFactory)
new SqlServerStringMethodTranslator(sqlExpressionFactory),
new SqlServerByteArrayMethodTranslator(sqlExpressionFactory)
});
}
}
Expand Down
21 changes: 21 additions & 0 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7282,6 +7282,27 @@ on ll.Name equals h.CommanderName
select h);
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Byte_array_contains_literal(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Squad>().Where(s => s.Banner.Contains((byte)1)),
ss => ss.Set<Squad>().Where(s => s.Banner != null && s.Banner.Contains((byte)1)));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Byte_array_contains_parameter(bool async)
{
var someByte = (byte)1;
return AssertQuery(
async,
ss => ss.Set<Squad>().Where(s => s.Banner.Contains(someByte)),
ss => ss.Set<Squad>().Where(s => s.Banner != null && s.Banner.Contains(someByte)));
}

protected async Task AssertTranslationFailed(Func<Task> testCode)
{
Assert.Contains(
Expand Down
6 changes: 5 additions & 1 deletion test/EFCore.Specification.Tests/TestModels/GearsOfWarModel/GearsOfWarData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,11 @@ public virtual IQueryable<TEntity> Set<TEntity>()
}

public static IReadOnlyList<Squad> CreateSquads()
=> new List<Squad> { new Squad { Id = 1, Name = "Delta", Banner = new byte[] { 0x00, 0x01 } }, new Squad { Id = 2, Name = "Kilo", Banner = null } };
=> new List<Squad>
{
new Squad { Id = 1, Name = "Delta", Banner = new byte[] { 0x00, 0x01 } },
new Squad { Id = 2, Name = "Kilo", Banner = new byte[] { 0x02, 0x03 } }
};

public static IReadOnlyList<Mission> CreateMissions()
=> new List<Mission>
Expand Down
22 changes: 22 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7265,6 +7265,28 @@ ELSE NULL
END IS NULL)");
}

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

AssertSql(
@"SELECT [s].[Id], [s].[Banner], [s].[InternalNumber], [s].[Name]
FROM [Squads] AS [s]
WHERE CHARINDEX(0x01, [s].[Banner]) > 0");
}

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

AssertSql(
@"@__someByte_0='1' (Size = 1)

SELECT [s].[Id], [s].[Banner], [s].[InternalNumber], [s].[Name]
FROM [Squads] AS [s]
WHERE CHARINDEX(CAST(@__someByte_0 AS varbinary(max)), [s].[Banner]) > 0");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
}
Expand Down
4 changes: 4 additions & 0 deletions test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ public override async Task Select_datetimeoffset_comparison_in_projection(bool a
FROM ""Missions"" AS ""m""");
}

public override Task Byte_array_contains_literal(bool async) => null;

public override Task Byte_array_contains_parameter(bool async) => null;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sqlite does have INSTR which works great for literal, but I couldn't find a way to cast/convert a non-literal byte into an Sqlite BLOB. Maybe @bricelam knows?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

SELECT *
FROM Squads
WHERE instr(Banner, char(@someByte)) > 0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, that converts Banner to TEXT, but the result is the same. cast(char(@someByte) as blob) would keep them both blob.

Note, without char(), someByte is converted from integer to blob which results in an 8-byte blob.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do this.


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