Click here to monitor SSC
SQLServerCentral is supported by Red Gate Software Ltd.
 
Log in  ::  Register  ::  Not logged in
 
 
 
        
Home       Members    Calendar    Who's On


Add to briefcase ««12

SQL Function takes more than 2 hours to return a table.. Expand / Collapse
Author
Message
Posted Tuesday, February 5, 2013 5:10 PM


SSC-Dedicated

SSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-Dedicated

Group: General Forum Members
Last Login: Yesterday @ 8:06 PM
Points: 36,786, Visits: 31,243
sanjaytiwari31 (2/4/2013)
Can anybody tell me how to optimize this function or where is the problem.


Yes I can but you're not going to like it. The code cannot be optimized no matter how much you play with it because of the nested cursors. It would be a far better thing sack the code entirely and rewrite it from the ground up without the cursors.

Because of the length of the code, I've not done a serious deep dive on it but I agree with what has already been said... it looks like it could be resolved by a single relatively simple query across 4 tables. If no one in your shop can handle the conversion, your shop should get some hired help.


--Jeff Moden
"RBAR is pronounced "ree-bar" and is a "Modenism" for "Row-By-Agonizing-Row".

First step towards the paradigm shift of writing Set Based code:
Stop thinking about what you want to do to a row... think, instead, of what you want to do to a column."

(play on words) "Just because you CAN do something in T-SQL, doesn't mean you SHOULDN'T." --22 Aug 2013

Helpful Links:
How to post code problems
How to post performance problems
Post #1416170
Posted Wednesday, February 6, 2013 3:23 AM
Old Hand

Old HandOld HandOld HandOld HandOld HandOld HandOld HandOld Hand

Group: General Forum Members
Last Login: Tuesday, February 25, 2014 3:42 AM
Points: 363, Visits: 87
Try to create a primary key/unique key on your table variable, and it is always better to go as a stored proc using temp tables and while loop instead of cursors and try building a clustered index on the temp table which will definitely improve your performance.
Post #1416368
Posted Wednesday, February 6, 2013 4:05 AM


SSCoach

SSCoachSSCoachSSCoachSSCoachSSCoachSSCoachSSCoachSSCoachSSCoachSSCoachSSCoach

Group: General Forum Members
Last Login: Yesterday @ 12:27 PM
Points: 15,541, Visits: 27,919
k.srikalyan (2/6/2013)
Try to create a primary key/unique key on your table variable, and it is always better to go as a stored proc using temp tables and while loop instead of cursors and try building a clustered index on the temp table which will definitely improve your performance.


A while loop is a cursor. That's not a replacement for one.


----------------------------------------------------
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood..." Theodore Roosevelt
The Scary DBA
Author of: SQL Server 2012 Query Performance Tuning
SQL Server 2008 Query Performance Tuning Distilled
and
SQL Server Execution Plans

Product Evangelist for Red Gate Software
Post #1416390
Posted Wednesday, February 6, 2013 5:31 AM
Forum Newbie

Forum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum Newbie

Group: General Forum Members
Last Login: Friday, April 5, 2013 3:00 AM
Points: 9, Visits: 24
Not really looking in detail but
I think it should be a store procedure.
And I quite sure we can eliminated the cursor using temp table and update and set variables in a single query.



Post #1416435
Posted Wednesday, February 6, 2013 8:24 AM


SSC-Insane

SSC-InsaneSSC-InsaneSSC-InsaneSSC-InsaneSSC-InsaneSSC-InsaneSSC-InsaneSSC-InsaneSSC-InsaneSSC-InsaneSSC-Insane

Group: General Forum Members
Last Login: Yesterday @ 9:11 PM
Points: 23,070, Visits: 31,598
k.srikalyan (2/6/2013)
Try to create a primary key/unique key on your table variable, and it is always better to go as a stored proc using temp tables and while loop instead of cursors and try building a clustered index on the temp table which will definitely improve your performance.


While loops are not all that better than cursors, it is still RBAR.



Lynn Pettis

For better assistance in answering your questions, click here
For tips to get better help with Performance Problems, click here
For Running Totals and its variations, click here or when working with partitioned tables
For more about Tally Tables, click here
For more about Cross Tabs and Pivots, click here and here
Managing Transaction Logs

SQL Musings from the Desert Fountain Valley SQL (My Mirror Blog)
Post #1416538
Posted Wednesday, February 6, 2013 1:03 PM


