Optimize store proc performance being called in c# on array loop

Question

I have a c# application(mvc) that allows a user to upload an excel sheet containing an account number key and amount value to be passed as parameters to a proc that then validates those records, the proc then return an error or warning if any are found. My issue is that the BI team implemeted a proc which I cannot modify to make it accept a table type as explained in this post Table Value

The issue is that I cannot modify the proc to accept an table type, so I need to optimize the stored procedure call in c# code. The performance is acceptable if there are about 30 to 50 lines the proc needs to validate, but once there are more record e.g a 100 or more the application is slow, is there a way in c# that I can improve the performance of this code, please see my implementation below.

using (SqlConnection conn = new SqlConnection(sqlConnection))
    {

        try
        {

            foreach (var claim in supplierClaimsData)
            {
                SqlCommand cmd = new SqlCommand();
                cmd.CommandTimeout = 60;
                SqlDataReader reader;
                cmd.Connection = conn;
                cmd.CommandText = "CRM.Supplier_Claim_Upload";
                cmd.CommandType = CommandType.StoredProcedure;

                cmd.Parameters.Add("@Invoice", SqlDbType.NVarChar).Value = claim.Line_Number;
                cmd.Parameters.Add("@Amount", SqlDbType.Decimal).Value = claim.Total_Claim;

                conn.Open();
                reader = cmd.ExecuteReader();

                while (reader.Read())
                {
                    claim.Error_1 = reader.GetString(reader.GetOrdinal("Error1"));

                    string lineNumberDoesNotExist = "Error: Invoice line number does not exist";
                    if (claim.Error_1.StartsWith(lineNumberDoesNotExist))
                    {
                        continue;
                    }

                    claim.Warning = reader.GetString(reader.GetOrdinal("Warning"));
                    claim.Error_2 = reader.GetString(reader.GetOrdinal("Error2"));

                }


                conn.Close();


            }


            foreach (CleanSupplierClaim saveToDBClaim in supplierClaimsData)
            {
                db.CleanSupplierClaims.Attach(saveToDBClaim);

                var entry = db.Entry(saveToDBClaim);
                entry.Property(aa => aa.Line_Number).IsModified = true;
                entry.Property(aa => aa.Total_Claim).IsModified = true;
                entry.Property(aa => aa.Currency).IsModified = true;
                entry.Property(aa => aa.ClaimReference).IsModified = true;
                entry.Property(aa => aa.Action).IsModified = true;
                entry.Property(aa => aa.Domain_Username).IsModified = true;
                entry.Property(aa => aa.Error_1).IsModified = true;
                entry.Property(aa => aa.Error_2).IsModified = true;
                entry.Property(aa => aa.Warning).IsModified = true;
                entry.Property(aa => aa.ImportFlag).IsModified = true;
                entry.Property(aa => aa.ReadyForImport).IsModified = true;
                db.Entry(saveToDBClaim).State = System.Data.Entity.EntityState.Modified;
                db.SaveChanges();


            }


        }

I would really appreciate your input in this regard.

Here is the procedure I got from the QA sql, it basically validates claims with Invoice number and Amount as parameters.

    ALTER PROCEDURE [CRM].[Supplier_Claim_Upload] 
(
@Invoice        varchar(20), 
@Amount         decimal(18,3) 
)
as
set nocount on

declare         @ST_Key varchar(20)     = @Invoice
declare         @Claim decimal(18,3)    = @Amount

select * into #Temp
from (
        select st.ST_Key, 
                case when sm.Supplier_Claim_Original > 0 then 'Warning: You Are going to Overwrite a Claim Value: ' + convert(varchar(50), round(sm.Supplier_Claim_Original, 0)) else '' end as 'Warning',
                case when st.Orig_Inv is not null then 'Error: You are attempting to claim against a returned invoice: ' + st.Orig_Inv else '' end as 'Error1',
                case when  @Claim > sm.System_Cost then 'Error: Claim value is higher than system cost: ' + convert(varchar(50), sm.System_Cost) else '' end as 'Error2'
        from Embrace.fact.Sales st
        left join Embrace.fact.Sales_Margin sm on sm.ST_Key = st.ST_Key and sm.Tarsus_Country = st.Tarsus_Country
        where st.ST_Key = @ST_Key
) x

if not exists (select top 1 * from #Temp)
begin
    select @ST_Key as 'ST_Key', 'Error: Invoice line number does not exist' as 'Error1'
end

else

begin
    select * from #Temp 
end

drop table #Temp

--Action    
--Line No   
--Total Claim   
--Currency  
--Claim Reference


set nocount off

Answer 1

I think this is a very strange way to validate anything and it's extremely unmaintainable.

Why don't you (the BI team) just return an error code (and maybe additional information in other columns). The error messages are no help at all because you need to use the same strings in your application to check which error message it is. If they correct a typo or change a letter or two or add another word then your app doesn't work anymore.

Even more strange the error code are actually your columns. So you have three error codes (columns): Warning, Error1, Error2 where the Error1 can be either there is no invoice or you are attempting to claim....

If you used error codes you could define an enum or constants and you instantly would know what's going on with the invoce.

---

As to the optimization... You cannot optimize it if you have to ask the database for each invoice. Tell the BI team that they should let you pass a bunch of invoice numbers (maybe a 100 or more at a time) to the stored procedure and give you back the error codes for them. This way you would already have saved a 100 database roundtrips.

There are several ways to accomplish it, see How to pass an array into a SQL Server stored procedure;

---

You shouldn't do everything on each loop. Open the connection once before the loop, create the command once before the loop starts too. You can even add the parameters. Inside the loop you should only set their values:

using (var cmd = new SqlCommand())
{
    cmd.CommandTimeout = 60;
    cmd.Connection = conn;
    cmd.CommandText = "CRM.Supplier_Claim_Upload";
    cmd.CommandType = CommandType.StoredProcedure;

    var invoiceParam = cmd.Parameters.Add("@Invoice", SqlDbType.NVarChar);
    var amountParam = cmd.Parameters.Add("@Amount", SqlDbType.Decimal);

    conn.Open();

    foreach (var claim in supplierClaimsData)
    {

        invoiceParam.Value = claim.Line_Number;
        amountParam.Value = claim.Total_Claim;

        using (var reader = cmd.ExecuteReader())
        {
            while (reader.Read())
            {
                ...
            }
        }
    }
}

Answer 2

Well, I don't see any

using

around

new SqlCommand()

I would try to use a basic using pattern around the new SqlCommand and would test it. SqlCommand inherits from DbCommand and implements IDisposable, so I will at least try to call a Cmd.dispose() in such a case.

Hope this helps