6

So, want to make a multi-row insert query, and I need to replace the keys with the values inside a loop where I have the values.

It was working by hardcoding the values into the query string, but I need to do it by using the "cmd.Parameters.AddValue() or cmd.Parameters.AddWithValue()" as I need to prevent SQL Injection.

So, my code is something like this:

         string query = "insert into dbo.Foo (column1, column2, column3) values ";    
         SqlCommand cmd
            foreach (line in rowsArray) {
                cmd.Parameters.Clear();
                cmd = new SqlCommand(query, cnn); //So, the problem is this override
                query += "(@key1, @key2, @key3), ";

                cmd.Parameters.AddWithValue("@key1", line.value1);
                cmd.Parameters.AddWithValue("@key2", line.value2);
                cmd.Parameters.AddWithValue("@key3", line.value3);
            } 
         query = query.Substring(0, query.Length-2); //Last comma
         cmd.ExecuteNonQuery();
         cnn.Close();

I want to ExecuteNonQuery(); outside the loop, to make just one insert.

Any ideas?

I thought about making a loop where I add the keys in the string with a identifier and then replacing all of them iterating another loop with the same id's, but I don't see that very efficient or a good practice.

Carlos López Marí
  • 1,432
  • 3
  • 18
  • 45
  • 1
    Give the parameters unique number indices, not `1, 2, 3` repeating. – GSerg May 13 '19 at 07:18
  • 1
    I'd probably just create a transaction, use multiple `ExecuteNonQuery` calls, and then commit the transaction at the end. There may well be better ways, but I'd at least expect that to work. Otherwise, you'd need to create multiple insert statements in your SQL, with uniquely-named parameters. – Jon Skeet May 13 '19 at 07:18
  • @JonSkeet But will the Execute method attack the database each time it's called? – Carlos López Marí May 13 '19 at 07:21
  • 1
    If you like, you can use `SqlBulkCopy` for bulk insert, use this link: https://stackoverflow.com/questions/3913371/sqlbulkcopy-from-a-list – hassan.ef May 13 '19 at 07:21
  • 4
    There is a limit to the length of a query that SQL Server can efficiently process, a hard limit of 1000 rows for the `VALUES` clause, and statements like these (with many different parameter counts) pollute the plan cache. Generally this approach is not worth it; use either a multi-statement transaction, a table-valued parameter or the `SqlBulkCopy` class. (And [don't use `AddWithValue`](https://blogs.msmvps.com/jcoehoorn/blog/2014/05/12/can-we-stop-using-addwithvalue-already/)). – Jeroen Mostert May 13 '19 at 07:22
  • @CarlosLópezMarí: Not sure. But I would generally try the simplest thing that could work first, and test whether it behaves as you want it to. – Jon Skeet May 13 '19 at 07:23

1 Answers1

0

I finnaly decided to make two loops as It worked better than expected.

I thought about making a loop where I add the keys in the string with a identifier and then replacing all of them iterating another loop with the same id's, but I don't see that very efficient or a good practice.

With this approach I finish the string query and then I add the values. I give the keys ids so I can replace them with the values in order in the next loop using the same id.

            string query = "insert into dbo.Foo (column1, column2, column3) values ";    
         
            int id = 0;
            foreach (line in rowsArray) {
                query += "(@key1"+id+", @key2"+id+", @key3"+id+"), ";
                id++;
            }
            query = query.Substring(0, query.Length-2); //Last comma


            SqlCommand cmd = new SqlCommand(query, cnn);
            id = 0;
            foreach (line in rowsArray) {
                cmd.Parameters.AddWithValue("@key1"+id, line.value1);
                cmd.Parameters.AddWithValue("@key2"+id, line.value2);
                cmd.Parameters.AddWithValue("@key3"+id, line.value3);
            } 
         
            cmd.ExecuteNonQuery();
            cnn.Close();

Also, mention the existance of SqlBulkCopy, which allows to upload DataTables and will result in a cleaner code.

Carlos López Marí
  • 1,432
  • 3
  • 18
  • 45
  • And you still refuse to learn how to parameterize correctly - don't use [addwithvalue](http://www.dbdelta.com/addwithvalue-is-evil/). – SMor May 13 '19 at 11:37
  • @SMor That's interesting but It was not the cause of the problem and I was not asking that so don't be rude with that topic as it has no influence in the solution. I don't refuse to learn anything, It's just that nobody has told me "AddWithValue is evil" before. – Carlos López Marí May 13 '19 at 11:45
  • 1
    Hell Carlos, have you considered using "User Defined Table Types", in this way you can create a table and insert your values in the table, after that send your table as parameter, in you SP, do a Insert using select statement – Antonio Avndaño Duran May 13 '19 at 15:50