Is there any way to make this query faster and build where clause outside of loop?

Question

I have the following code, (and I am completely aware about parameterized queries and SQL Injection):

foreach(var item in items)
{
    string query = "select sum(convert(decimal(18,3),tbl.Price)) p, sum(convert(decimal(18,2),tbl.Sale)) s from table1 tbl " +
               $"where tbl.ID = {item .ID}";
    Execute(query);
    //Do stuff with query result
}

The problem is I have a lot of items and I have to execute the query for each of the items because the where clause will be complete in each step. I think if I will be able to make my query out side of my loop, my query will be faster. But I don't know how. Is there any way to do this?

I would say the problem is in that Convert you use in the query. What is the datatype of the columns Price and Sale? Why you need to convert? — Steve, Jan 28 '18 at 10:40
" I am completely aware about parameterized queries and SQL Injection" and yet you have this query. Why? You have basically two solutions here: 1. Create a stored procedure and use a table valued parameter, 2. Generate parameters dynamically and use the `IN` clause. — Zohar Peled, Jan 28 '18 at 10:43
@ZoharPeled I have shorten my code and my real code is using parameterized queries. — , Jan 28 '18 at 10:43
Try to remove those convert and let the SQL work without requesting the conversion. Do it in your code when you have the sum. What do you do in that Execute method? — Steve, Jan 28 '18 at 10:47
Look at [this q & a](https://stackoverflow.com/questions/46806620/in-operator-in-oledb/46806712#46806712). — Zohar Peled, Jan 28 '18 at 10:51
@JasonP why not execute query in one go by passing all Id's of item using IN clause. As part of select you can return that tbl.Id as well and later on do join to perform "Do stuff with query result" — rahulaga-msft, Jan 28 '18 at 10:58
You may also be able to `UNION` a bunch of statements together so you're only hitting the db once, — Evan Trimboli, Jan 28 '18 at 10:58
@Steve No luck. The problem is the same even without convert. — , Jan 28 '18 at 11:05

score 6 · Accepted Answer · answered Jan 28 '18 at 11:12

6

Instead of executing the query for every item. You can add group by to your query and execute only once.

string query = "select tbl.ID, sum(convert(decimal(18,3),tbl.Price)) p, sum(convert(decimal(18,2),tbl.Sale)) s from table1 tbl group by tbl.ID ";
var result = Execute(query);

foreach(var item in items)
{
    var row = result.Select(r => r.ID == item.ID).FirstOrDefault();
    //Do stuff with query result
}

answered Jan 28 '18 at 11:12

Serkan Arslan

13,158
4
29
44

2

Unless `items` is a small subset of the records in `table1` - in this case your query will select a lot of unusable data. +1 anyway. – Zohar Peled Jan 28 '18 at 11:19

score 2 · Answer 2 · answered Jan 28 '18 at 11:18

Do not execute the query for each ID separately. Instead, execute a single query for all Ids using group by to get the p and s values for each id and a parameterized in clause (or better yet, a stored procedure with a table valued parameter).

Here is the IN version of the query:

select Id,
       sum(convert(decimal(18,3),tbl.Price)) p, 
       sum(convert(decimal(18,2),tbl.Sale)) s 
       from table1 tbl
       Where Id IN(<1,2,3,4....>)
       group by Id

Replace <1,2,3,4....> with parameters like described in this answer.

Here is the table valued parameter version of the query:

select tbl.Id,
       sum(convert(decimal(18,3),tbl.Price)) p, 
       sum(convert(decimal(18,2),tbl.Sale)) s 
       from table1 tbl
       inner join @items i on tbl.Id = i.Id
       group by tbl.Id

For a detailed explanation about using table valued parameters, read this answer.

Is there any way to make this query faster and build where clause outside of loop?

2 Answers2