0

I have delete an application in c# and i utilize linq to entities to build queries in my application. However I find my application to be very slow in adding and deleting records. Delete 500 records would take me about 15 mins. That is too long. my delete query is below.

public void deletePayrollBatch(int batchNo) 
{
          var context = new DataSources.sspEntities();
          var query1 = (from a in context.Payroll_Details
                        where a.Payroll_BatchBatch_no == batchNo
                        select a).ToList();

           foreach (var x in query1) 
           {
                context.DeleteObject(x);
                context.SaveChanges();
           }
}

My query is rather simple. One row of data does contain about 20 columns. I dont know if that would have an effect. Anyhow, if i can get some expert suggestion, it would be helpful. thank you.

EDITED.

I already had good suggestions based on the delete. One of which is to used context.SaveChanges() outside of the loop. In my insert, i have similar code (shown below), however I need the record to be save first because i want the ID to use it in another function. In this case, how may i proceed?

foreach (var xyz in x)
{
   Try{
          pDetails1.EstateID = 143;
          pDetails1.Employee_Personal_InfoEmp_id = xyz.PD_EmpPersonal_Id;
          pDetails1.PAYE = (decimal)xyz.PD_PAYE.GetValueOrDefault();
          pDetails1.PAYE_YTD = (decimal)PayeYTD.GetValueOrDefault();
          pDetails1.Basic_Pay = (decimal)xyz.PD_BasicPay.GetValueOrDefault();
          pDetails1.Basic_Pay_YTD = (decimal)basicpay_YTD.GetValueOrDefault();
          pDetails1.Gross_Pay = (decimal)xyz.PD_GrossPay.GetValueOrDefault();
          pDetails1.Gross_Pay_YTD = (decimal)GrossPay_YTD.GetValueOrDefault();

          context.SaveChanges();
          Functions.RandomFunct rf = new RandomFunct();
          rf.AdditionalEarningsIsContinuous(xyz.PD_EmpPersonal_Id); // I NEED THE ID IN ORDER TO RUN THIS FUNCTION. IN THIS CASE I HAVE TO SAVE ON EACH ELEMENT IN THE FOR EACH LOOP??

    }
    catch (exeception e)
        debug.writeline(e.message);
    }
Jim Wooley
  • 10,169
  • 1
  • 25
  • 43
Maverick1415
  • 113
  • 1
  • 12

1 Answers1

7

You are calling context.SaveChanges for every record. You can do that once at the end of your foreach loop.

foreach (var x in query1)
{
   context.DeleteObject(x);
}
context.SaveChanges();

That will send the request to delete once to the database for all records. Currently you are sending DELETE request for each record and that is taking time.

Habib
  • 219,104
  • 29
  • 407
  • 436
  • 4
    @Maverick1415 Moreover...why `ToList()`? Keep `IEnumerable` and objects won't all be loaded in a list in memory. LINQ **may** even optimize your query to perform only `DELETE Payroll_Details WHERE Payroll_BatchBatch_no == batchNo` instead of a `DELETE Payroll_Details WHERE ID IN (SELECT ID FROM Payroll_Details WHERE Payroll_BatchBatch_no == batchNo)` – Adriano Repetti Mar 14 '14 at 15:20
  • @Adriano - you should add that as an answer – Chris Ballard Mar 14 '14 at 15:23
  • @ChrisBallard it's just a guess, (and Habib is pretty right about `SaveChanges()` too). Someone should check and measure! – Adriano Repetti Mar 14 '14 at 15:24
  • This discussion is relevant http://stackoverflow.com/questions/869209/bulk-deleting-in-linq-to-entities – Chris Ballard Mar 14 '14 at 15:31
  • @Adriano No, it cannot perform the second optimization you mention. The `foreach` will force the query to be executed at that point in time. It's *possible* for EF to create a `DeleteAll` extension method on `IQueryable`, adjust the query provider to understand that method, and then convert it into a query like you mentioned, but currently none of that is in place. – Servy Mar 14 '14 at 15:44
  • I made an update to the question above. If you can take a look i would be greatful. thank you. @Adriano you are saying do not use .ToList() .. just leave it as is? How would i iterate through the values then? – Maverick1415 Mar 14 '14 at 15:44
  • @Habib Also note that if you try to act on too many items at once you can run into problems. Generally you'll want to batch the items into batches of some size that can be efficiently submitted to the database, rather than always batching *everything*. Obviously if the whole query is always sufficiently small, that's not necessary. – Servy Mar 14 '14 at 15:45
  • @Maverick1415 `foreach` iterates through the values. It doesn't need to be a list to be enumerable. `IQueryable` *already* provides all of the needed methods for it to be iterated. – Servy Mar 14 '14 at 15:46
  • @Maverick1415, you can't get an ID without calling `SaveChanges`, So you can't use the same rule in your INSERT code. Also consider Servy's suggestion for sending a batch of records for deletion depending on the number of records returned for deletion. – Habib Mar 14 '14 at 15:50
  • @Habib thank you. you dont have any suggestion for the insertion or i have to stick to the same method i am using? – Maverick1415 Mar 14 '14 at 15:52
  • @Maverick1415 Just call all of those methods in a second loop after you've done all of the updates, rather than calling it after each update. Either that or consider making a function/stored procedure on the DB end that does all of the things that you need to do for an item. – Servy Mar 14 '14 at 15:53
  • @Servy okay .. i was thinking about making a second loop. didnt know if it wouldve been the most efficient way of doing things but thank you very much. – Maverick1415 Mar 14 '14 at 15:54
  • @Maverick1415 Performing two DB round trips vs N. Yeah, I'd say a second loop is likely to be more efficient. By a *lot*. – Servy Mar 14 '14 at 15:57
  • @Servy you're right, with a plain foreach LINQ will go out of games. I'm surprised about batches, if needed isn't it managed internally by SaveChanges? When objects are all in memory after a foreach then we shouldn't care about it! :| – Adriano Repetti Mar 14 '14 at 16:40
  • 1
    @Adriano `SaveChanges` *should* batch the data on its own, yes. In my personal experience with quite a number of different query providers, I have yet to find one that does. If/when the query providers start handling this on their own, it would remove the need to explicitly batch. – Servy Mar 14 '14 at 16:42
  • @Servy "It's possible for EF to create a DeleteAll extension method on IQueryable" .. i was wondering if you assist me to do this. thank you. – Maverick1415 Mar 14 '14 at 17:16
  • @Maverick1415 Entity framework could write this. You couldn't. Or at least, you could write the method, but you couldn't alter the query provider such that it would understand it. So unless you plan to write your own query provider, that's not a feasible option for you. – Servy Mar 14 '14 at 17:29
  • @Adriano I tried your method above to delete. However when delete a lot of record, i cannot query the table in sql management studio. is this normal? – Maverick1415 Mar 19 '14 at 17:25
  • @Habib i tried your method above to delete. However when delete a lot of record, i cannot query the table in sql management studio. is this normal? – Maverick1415 Mar 19 '14 at 17:25
  • @Maverick1415, yes that is normal, probably the table is getting locked. You can either delete in batches, or use SET TRANSACTION LEVEL READ UNCOMMITTED in Management studio before executing your query. You should also look into making and sending batches for deletion instead of sending all records for deletion. – Habib Mar 19 '14 at 17:27