Stored procedure optimization

Question

Hi, i have a stored procedure which takes lot of time to execure .Can any one suggest a better approch so that the same result set is achived.

ALTER PROCEDURE [dbo].[spFavoriteRecipesGET] 
@USERID INT, @PAGENUMBER INT, @PAGESIZE INT, @SORTDIRECTION VARCHAR(4), @SORTORDER VARCHAR(4),@FILTERBY INT
AS 
BEGIN

    DECLARE 
        @ROW_START INT
    DECLARE 
        @ROW_END INT
    SET 
        @ROW_START = (@PageNumber-1)* @PageSize+1
    SET 
        @ROW_END =  @PageNumber*@PageSize 

    DECLARE 
        @RecipeCount INT

    DECLARE
        @RESULT_SET_TABLE 
    TABLE
    (
        Id INT NOT NULL IDENTITY(1,1),
        FavoriteRecipeId INT,
        RecipeId INT,
        DateAdded DATETIME,
        Title NVARCHAR(255),
        UrlFriendlyTitle NVARCHAR(250),
        [Description] NVARCHAR(MAX),
        AverageRatingId FLOAT,
        SubmittedById INT,
        SubmittedBy VARCHAR(250),
        RecipeStateId INT,
        RecipeRatingId INT,
        ReviewCount INT,
        TweaksCount INT,
        PhotoCount INT,
        ImageName NVARCHAR(50)              
    )
    INSERT INTO  @RESULT_SET_TABLE
        SELECT 
            FavoriteRecipes.FavoriteRecipeId,
            Recipes.RecipeId,
            FavoriteRecipes.DateAdded,
            Recipes.Title,
            Recipes.UrlFriendlyTitle,
            Recipes.[Description],
            Recipes.AverageRatingId,
            Recipes.SubmittedById,
            COALESCE(users.DisplayName,users.UserName,Recipes.SubmittedBy) As SubmittedBy, 
            Recipes.RecipeStateId,
            RecipeReviews.RecipeRatingId,
            COUNT(RecipeReviews.Review),
            COUNT(RecipeTweaks.Tweak),
            COUNT(Photos.PhotoId),      
            dbo.udfGetRecipePhoto(Recipes.RecipeId) AS ImageName
        FROM
            FavoriteRecipes
        INNER JOIN Recipes ON FavoriteRecipes.RecipeId=Recipes.RecipeId AND Recipes.RecipeStateId <> 3 
        LEFT OUTER JOIN RecipeReviews ON RecipeReviews.RecipeId=Recipes.RecipeId  AND RecipeReviews.ReviewedById=@UserId 
        AND RecipeReviews.RecipeRatingId=   (
                                                SELECT MAX(RecipeReviews.RecipeRatingId)
                                                FROM RecipeReviews
                                                WHERE RecipeReviews.ReviewedById=@UserId
                                                AND RecipeReviews.RecipeId=FavoriteRecipes.RecipeId
                                            )  

        OR RecipeReviews.RecipeRatingId IS NULL 
        LEFT OUTER JOIN  RecipeTweaks ON RecipeTweaks.RecipeId = Recipes.RecipeId AND RecipeTweaks.TweakedById= @UserId 
        LEFT OUTER JOIN Photos ON   Photos.RecipeId = Recipes.RecipeId 
                                    AND Photos.UploadedById = @UserId   AND Photos.RecipeId =  FavoriteRecipes.RecipeId 
                                    AND Photos.PhotoTypeId = 1  

        LEFT OUTER JOIN users ON Recipes.SubmittedById = users.UserId       
        WHERE 
            FavoriteRecipes.UserId=@UserId
        GROUP BY 
            FavoriteRecipes.FavoriteRecipeId,
            Recipes.RecipeId,
            FavoriteRecipes.DateAdded,
            Recipes.Title,
            Recipes.UrlFriendlyTitle,
            Recipes.[Description],
            Recipes.AverageRatingId,
            Recipes.SubmittedById,
            Recipes.SubmittedBy,
            Recipes.RecipeStateId,
            RecipeReviews.RecipeRatingId,
            users.DisplayName,
            users.UserName,
            Recipes.SubmittedBy;

        WITH SortResults
        AS
        (
                SELECT 
                ROW_NUMBER() OVER (
                    ORDER BY  CASE WHEN @SORTDIRECTION = 't' AND @SORTORDER='a' THEN TITLE END ASC,
                          CASE WHEN @SORTDIRECTION = 't' AND @SORTORDER='d' THEN TITLE END DESC,
                          CASE WHEN @SORTDIRECTION = 'r' AND @SORTORDER='a' THEN AverageRatingId END ASC,
                          CASE WHEN @SORTDIRECTION = 'r' AND @SORTORDER='d' THEN AverageRatingId END DESC,
                          CASE WHEN @SORTDIRECTION = 'mr' AND @SORTORDER='a' THEN RecipeRatingId END ASC,
                          CASE WHEN @SORTDIRECTION = 'mr' AND @SORTORDER='d' THEN RecipeRatingId END DESC,
                          CASE WHEN @SORTDIRECTION = 'd' AND @SORTORDER='a' THEN DateAdded END ASC,
                          CASE WHEN @SORTDIRECTION = 'd' AND @SORTORDER='d' THEN DateAdded END DESC
                ) RowNumber,
                FavoriteRecipeId,
                RecipeId,
                DateAdded,
                Title,
                UrlFriendlyTitle,
                [Description], 
                AverageRatingId,
                SubmittedById,
                SubmittedBy, 
                RecipeStateId,
                RecipeRatingId,
                ReviewCount,
                TweaksCount,
                PhotoCount, 
                ImageName           

            FROM 
                @RESULT_SET_TABLE
            WHERE   
                    ((@FILTERBY = 1 AND SubmittedById= @USERID)
                OR  ( @FILTERBY = 2 AND (SubmittedById <> @USERID OR SubmittedById IS NULL))
                OR  ( @FILTERBY <> 1 AND @FILTERBY <> 2))
        )         
        SELECT 
            RowNumber,
            FavoriteRecipeId,
            RecipeId,
            DateAdded,
            Title,
            UrlFriendlyTitle,
            [Description], 
            AverageRatingId,
            SubmittedById,
            SubmittedBy, 
            RecipeStateId,
            RecipeRatingId,
            ReviewCount,
            TweaksCount,
            PhotoCount, 
            ImageName   
        FROM 
            SortResults

        WHERE  
        RowNumber BETWEEN @ROW_START AND @ROW_END 
        print @ROW_START    
        print @ROW_END
       SELECT 
            @RecipeCount=dbo.udfGetFavRecipesCount(@UserId)
        SELECT 
            @RecipeCount AS RecipeCount
        SELECT COUNT(Id) as FilterCount FROM @RESULT_SET_TABLE               
        WHERE       
                    ((@FILTERBY = 1 AND SubmittedById= @USERID)
                OR  (@FILTERBY = 2 AND (SubmittedById <> @USERID OR SubmittedById IS NULL))
                OR  (@FILTERBY <> 1 AND @FILTERBY <> 2))


