Take the 2-minute tour ×
Database Administrators Stack Exchange is a question and answer site for database professionals who wish to improve their database skills and learn from others in the community. It's 100% free, no registration required.

Just wondering if I could solicit some feedback on a stored procedure I'm running and whether there's a more efficient way of handling the scenario (I'm pretty sure there will be!).

Basically I have a single SP that I call to return a list of records (Jobs) that may have one or more statuses and a sort order (I'm using RowNum for paging). At the moment I'm using WITH RECOMPILE because the variations on the statuses can change all the time (depending on user etc). There's also some filtering going on.

I'm using an IF statement to essentially run the same bit of code with the only change being the sort order.

I guess my questions are: Is there a better way of doing this (maybe different SP's for different statuses)? Am I overcomplicating things due to lack of knowledge (quite likely) Is the SP actually ok, but requires minor tweaks to reduce the number of lines?

I've pasted a portion of the SP below - the only difference to the full code is the additional IF statements for the different sort orders...

I'd appreciate any feedback.

Thanks in advance!

PROCEDURE [dbo].[sp_Jobs] 

@PageNumber int, 
@PageSize int, 
@FilterExpression varchar(500), 
@OrderBy varchar(50), 
@CustomerID int, 
@ShowNotSet bit, 
@ShowPlaced bit, 
@ShowProofed bit, 
@ShowReProofed bit, 
@ShowApproved bit, 
@ShowOnTime bit, 
@ShowLate bit, 
@ShowProblem bit, 
@ShowCompleted bit, 
@ShowDispatched bit, 
@ShowUnapproved bit, 
@ShowClosed bit, 
@ShowReturned bit, 
@UserID int

WITH RECOMPILE 

AS

--JobNumber DESC 
if @OrderBy='JobNumberDESC' 
BEGIN 

WITH Keys AS (SELECT TOP (@PageNumber * @PageSize) ROW_NUMBER() OVER (ORDER BY JobNumber DESC) as rn,P1.jobNumber,P1.CustID,P1.DateIn,P1.DateDue,P1.DateOut,p1.client,p1.MasterJobStatusID,p1.MasterJobStatusTimestamp,p1.OwnerID 

FROM 
vw_Jobs_List P1 WITH (NOLOCK) 

WHERE 
(@CustomerID = 0 OR CustID = @CustomerID) 
AND (@UserID = 0 OR OwnerID = @UserID) 
AND ((@ShowNotSet = 1 AND MasterJobStatusID=1) OR (@ShowPlaced = 1 AND MasterJobStatusID=2) OR (@ShowProofed = 1 AND MasterJobStatusID=3) OR (@ShowReProofed = 1 AND MasterJobStatusID=4) OR (@ShowApproved = 1 AND MasterJobStatusID=5) OR (@ShowOnTime = 1 AND MasterJobStatusID=6) OR (@ShowLate = 1 AND MasterJobStatusID=7) OR (@ShowProblem = 1 AND MasterJobStatusID=8) OR (@ShowCompleted = 1 AND MasterJobStatusID=9) OR (@ShowDispatched = 1 AND MasterJobStatusID=10) OR (@ShowUnapproved = 1 AND MasterJobStatusID=11) OR (@ShowClosed = 1 AND MasterJobStatusID=12) OR (@ShowReturned = 1 AND MasterJobStatusID=13)) AND (Search LIKE '%'+@FilterExpression+'%')

ORDER BY 
P1.JobNumber DESC ),SelectedKeys AS (
SELECT TOP (@PageSize)SK.rn,SK.JobNumber,SK.CustID,SK.DateIn,SK.DateDue,SK.DateOut 

FROM 
Keys SK 

WHERE 
SK.rn > ((@PageNumber-1) * @PageSize) 

ORDER BY 
SK.JobNumber DESC) 

SELECT SK.rn,J.JobNumber,J.OwnerID,J.Description,J.Client,SK.CustID,OrderNumber, CAST(DateAdd(d, -2, CAST(isnull(SK.DateIn,0) AS DateTime)) AS nvarchar) AS DateIn, CAST(DateAdd(d, -2, CAST(isnull(SK.DateDue,0) AS DateTime)) AS nvarchar) AS DateDue,CAST(DateAdd(d, -2, CAST(isnull(SK.DateOut,0) AS DateTime)) AS nvarchar) AS DateOut, Del_Method,Ticket#, InvoiceEmailed, InvoicePrinted, InvoiceExported, InvoiceComplete, JobStatus,j.MasterJobStatusID,j.MasterJobStatusTimestamp,js.MasterJobStatus 

FROM SelectedKeys SK JOIN vw_Jobs_List J WITH (NOLOCK) ON j.JobNumber=SK.JobNumber JOIN tbl_SYSTEM_MasterJobStatus js WITH (NOLOCK) ON j.MasterJobStatusID=js.MasterJobStatusID 

ORDER BY 
SK.JobNumber DESC 
END

--ELSE IF for other column sorting

share|improve this question

1 Answer 1

up vote 11 down vote accepted

The sorting can be taken care of with a CASE expression, something along the lines of:

ORDER BY
    CASE WHEN @SortDirection = 'A' THEN
        CASE 
           WHEN @SortBy = 'JobNumber' THEN JobNumber
           WHEN @SortBy = 'JobId' THEN JobId 
        END
    END ASC
    , CASE WHEN @SortDirection = 'D' THEN
        CASE 
           WHEN @SortBy = 'JobNumber' THEN JobNumber
           WHEN @SortBy = 'JobId' THEN JobId 
        END
    END DESC

You may want to reconsider the OR'd where conditions as they are likely to generate poor plans. One of the best articles I've read covering this (and the alternative approaches) is Dynamic Search Conditions in T-SQL

Edit: Looking again at your parameter list, the primary filters appear to be @CustomerId and @UserId. I'd suggest creating two procs, spJobs_SelectByCustomerId and spJobs_SelectByUserId, which filter by their respective parameters so you eliminate the '@Param = 0 or Column = @Param' conditions. I guess the next important param is @ShowCompleted (presuming that once a job is 'done', it isn't shown unless @ShowCompleted=1), which i'd consider including in indexes on the CustomerId and UserId.

Edit2: Funny how these questions sometimes tick over in the back of your mind! :) On indexing @ShowCompleted, this is one of the occasions where Using a Low-Selectivity BIT Column First Can Be the Best Strategy. Filtered indexes should also be considered.

share|improve this answer
    
Wooosh! All straight over my head, but I'm not afraid of reading and learning! Thanks Mark for taking the time to respond. It is indeed funny how the subconcious continues to work through these things. I find beer and nicotine help too :) –  VaticNZ Jul 27 '11 at 4:03
    
If anything needs clarification, feel free to extend your question or start a new post. –  Mark Storey-Smith Jul 27 '11 at 11:10
    
Thanks Mark. I've implemented some of your suggestions and all is good except a weird problem... I've posted on another thread: dba.stackexchange.com/questions/4162/… –  VaticNZ Jul 27 '11 at 23:02
    
My bad, didn't explain that you have to deal with different types in seperate case expressions. Have added an answer to your new question. –  Mark Storey-Smith Jul 27 '11 at 23:50
    
won't this solution (CASE-based) generate a poor execution plan too? Won't this CASE be evaluated for each row? –  Andrei Rinea Apr 4 '12 at 14:11

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.