Make this SQL query more efficient

Question

I have the following query which checks goods receipts against purchase orders to see what items were originally ordered and how many have been booked in via goods receipts. E.g I place a purchase order for 10 banana milkshakes, I then generate a goods receipt stating that I received 5 of these milkshakes on said purchase order.

SELECT t.PONUM, t.ITMNUM, t.ordered, 
       SUM(t.received) as received, 
       t.ordered - ISNULL(SUM(t.received),0) as remaining, 
       SUM(t.orderedcartons) as orderedcartons, 
       SUM(t.cartonsreceived) as cartonsreceived, 
       SUM(t.remainingcartons) as remainingcartonsFROM(SELECT pod.PONUM, 
       pod.ITMNUM, pod.QTY as ordered, ISNULL(grd.QTYRECEIVED, 0) as received, 
       pod.DELIVERYSIZE as orderedcartons, 
       ISNULL(grd.DELIVERYSIZERECEIVED, 0) as cartonsreceived, 
       (pod.DELIVERYSIZE - ISNULL(grd.DELIVERYSIZERECEIVED, 0)) as remainingcartons
FROM TBLPODETAILS pod 
  LEFT OUTER JOIN TBLGRDETAILS grd 
    ON pod.PONUM = grd.PONUM and pod.ITMNUM = grd.ITMNUM) t
GROUP BY t.ITMNUM, t.PONUM, t.ordered
ORDER BY t.PONUM

Which returns the following data:

PONUM   ITMNUM  ordered received remaining orderedcartons cartonsreceived remainingcartons

1       1     5.0000    3.0000      2.0000   5.0000         3.0000          2.0000

Next I have a C# loop to generate update queries based on the data I get back from the above query:

