Skip to content

Added Support for translating Array.IndexOf methods for byte arrays for SqlServer & SQLite - #34457

Draft
nikhil197 wants to merge 5 commits into
dotnet:mainfrom
nikhil197:fix-19287-byteArr-IndexOf
Draft

Added Support for translating Array.IndexOf methods for byte arrays for SqlServer & SQLite#34457
nikhil197 wants to merge 5 commits into
dotnet:mainfrom
nikhil197:fix-19287-byteArr-IndexOf

Conversation

@nikhil197

@nikhil197 nikhil197 commented Aug 17, 2024

Copy link
Copy Markdown
  • Added Support for translating byte[].IndexOf method for Sql Server & SQLite

  • Added tests for the same
    Fixes Query: Translate byte array.IndexOf #19287

  • I've read the guidelines for contributing and seen the walkthrough

  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team

  • The code builds and tests pass locally (also verified by our automated build checks)

  • Commit messages follow this format:

        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber
  • Tests for the changes have been added (for bug fixes / features)
  • Code follows the same patterns and style as existing code in this repo

@nikhil197 nikhil197 changed the title [WIP] Added Support for translating byte[].IndexOf methods for SqlServer Added Support for translating byte[].IndexOf methods for SqlServer Aug 17, 2024
@nikhil197

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

Comment thread test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs Outdated
@nikhil197 nikhil197 changed the title Added Support for translating byte[].IndexOf methods for SqlServer Added Support for translating Array.IndexOf methods for byte arrays for SqlServer Aug 17, 2024
@nikhil197 nikhil197 changed the title Added Support for translating Array.IndexOf methods for byte arrays for SqlServer Added Support for translating Array.IndexOf methods for byte arrays for SqlServer & SQLite Aug 18, 2024
@nikhil197

Copy link
Copy Markdown
Author

Hi @roji
I think the PR is ready for review now. I have added translation for both SQL Server & SQLite and have also added tests for both.
Please take a look when you get sometime

Comment thread src/Shared/ArrayMethods.cs Outdated
Comment thread test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs Outdated
public virtual Task Byte_array_with_max_possible_length_filter_by_index_of_literal(bool async)
=> AssertQuery(
async,
ss => ss.Set<Squad>().Where(w => Array.IndexOf(w.Banner, (byte)1) == 1),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The cast here shouldn't be needed, no?

Suggested change
ss => ss.Set<Squad>().Where(w => Array.IndexOf(w.Banner, (byte)1) == 1),
ss => ss.Set<Squad>().Where(w => Array.IndexOf(w.Banner, 1) == 1),

@nikhil197 nikhil197 Aug 19, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No actually, it is needed. Without this it's picking non-generic version of the method IndexOf(Array arr, object value) (because 1 is an Int32 by default and I haven't specified the type argument on the IndexOf).

Do we want to support that too?

Comment thread test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs Outdated
Comment thread test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs Outdated
@nikhil197
nikhil197 requested a review from roji August 19, 2024 14:22
@nikhil197

Copy link
Copy Markdown
Author

Hi @roji
Could you please take a look at this PR, when you get sometime? I have resolved the comments that you added earlier. Please let me know if there's something else needed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds translation support for Array.IndexOf over byte[] in EF Core’s SQL Server and SQLite providers, aligning query translation behavior with existing byte[] method translations and expanding coverage via functional/specification tests (Fixes #19287).

Changes:

  • SQL Server: translate Array.IndexOf(byte[], byte) and Array.IndexOf(byte[], byte, int) via CHARINDEX(...) - 1.
  • SQLite: translate Array.IndexOf(byte[], byte) via instr(...) - 1 (start-index overloads remain unsupported).
  • Add new spec tests and provider-specific SQL assertion overrides across GearsOfWar query test suites.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/EFCore.SqlServer/Query/Internal/Translators/SqlServerByteArrayMethodTranslator.cs Adds SQL Server translation for Array.IndexOf on byte[] (with/without startIndex).
src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteByteArrayMethodTranslator.cs Adds SQLite translation for Array.IndexOf on byte[] (no startIndex support).
test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs Introduces new specification tests covering Array.IndexOf on byte[].
test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs Adds SQL assertions for the new Array.IndexOf translations (and keeps related byte[] assertions organized).
test/EFCore.SqlServer.FunctionalTests/Query/TPTGearsOfWarQuerySqlServerTest.cs Adds SQL assertions for Array.IndexOf translations in TPT mapping.
test/EFCore.SqlServer.FunctionalTests/Query/TPCGearsOfWarQuerySqlServerTest.cs Adds SQL assertions for Array.IndexOf translations in TPC mapping.
test/EFCore.SqlServer.FunctionalTests/Query/TemporalGearsOfWarQuerySqlServerTest.cs Adds SQL assertions for Array.IndexOf translations under temporal queries.
test/EFCore.Sqlite.FunctionalTests/Query/GearsOfWarQuerySqliteTest.cs Adds SQL assertions for SQLite instr(...) - 1 translation and translation-failure assertions for startIndex overloads.
test/EFCore.Sqlite.FunctionalTests/Query/TPTGearsOfWarQuerySqliteTest.cs Adds SQL assertions for SQLite Array.IndexOf translation in TPT mapping + startIndex translation-failure assertions.
test/EFCore.Sqlite.FunctionalTests/Query/TPCGearsOfWarQuerySqliteTest.cs Adds SQL assertions for SQLite Array.IndexOf translation in TPC mapping + startIndex translation-failure assertions.

=> AssertQuery(
async,
ss => ss.Set<Squad>().Where(w => Array.IndexOf(w.Banner5, (byte)5) == 1),
ss => ss.Set<Squad>().Where(w => w.Banner != null && Array.IndexOf(w.Banner5, (byte)5) == 1));
return AssertQuery(
async,
ss => ss.Set<Squad>().Where(w => Array.IndexOf(w.Banner5, b) == 0),
ss => ss.Set<Squad>().Where(w => w.Banner != null && Array.IndexOf(w.Banner5, b) == 0));
=> AssertQuery(
async,
ss => ss.Set<Squad>().Where(w => Array.IndexOf(w.Banner5, (byte)5, 1) == 1),
ss => ss.Set<Squad>().Where(w => w.Banner != null && Array.IndexOf(w.Banner5, (byte)5, 1) == 1));
return AssertQuery(
async,
ss => ss.Set<Squad>().Where(w => Array.IndexOf(w.Banner5, b, startPos) == 0),
ss => ss.Set<Squad>().Where(w => w.Banner != null && Array.IndexOf(w.Banner5, b, startPos) == 0));
@AndriySvyryd AndriySvyryd assigned AndriySvyryd and unassigned roji Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Query: Translate byte array.IndexOf

4 participants