END

Answer 1

You need to look at the execution plan to see where the time is going. It could be indexes, table-scans caused by your UDF, any number of things. As you anayze the plan, try to break up the query into smaller pieces to see if you can make a difference in them.

Then learn about ROW_NUMBER to see if you can do without the local table.

Answer 2

Couple notes

Indexing - often times when people create procedures which use temp table or table variable they fail to realize you can create indexes on those objects and this can have massive performance implications.

UDF - Sometimes the query processor will effectively inline UDF logic and sometimes not, look closely at your query plan an see how this is being handled. Often times if you manually inline this logic in something like a correlated sub-query you can boost performance a lot.

Answer 3

You need to add parentheses around your OR conditions.

LEFT OUTER JOIN RecipeReviews
  ON RecipeReviews.RecipeId = Recipes.RecipeId 
    AND RecipeReviews.ReviewedById = @UserId  
    AND 
    -- insert open parenthesis here:
    (
      RecipeReviews.RecipeRatingId = (... subquery ...)
      OR RecipeReviews.RecipeRatingId IS NULL  
      -- insert close parenthesis here:
    )

Answer 4

George Zacharia do you even know how to intepret an execution plan? Download this book so that you can understand how they work. http://downloads.red-gate.com/ebooks/HighPerformanceSQL_ebook.zip

Answer 5

As others have said, the only way to know is to look at explain plans. Glancing over the code, this part looks kind of fishy:

    AND RecipeReviews.RecipeRatingId=   (
                                            SELECT MAX(RecipeReviews.RecipeRatingId)
                                            FROM RecipeReviews
                                            WHERE RecipeReviews.ReviewedById=@UserId
                                            AND RecipeReviews.RecipeId=FavoriteRecipes.RecipeId
                                        )

In general, doing non-trivial stuff in join conditions is a Bad Idea. I would factor that out into a sub-select, and since it's an outer join, you'd probably have to combine that with RecipeReviews somehow.

BUT: All of this is speculation! Explain! Measure!

Answer 6

Well in addition to the possible poor performance of the UDF, this line of code concerns me

 LEFT OUTER JOIN RecipeReviews 
 ON RecipeReviews.RecipeId=Recipes.RecipeId  
    AND RecipeReviews.ReviewedById=@UserId  
    AND RecipeReviews.RecipeRatingId=   
        (SELECT MAX(RecipeReviews.RecipeRatingId) 
        FROM RecipeReviews 
        WHERE RecipeReviews.ReviewedById=@UserId 
        AND RecipeReviews.RecipeId=FavoriteRecipes.RecipeId )   
        OR RecipeReviews.RecipeRatingId IS NULL  

It is generally a poor practice to use a subquery as part of a join. I would strongly supect this is not using any indexes you may have. And the OR part doesn;t make sense to mea atll all, the left join shoudl get you this.

Rewrite it to make a derived table instead.

If you have a lot of records a temp table usually performs better than a table variable and can (and probably should) be indexed.

Answer 7

the very first, simple thing i would do, is move all your declare statements to the top.

    DECLARE @ROW_START INT,
        @ROW_END INT,
        @RecipeCount INT

    DECLARE 
    @RESULT_SET_TABLE  
TABLE 
( 
    Id INT NOT NULL IDENTITY(1,1), 
       )

The next part, which is still rather simple, is stuff like this:

AND Recipes.RecipeStateId <> 3
AND RecipeTweaks.TweakedById= @UserId

This can be taken out of the join and move to the where clause. if you can, change the <> to an in statement so that it can utlize an index seek.

AND RecipeReviews.RecipeRatingId=   
( 
    SELECT MAX(RecipeReviews.RecipeRatingId) 
    FROM RecipeReviews 
    WHERE RecipeReviews.ReviewedById=@UserId 
        AND RecipeReviews.RecipeId=FavoriteRecipes.RecipeId 
)

that's jsut crazy looking and needs to be completely redone.