SSC-Dedicated

SSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-Dedicated

Group: General Forum Members
Last Login: Yesterday @ 8:06 PM
Points: 36,786, Visits: 31,243
k.srikalyan (2/6/2013)
Try to create a primary key/unique key on your table variable, and it is always better to go as a stored proc using temp tables and while loop instead of cursors and try building a clustered index on the temp table which will definitely improve your performance.


bobie (2/6/2013)
Not really looking in detail but
I think it should be a store procedure.
And I quite sure we can eliminated the cursor using temp table and update and set variables in a single query.


Absolutely not. Temp table/While loop replacements for cursors is like smoking a pipe to quit cigars. They're essential identical and a well written firehose cursor (read only, forward only) is frequently faster than the Temp Table/While loop combination. To wit, replacing cursors with While Loops does little to help the situation and will sometimes cause even worse performance because most people aren't good at writing either.

Please don't make such recommendations in the future because it's a myth and a general bad recommedation to replace cursors with Temp Tables/While loops.


--Jeff Moden
"RBAR is pronounced "ree-bar" and is a "Modenism" for "Row-By-Agonizing-Row".

First step towards the paradigm shift of writing Set Based code:
Stop thinking about what you want to do to a row... think, instead, of what you want to do to a column."

(play on words) "Just because you CAN do something in T-SQL, doesn't mean you SHOULDN'T." --22 Aug 2013

Helpful Links:
How to post code problems
How to post performance problems
Post #1416703
Posted Thursday, February 7, 2013 7:38 PM
Forum Newbie

Forum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum Newbie

Group: General Forum Members
Last Login: Friday, April 5, 2013 3:00 AM
Points: 9, Visits: 24
What I mean is not a combination temp table and while loop but temp table with an bulk update statement with help of variable, is it the same as while loop?
Because if I check the statistics io and the time needed to execute it is much2 lower io and faster.



Post #1417428
Posted Friday, February 8, 2013 8:00 AM


SSC-Dedicated

SSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-Dedicated

Group: General Forum Members
Last Login: Yesterday @ 8:06 PM
Points: 36,786, Visits: 31,243
ChrisM@Work (2/5/2013)
Steven Willis (2/5/2013)
Just a generic observation...

If this function is taking two hours to run--even with all of the cursors and subcursors--then you are probably accessing a LOT of data. I don't think a function is even appropriate. Is this being called by another procedure or query? If so the issues raised are multiplied by how many times the function itself is being called.

You should write a procedure instead. ..


Not necessarily. A properly written iTVF inlines like a view. This function is just a 4-table query with a few sums in it. I can't see any reason why it shouldn't be written as an iTVF regardless of how many rows the base tables contain.


+100!


--Jeff Moden
"RBAR is pronounced "ree-bar" and is a "Modenism" for "Row-By-Agonizing-Row".

First step towards the paradigm shift of writing Set Based code:
Stop thinking about what you want to do to a row... think, instead, of what you want to do to a column."

(play on words) "Just because you CAN do something in T-SQL, doesn't mean you SHOULDN'T." --22 Aug 2013

Helpful Links:
How to post code problems
How to post performance problems
Post #1417718
Posted Friday, February 8, 2013 8:03 AM


SSC-Dedicated

SSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-DedicatedSSC-Dedicated

Group: General Forum Members
Last Login: Yesterday @ 8:06 PM
Points: 36,786, Visits: 31,243
bobie (2/7/2013)
What I mean is not a combination temp table and while loop but temp table with an bulk update statement with help of variable, is it the same as while loop?
Because if I check the statistics io and the time needed to execute it is much2 lower io and faster.


Ah! Understood. Yes. Doing a "Divid'n'conquer" using a Temp Table to hold interim results during "bulk updates" is much better than using cursors or while loops. Of course, you can't use Temp Tables in functions and Table Variables usually aren't the thing to use when a lot of rows are present.


--Jeff Moden
"RBAR is pronounced "ree-bar" and is a "Modenism" for "Row-By-Agonizing-Row".

First step towards the paradigm shift of writing Set Based code:
Stop thinking about what you want to do to a row... think, instead, of what you want to do to a column."

(play on words) "Just because you CAN do something in T-SQL, doesn't mean you SHOULDN'T." --22 Aug 2013

Helpful Links:
How to post code problems
How to post performance problems
Post #1417721
« Prev Topic | Next Topic »

Add to briefcase ««12

Permissions Expand / Collapse