Inserting Multiple Records into SQL Server database using for loop

Question

I am working on a Windows form project in C#. I am trying to insert multiple records into the SQL Server database from an array.

After entering the first row I get an exception

@UserID has already been declared. Variable names must be unique within a query batch or stored procedure.

There is no issue with the primary key in the database as UserID is not the primary key.

This is what I am trying to do.

public static void featuresentry()
{
    SqlConnection connection = new SqlConnection(HandVeinPattern.Properties.Settings.Default.HandVeinPatternConnectionString);

    SqlCommand command = new SqlCommand();
    connection.Open();

    try
    {
        command = connection.CreateCommand();

        for (int i = 0; i < Details.modelKeyPoints.Size; i++)
        {
            command.CommandText = "INSERT INTO FEATURES(UserID, Angle, ClassID, Octave, PointX, PointY, Response, Size) VALUES(@UserID, @Angle, @ClassID, @Octave, @PointX, @PointY, @Response, @Size)";

            command.Parameters.AddWithValue("@UserID", Details.ID);
            command.Parameters.AddWithValue("@Angle", Convert.ToDouble(Details.modelKeyPoints[i].Angle));
            command.Parameters.AddWithValue("@ClassID", Convert.ToDouble(Details.modelKeyPoints[i].ClassId));
            command.Parameters.AddWithValue("@Octave", Convert.ToDouble(Details.modelKeyPoints[i].Octave));
            command.Parameters.AddWithValue("@PointX", Convert.ToDouble(Details.modelKeyPoints[i].Point.X));
            command.Parameters.AddWithValue("@PointY", Convert.ToDouble(Details.modelKeyPoints[i].Point.Y));
            command.Parameters.AddWithValue("@Response", Convert.ToDouble(Details.modelKeyPoints[i].Response));
            command.Parameters.AddWithValue("@Size", Convert.ToDouble(Details.modelKeyPoints[i].Size));

            command.ExecuteNonQuery();
        }
    }
    catch (Exception)
    {
        throw;
    }
    finally
    {
        if (connection.State == ConnectionState.Open)
        {
            connection.Close();
        }
    }
}

Solution 1

You should do this properly:

• define your parameters once outside the loop
• define the values of your parameters inside the loop for each iteration
• use using(...) { ... } blocks to get rid of the try ... catch ... finally (the using block will ensure proper and speedy disposal of your classes, when no longer needed)
• stop using a try...catch if you're not actually handling the exceptions - just rethrowing them (makes no sense)

Try this code:

public static void featuresentry()
{
    string connectionString = HandVeinPattern.Properties.Settings.Default.HandVeinPatternConnectionString;
    string insertQuery = "INSERT INTO FEATURES(UserID, Angle, ClassID, Octave, PointX, PointY, Response, Size) VALUES(@UserID, @Angle, @ClassID, @Octave, @PointX, @PointY, @Response, @Size)";

    using (SqlConnection connection = new SqlConnection(connectionString))
    using (SqlCommand command = new SqlCommand(insertQuery, connection))
    {
        // define your parameters ONCE outside the loop, and use EXPLICIT typing
        command.Parameters.Add("@UserID", SqlDbType.Int);
        command.Parameters.Add("@Angle", SqlDbType.Double);
        command.Parameters.Add("@ClassID", SqlDbType.Double);
        command.Parameters.Add("@Octave", SqlDbType.Double);
        command.Parameters.Add("@PointX", SqlDbType.Double);
        command.Parameters.Add("@PointY", SqlDbType.Double);
        command.Parameters.Add("@Response", SqlDbType.Double);
        command.Parameters.Add("@Size", SqlDbType.Double);

        connection.Open();

        for (int i = 0; i < Details.modelKeyPoints.Size; i++)
        {
            // now just SET the values
            command.Parameters["@UserID"].Value = Details.ID;
            command.Parameters["@Angle"].Value = Convert.ToDouble(Details.modelKeyPoints[i].Angle);
            command.Parameters["@ClassID"].Value = Convert.ToDouble(Details.modelKeyPoints[i].ClassId);
            command.Parameters["@Octave"].Value = Convert.ToDouble(Details.modelKeyPoints[i].Octave);
            command.Parameters["@PointX"].Value = Convert.ToDouble(Details.modelKeyPoints[i].Point.X);
            command.Parameters["@PointY"].Value = Convert.ToDouble(Details.modelKeyPoints[i].Point.Y);
            command.Parameters["@Response"].Value = Convert.ToDouble(Details.modelKeyPoints[i].Response);
            command.Parameters["@Size"].Value = Convert.ToDouble(Details.modelKeyPoints[i].Size);

            command.ExecuteNonQuery();
        }
    }
}

Solution 2

If you put command = connection.CreateCommand(); inside your for loop, it will work. The problem is you are looping over the command parameters only, so its trying to add more parameters to your existing command, but theyre already in there. So you need to make a new command every loop instead.

Solution 3

In order to obtain the maximum performance, you may consider a BulkInsert. This ensures that your insert are done as fast as possible, as any issued query has some overhead (a large query will generally execute faster than many small ones). It should look something like the following:

1) define AsDataTable extension method from here:

   public static DataTable AsDataTable<T>(this IEnumerable<T> data)
   {
       PropertyDescriptorCollection properties = TypeDescriptor.GetProperties(typeof(T));
       var table = new DataTable();
       foreach (PropertyDescriptor prop in properties)
           table.Columns.Add(prop.Name, Nullable.GetUnderlyingType(prop.PropertyType) ?? prop.PropertyType);
       foreach (T item in data)
       {
           DataRow row = table.NewRow();
           foreach (PropertyDescriptor prop in properties)
               row[prop.Name] = prop.GetValue(item) ?? DBNull.Value;
           table.Rows.Add(row);
       }
       return table;
   }

2) execute the actual BulkInsert like this (not tested):

using (SqlConnection connection = new SqlConnection(connectionString))
{
     connection.Open();
     SqlTransaction transaction = connection.BeginTransaction();

     using (var bulkCopy = new SqlBulkCopy(connection, SqlBulkCopyOptions.Default, transaction))
     {
        bulkCopy.BatchSize = 100;
        bulkCopy.DestinationTableName = "dbo.FEATURES";
        try
        {
            // define mappings for columns, as property names / generated data table column names
            // is different from destination table column name
            bulkCopy.ColumnMappings.Add("ID","UserID");
            bulkCopy.ColumnMappings.Add("Angle","Angle");
            // the other mappings come here

            bulkCopy.WriteToServer(Details.modelKeyPoints.AsDataTable());
        }
        catch (Exception)
        {
            transaction.Rollback();
            connection.Close();
        }
      }

      transaction.Commit();
}

Of course, if convention over configuration would be used (object properties names would match exactly destination table column names), no mapping would be required.

Comments

[Zohar Peled]

Or even better, use a table valued parameter and execute just a single statement...

[Mr. Boy]

@ZoharPeled it's a shame you didn't provide an answer along those lines!