foreach (DataRow POUpdate in dt.Rows) {...

query += "UPDATE MYTABLE SET REMAININGITEMS=" + remainingQty.ToString() 
       + ", REMAININGDELIVERYSIZE=" + remainingBoxes.ToString() + " WHERE ITMNUM=" 
       + itemNumber + " AND PONUM=" + poNumber + ";";

I then execute each update query against the DB. Which works fine on my local dev machine.

However deploying to production server pulls back over 150,000 records on that first query.

So looping around so many rows locks up SQL and my app. Is it the foreach? Is it the original select loading all that data into memory? Both? Can I make this query into one single query and cut out the C# loop? If so what's the most efficient way to achieve this?

Answer 1

In SQL, the goal should be to write operations on entire tables at once. The SQL server can be very efficient at doing so, but will need a significant overhead on any interaction, since it needs to deal with consistency, atomicity of transactions, etc. So in a way, your fixed cost per transaction is high, for the server to do its thing, but your marginal cost for additional rows in a transaction is very low - updating 1m rows may be 1/2 as fast as updating 10.

This means that the foreach is going to cause the SQL server to constantly go back and forth with your application, and that fixed cost of locking/unlocking and doing transactions is being occurred every time.

Can you write the query to operate in SQL, instead of manipulating data in C#? It seems you want to write a relatively simple update based on your select statement (See, for instance, SQL update from one Table to another based on a ID match.

Try something like the following (Not code tested, since i don't have access to your database structure, etc.):

UPDATE MYTABLE 
  SET REMAININGITEMS = remainingQty, 
  REMAININGDELIVERYSIZE=remainingBoxes
From 
(SELECT t.PONUM, t.ITMNUM, t.ordered, 
       SUM(t.received) as received, 
       t.ordered - ISNULL(SUM(t.received),0) as remaining, 
       SUM(t.orderedcartons) as orderedcartons, 
       SUM(t.cartonsreceived) as cartonsreceived, 
       SUM(t.remainingcartons) as remainingcartonsFROM(SELECT pod.PONUM, 
       pod.ITMNUM, pod.QTY as ordered, ISNULL(grd.QTYRECEIVED, 0) as received, 
       pod.DELIVERYSIZE as orderedcartons, 
       ISNULL(grd.DELIVERYSIZERECEIVED, 0) as cartonsreceived, 
       (pod.DELIVERYSIZE - ISNULL(grd.DELIVERYSIZERECEIVED, 0)) as remainingcartons
FROM TBLPODETAILS pod 
  LEFT OUTER JOIN TBLGRDETAILS grd 
    ON pod.PONUM = grd.PONUM and pod.ITMNUM = grd.ITMNUM) t
GROUP BY t.ITMNUM, t.PONUM, t.ordered
ORDER BY t.PONUM ) as x

join MYTABLE on MYTABLE.ITMNUM=x.itmnum AND MYTABLE.PONUM=i.ponum

Answer 2

As KM says in the comments, the problem here is coming back to the client app, and to then operate on each row with another database trip. That's slow, and can lead to stupid little bugs, which could cause bogus data.

Also, concatenating strings into SQL as you're doing is generally considered a very bad idea - SQL Injection (as Joel Coehoorn writes) is a real possibility.

How about:

create view OrderBalance 
as 
SELECT t.PONUM, t.ITMNUM, t.ordered, 
       SUM(t.received) as received, 
       t.ordered - ISNULL(SUM(t.received),0) as remaining, 
       SUM(t.orderedcartons) as orderedcartons, 
       SUM(t.cartonsreceived) as cartonsreceived, 
       SUM(t.remainingcartons) as remainingcartonsFROM(SELECT pod.PONUM, 
       pod.ITMNUM, pod.QTY as ordered, ISNULL(grd.QTYRECEIVED, 0) as received, 
       pod.DELIVERYSIZE as orderedcartons, 
       ISNULL(grd.DELIVERYSIZERECEIVED, 0) as cartonsreceived, 
       (pod.DELIVERYSIZE - ISNULL(grd.DELIVERYSIZERECEIVED, 0)) as remainingcartons
FROM TBLPODETAILS pod 
  LEFT OUTER JOIN TBLGRDETAILS grd 
    ON pod.PONUM = grd.PONUM and pod.ITMNUM = grd.ITMNUM) t
GROUP BY t.ITMNUM, t.PONUM, t.ordered

This seems to have exactly the data that your "MYTABLE" has - maybe you don't even need MYTABLE anymore, and you can just use the view!

If you have other data on MYTABLE, your update becomes:

UPDATE MYTABLE 
SET REMAININGITEMS       = ob.remainingitems, 
    REMAININGDELIVERYSIZE = ob.remainingBoxes
from MYTABLE mt 
   join OrderBalance ob 
on mt.ITMNUM = ob.itemNumber 
AND mt.PONUM = ob.poNumber

(Although, as David Mannheim writes, it may be better to not use a view and use a solution similar to the one he proposes).

Answer 3

The other answers show you a great way to perform the whole update entirely in RDBMS. If you can do it like that, that's the perfect solution: you cannot beat it with a C# / RDBMS combination because of extra roundtrips and data transfer issues.

However, if your update requires some calculations that for one reason or the other cannot be performed in RDBMS, you should modify your code to construct a single parameterized update in place of a potentially gigantic 150000-row update that you are currently constructing.

using (var upd = conn.CreateCommand()) {
    upd.CommandText = @"
        UPDATE MYTABLE SET
            REMAININGITEMS=@remainingQty
        ,   REMAININGDELIVERYSIZE=@remainingBoxes
        WHERE ITMNUM=@itemNumber AND PONUM=@poNumber";
    var remainingQtyParam = upd.CreateParameter();
    remainingQtyParam.ParameterName = "@remainingQty";
    remainingQtyParam.DbType = DbType.Int64; // <<== Correct for your specific type
    upd.Parameters.Add(remainingQtyParam);
    var remainingBoxesParam = upd.CreateParameter();
    remainingBoxesParam.ParameterName = "@remainingBoxes";
    remainingBoxesParam.DbType = DbType.Int64; // <<== Correct for your specific type
    upd.Parameters.Add(remainingBoxesParam);
    ...
    foreach (DataRow POUpdate in dt.Rows) {
        remainingQtyParam.Value = ...
        remainingBoxesParam.Value = ...
        upd.ExecuteNonQuery();
    }
}

The idea is to make 150,000 updates that look the same into a single parameterized update that is actually a single statement.