1

I am trying to insert into table from array of Json as well as select records from SQL Server DB table. When executing the below method it is almost taking more than 10 minutes.

public async Task CreateTableAsync(string formsJson, string connectionString)
{
    SqlConnection con = new SqlConnection(connectionString);
    List<FormsJson> listOfformsJson = JsonConvert.DeserializeObject<List<FormsJson>>(formsJson);
    foreach (var form in listOfformsJson)
    {
        string formId = Guid.NewGuid().ToString();
        //insert into forms Table
        string formQuery = "insert into Forms([FormId]) values(@FormId)";
        using (var cmd = new SqlCommand(formQuery, con))
        {
            cmd.CommandTimeout = 120;
            //Pass values to Parameters
            cmd.Parameters.AddWithValue("@FormId", formId);
            if (con.State == System.Data.ConnectionState.Closed)
            {
                con.Open();
            }
            cmd.ExecuteNonQuery();
        }

        //relationship between forms and ETypes,get all the eTypes and fill
        foreach (var typeOf in form.TypeOf)
        {
            //get all the eTypeIds for this typeof field
            string query = "select Id from ETypes Where TypeOf = @typeOf";
            List<string> eTypeIdList = new List<string>();
            using (var sqlcmd = new SqlCommand(query, con))
            {
                sqlcmd.CommandTimeout = 120;
                //Pass values to Parameters
                sqlcmd.Parameters.AddWithValue("@typeOf", typeOf);
                if (con.State == System.Data.ConnectionState.Closed)
                {
                    con.Open();
                }
                SqlDataReader sqlDataReader = sqlcmd.ExecuteReader();
                while (sqlDataReader.Read())
                {
                    string eTypeId = sqlDataReader[0].ToString();
                    eTypeIdList.Add(eTypeId);
                }
                sqlDataReader.Close();
            }
            //insert into Forms ETypes Relationship
            string fe_query = "";
            foreach (var eTypeId in eTypeIdList)
            {
                fe_query = "insert into Forms_ETypes([Form_Id],[EType_Id]) values (@Form_Id,@EType_Id)";
                if (con.State == System.Data.ConnectionState.Closed)
                {

                    con.Open();
                }
                using (var fesqlcmd = new SqlCommand(fe_query, con))
                {
                    fesqlcmd.CommandTimeout = 120;
                    //Pass values to Parameters
                    fesqlcmd.Parameters.AddWithValue("@Form_Id", formId);
                    fesqlcmd.Parameters.AddWithValue("@EType_Id", eTypeId);
                    fesqlcmd.ExecuteNonQuery();
                }
            }
        }
    }
}

Outer foreach(...listofformsJson) loop more than hundreds of records.
And same for the inner loop around hundreds of rows.

In between commandTimeout, keeping the open connection with server statements. Any help to optimize the time and remove/add ADO statements.

Steve
  • 213,761
  • 22
  • 232
  • 286
cj devin
  • 1,045
  • 3
  • 13
  • 48
  • Start moving all the invariant code out of the loops. SqlCommands can be prepared only one time and then you can simply change the parameters values. – Steve Oct 27 '18 at 20:41
  • You might find that utilising a Stored Procedure for each of these SQL commands would be more efficient that creating the SQL text yourself. It gives the database the opportunity to store a preset execution plan for repetitive queries which save the calculation time involved – Martin Oct 27 '18 at 20:45
  • I would point my finger also to those AddWithValue. They are a road block to reuse the query plans prepared by sql server. Instead creating the parameter (in particular string parameters) specifying the same size for the strings allow the optimization engine inside sql server to recognize and reuse the same query plan just changing the parameter value and not reparsing everything – Steve Oct 27 '18 at 21:00
  • How many rows are in the ETypes table? It may be more worth it to just grab all the values first and store those in an array then do lookups off the array rather than hitting SQL over and over for those types. Local array lookups would be MUCH faster. – Element Zero Oct 27 '18 at 21:01
  • @MartinParkin `It gives the database the opportunity to store a preset execution plan for repetitive queries which save the calculation time involved` - https://stackoverflow.com/questions/8559443/why-execute-stored-procedures-is-faster-than-sql-query-from-a-script – mjwills Oct 27 '18 at 21:14
  • @ElementZero How many rows are in the ETypes table?..right now around two handrads of rows is there in ETypes table. – cj devin Oct 27 '18 at 21:44
  • 1
    I suggest you to follow the advices given in the @mjwills answer below. In any case you should build an SqlCommand for each query required, create the proper parameters with command.Parameters.Add("@name", type, size) before entering the loops and inside the loops just set the parameter value (command.Parameters["@name"].Value=xxxx). – Steve Oct 27 '18 at 21:46
  • 1
    As a side note the very important object that benefits of the _using_ statement is the connection and you haven't put it in a using block – Steve Oct 27 '18 at 21:48

1 Answers1

3

The primary issue here is that you are pulling all of the data out of the database and then, row by row, inserting it back in. This is not optimal from the database's point of view. It is great at dealing with sets - but you are treating the set as lots of individual rows. Thus it becomes slow.

From a set-based standpoint, you have only two statements that you need to run:

  1. Insert the Forms row
  2. Insert the Forms_ETypes rows (as a set, not one at a time)

1) should be what you have now:

insert into Forms([FormId]) values(@FormId)

2) should be something like:

insert Forms_ETypes([Form_Id],[EType_Id]) SELECT @FormId, Id from ETypes Where TypeOf IN ({0});

using this technique to pass in your form.TypeOf values. Note this assumes you have fewer than 500 entries in form.TypeOf. If you have many (e.g. greater than 500) then using a UDT is a better approach (note some info on UDTs suggest that you need to use a stored proc, but that isn't the case).

This will enable you to run just two SQL statements - the first, then the second (vs possibly thousands with your current solution).

This will save time for two reasons:

  1. The database engine didn't need to pass the data over the wire twice (from your DB server to your application, and back again).
  2. You enabled the database engine to do a large set based operation, rather than lots of smaller operations (with latency due to the request-response nature of the loop).
mjwills
  • 23,389
  • 6
  • 40
  • 63