CODEWITHSUNDEEP
c#sqlsql-injection
bottaio
bottaio

Reputation: 5093

Using SqlCommand as multidelete statement

I want to delete rows from database based on their Id. Let's look at my current code

private static SqlCommand GenerateDeleteCommand(SqlConnection connection, string table, IEnumerable<int> ids)
{
    var idList = ids.ToList();
    var command = new SqlCommand($"delete from {table} where Id in ({string.Join(", ", idList)}", connection);
    return command;
}

It probably doesn't need any explaination. However, I tried to use placeholders (mostly preferred way)

private static SqlCommand GenerateDeleteCommand2(SqlConnection connection, string table, IEnumerable<int> ids)
{
    var idList = ids.ToList();
    var command = new SqlCommand($"delete from {table} where Id in ({string.Join(", ", idList.Select(x => $"@{x}"))})", connection);

    idList.ForEach(x => command.Parameters.Add($"@{x}", SqlDbType.Int));
    idList.ForEach(x => command.Parameters[$"@{x}"].Value = x);

    return command;
}

But this just seems stupid to me. Nothing really can go wrong in this scenario with respect to the data type. I am passing strongly typed collection here so the values must be integers and there is no risk of SQL Injection. Of course, passing a table name might be risky but let's assume that we are always passing one of the predefined const strings containing table names.

My question is, do we have any advantage in the second scenario (it costs us some performance loss).

Upvotes: 0

Views: 65

Answers (1)

Igor
Igor

Reputation: 62238

Sql Server and most other database providers have to generate a query plan for each incoming query. These query plans can be reused, the query plans are matched to the incoming query text. So if you execute a query that uses parameters the query text itself stays the same even when you use different parameters. This allows Sql Server to cache the plan as well as create and maintain statistics on the query execution. If you inject your parameter values into the query string a new plan has to be created each time the query is executed which is much less efficient.

For more details on this issue see Execution Plan Caching and Reuse

Upvotes: 1

Related Questions