Looping through list to create SQL Query [closed]

Question

I have a list of variables which I need to convert to LIKE statements in SQL. The final query should look like the following:

...
WHERE ([TraceableItem].[IdentificationNo] LIKE N'%015558' 
       OR [TraceableItem].[IdentificationNo] LIKE N'%123456' 
       OR [TraceableItem].[IdentificationNo] LIKE N'%654321')
...

Below is my code to do it, but I feel it is fairly inefficient / redundant. Especially features like having a bool for firstLoop, and constantly checking for items already existing. I would be very grateful for a code review and tips on how to improve - I'm fairly rusty on C#

string queryInsertString = "([TraceableItem].[IdentificationNo] LIKE ";
string lastBatchNo = batchnos.Last();
List<string> alreadyAdded = new List<string>();
bool firstLoop = true;
foreach(string item in batchnos)
            {
    string splitItem = item.Split('/')[1];
    if (!alreadyAdded.Contains(item)){
        if (!firstLoop)
        {
            queryInsertString += " OR [TraceableItem].[IdentificationNo] LIKE N'%" + splitItem + "'";
        } else
        {
            queryInsertString += "N'%" + splitItem + "'";
            firstLoop = false;
        }
        alreadyAdded.Add(item);
    }
}

Answer 1

Using String.Join makes this kind of string operation a breeze:

// DON'T USE THIS - SQL INJECTION
string predicates = string.Join(" OR ", 
        batchNos.Select(item => item.Split('/')[1])
                .Distinct()
                .Select(splitItem => $"[TraceableItem].[IdentificationNo] LIKE N'%{splitItem}'"));
string queryInsertString = "(" + predicates + ")";

You'll notice the all capital warning not to use that code. It's not just because I haven't even checked if it compiles/works, it's because this kind of thing introduces a SQL Injection Vulnerability.

What you need to do is read up on using SqlParameters in C#. You'll want to generate SQL that looks more like:

[TraceableItem].[IdentificationNo] LIKE @param1 OR [TraceableItem].[IdentificationNo] LIKE @param2

You then need to create a SqlParameter instance for each one and add that to the SqlCommand before you execute it. That's assuming you're using Ado.Net, if you're using a framework you'll have to look up the right way of doing it in the one you're using.

The danger is that if someone passes a batch number of "a/'); delete from Users; --", you'll end up creating a query that looks like:

WHERE ([TraceableItem].[IdentificationNo] LIKE N''); delete from Users; -- ...

You do not want a user to be able to do that!

Answer 2

Your SQL query is never going to be efficient because it has wildcard at the start, and is not Sargable (LIKE N'%015558').

Assuming your database allows it, what you should do in this scenario is to create a PERSISTED computed SQL column which is set to REVERSE(IdentificationNo), and create an index on that column.

Combine this with the advice from @RobH, and make your query something similar.

WHERE [ReversedIdentificationNo] LIKE + splitItem.Reverse() + '%'

This way, your query will use an index seek (instead of an index scan) and potentially be magnitudes faster.

EDIT: With a variable number of IdentificationNo that you intend of searching, you run into a cache pollution issue. For every distinct number of parameters, your SQL engine will not be able to reuse an existing query cache plan. This is very common issue when dynamically generating SQL statements (especially without SQL paramters).

To avoid this, you could use a temp table / table type.

CREATE TYPE Ids AS TABLE ( ID varchar(128) PRIMARY KEY )

Then, instead of constructing a bunch of OR statements, you just insert all your splitItem into this temp table. Finally, your query looks something like the following.

WHERE EXISTS (
    SELECT 1 
    FROM @Ids
    WHERE [ReversedIdentificationNo] LIKE [Id] + '%'
)