Agfdhyk

Is this the correct way to UNION ALL in a stored procedure?

ALTER PROCEDURE [GetHomePageObjectPageWise]

      @PageIndex INT = 1

      ,@PageSize INT = 10

      ,@PageCount INT OUTPUT

      ,@whereStoryID varchar(2000)

      ,@whereAlbumID varchar(2000)

      ,@wherePictureID varchar(2000)

AS

BEGIN

      SET NOCOUNT ON;



      SELECT StoryID

      , AlbumID

      , StoryTitle

      , NULL AS AlbumName

      , (SELECT URL FROM AlbumPictures WHERE (AlbumID = dbo.Stories.AlbumID) AND (AlbumCover = 'True')) AS AlbumCover

      , Votes

      , NULL AS PictureId

      , 'stories' AS tableName

      , NEWID() AS Sort 



INTO #Results1

FROM Stories WHERE StoryID IN (SELECT StringVal FROM funcListToTableInt(@whereStoryID))



      SELECT    NULL AS StoryID

      , AlbumID

      , NULL AS StoryTitle

      , AlbumName

      , (SELECT URL FROM AlbumPictures AS AlbumPictures_3 WHERE (AlbumID = Albums.AlbumID) AND (AlbumCover = 'True')) AS AlbumCover

      , Votes

      , NULL AS PictureId

      , 'albums' AS tableName

      , NEWID() AS Sort

INTO #Results2

FROM Albums WHERE AlbumID IN (SELECT StringVal FROM funcListToTableInt(@whereAlbumID))



        SELECT NULL AS StoryID

        , NULL AS AlbumID

        , NULL AS StoryTitle

        , NULL AS AlbumName

        , URL

        , Votes

        , PictureID

        , 'pictures' AS tableName

        , NEWID() AS Sort

        INTO #Results3

FROM AlbumPictures AS AlbumPictures_1

WHERE PictureID IN (SELECT StringVal FROM funcListToTableInt(@wherePictureID))



SELECT * INTO #Results4 FROM #Results1

UNION ALL

SELECT * FROM #Results2

UNION ALL

SELECT * FROM #Results3



SELECT ROW_NUMBER() OVER

            (

                  ORDER BY [Sort] DESC

            )AS RowNumber

            , * INTO #Results

            FROM #Results4





      DECLARE @RecordCount INT

      SELECT @RecordCount = COUNT(*) FROM #Results



      SET @PageCount = CEILING(CAST(@RecordCount AS DECIMAL(10, 2)) / CAST(@PageSize AS DECIMAL(10, 2)))



      SELECT * FROM #Results

      WHERE RowNumber BETWEEN(@PageIndex -1) * @PageSize + 1 AND(((@PageIndex -1) * @PageSize + 1) + @PageSize) - 1



      DROP TABLE #Results

      DROP TABLE #Results1

      DROP TABLE #Results2

      DROP TABLE #Results3

      DROP TABLE #Results4

END

These days I like to use non-materialized CTEs rather than temp tables - although in certain circumstances (say the data needs an index) I'll use temp tables.

Mainly lots of cosmetic stuff I'd change really all in the way of hopefully making it more readable in the future (this is not tested as I do not have a copy of your data)

ALTER PROCEDURE [GetHomePageObjectPageWise]

      @PageIndex        INT = 1

      ,@PageSize        INT = 10

      ,@PageCount       INT OUTPUT

      ,@whereStoryID    VARCHAR(2000)

      ,@whereAlbumID    VARCHAR(2000)

      ,@wherePictureID VARCHAR(2000)

AS

BEGIN

     SET NOCOUNT ON;



    WITH Results1 AS

        (

        SELECT 

            StoryID,

            AlbumID,

            StoryTitle,

            [AlbumName] = NULL,

            [AlbumCover] = 

                (

                SELECT URL 

                FROM AlbumPictures 

                WHERE (AlbumID = dbo.Stories.AlbumID) AND (AlbumCover = 'True')

                ),

            Votes,

            [PictureId] = NULL,

            [tableName] = 'stories',

            [Sort] = NEWID()

        FROM Stories 

        WHERE 

                StoryID IN 

                (

                SELECT StringVal 

                FROM funcListToTableInt(@whereStoryID)

                )

        )

    , Results2 AS

        (

        SELECT    

            [StoryID] = NULL ,

            AlbumID,

            [StoryTitle] NULL,

            AlbumName,

            [AlbumCover] = 

                (

                SELECT URL 

                FROM AlbumPictures AS AlbumPictures_3 --<<<DO YOU NEED THIS ALIAS?

                WHERE (AlbumID = Albums.AlbumID) AND (AlbumCover = 'True')

                ),

            Votes,

            [PictureId] = NULL,

            [tableName] = 'albums',

            [Sort] = NEWID()

        FROM Albums 

        WHERE 

            AlbumID IN 

                (

                SELECT StringVal 

                FROM funcListToTableInt(@whereAlbumID)

                )

        )       

    , Result3 AS  

        (

        SELECT 

            [StoryID] = NULL, 

            [AlbumID] = NULL,

            [StoryTitle] = NULL,

            [AlbumName] = NULL,

            URL,

            Votes,

            PictureID,

            [tableName] = 'pictures',

            [Sort] = NEWID()

        FROM AlbumPictures --AS AlbumPictures_1 <<<DO YOU NEED THIS ALIAS?

        WHERE 

            PictureID IN 

                (

                SELECT StringVal 

                FROM funcListToTableInt(@wherePictureID)

                )

        )

    , Result4 AS  

        (

        SELECT * FROM Results1 UNION ALL

        SELECT * FROM Results2 UNION ALL

        SELECT * FROM Results3

        )

    , Results AS

        (

        SELECT 

                [RowNumber] = ROW_NUMBER() OVER (ORDER BY [Sort] DESC),

            x.* 

        FROM Results4   x

        )

 SELECT * 

 FROM Results

 WHERE RowNumber BETWEEN(@PageIndex -1) * @PageSize + 1 AND(((@PageIndex -1) * @PageSize + 1) + @PageSize) - 1;

 DECLARE @RecordCount INT = @@RowCount; 



 SET @PageCount = CEILING(CAST(@RecordCount AS DECIMAL(10, 2)) / CAST(@PageSize AS DECIMAL(10, 2)));



END

I'm generally use Aaron Bertrand's suggestions for writing Stored Procedures this blog post is my checklist and the template I use to try to unify the style I use with all my Sprocs:

https://sqlblog.org/2008/10/30/my-stored-procedure-best-practices-checklist

I think as Gordon suggested you could move lots of the logic out of the stored procedure and create a VIEW like this:

CREATE VIEW [console].[vw_mySimpleView]

AS



BEGIN

    SET NOCOUNT ON;



    WITH Results1 AS

        (

        SELECT 

            StoryID,

            AlbumID,

            StoryTitle,

            [AlbumName] = NULL,

            [AlbumCover] = 

                (

                SELECT URL 

                FROM AlbumPictures 

                WHERE (AlbumID = dbo.Stories.AlbumID) AND (AlbumCover = 'True')

                ),

            Votes,

            [PictureId] = NULL,

            [tableName] = 'stories',

            [Sort] = NEWID()

        FROM Stories 

        )

    , Results2 AS

        (

        SELECT    

            [StoryID] = NULL ,

            AlbumID,

            [StoryTitle] NULL,

            AlbumName,

            [AlbumCover] = 

                (

                SELECT URL 

                FROM AlbumPictures 

                WHERE (AlbumID = Albums.AlbumID) AND (AlbumCover = 'True')

                ),

            Votes,

            [PictureId] = NULL,

            [tableName] = 'albums',

            [Sort] = NEWID()

        FROM Albums 

        )       

    , Result3 AS  

        (

        SELECT 

            [StoryID] = NULL, 

            [AlbumID] = NULL,

            [StoryTitle] = NULL,

            [AlbumName] = NULL,

            URL,

            Votes,

            PictureID,

            [tableName] = 'pictures',

            [Sort] = NEWID()

        FROM AlbumPictures

        )

    , Result4 AS  

        (

        SELECT * FROM Results1 UNION ALL

        SELECT * FROM Results2 UNION ALL

        SELECT * FROM Results3

        )

SELECT *

FROM Results4;



GO 

Then the Sproc would be a lot shorter:

ALTER PROCEDURE [GetHomePageObjectPageWise]

      @PageIndex        INT = 1

      ,@PageSize        INT = 10

      ,@PageCount       INT OUTPUT

      ,@whereStoryID    VARCHAR(2000)

      ,@whereAlbumID    VARCHAR(2000)

      ,@wherePictureID VARCHAR(2000)

AS

BEGIN

     SET NOCOUNT ON;



    SELECT * 

    FROM 

        (

        SELECT 

            [RowNumber] = ROW_NUMBER() OVER (ORDER BY [Sort] DESC),

            x.* 

        FROM 

            (

            SELECT *

            FROM [dbo].[vw_mySimpleView] 

            WHERE 

                StoryID IN 

                    (

                    SELECT StringVal 

                    FROM funcListToTableInt(@whereStoryID)

                    )

                            OR  

                            AlbumID IN 

                                   (

                                   SELECT StringVal 

                                   FROM funcListToTableInt(@whereAlbumID)

                                   )

            )   x

        )

    WHERE RowNumber BETWEEN(@PageIndex -1) * @PageSize + 1 AND(((@PageIndex -1) * @PageSize + 1) + @PageSize) - 1;

    DECLARE @RecordCount INT = @@RowCount; 



    SET @PageCount = CEILING(CAST(@RecordCount AS DECIMAL(10, 2)) / CAST(@PageSize AS DECIMAL(10, 2)));





END

Here are some comments:

(1) I prefer table valued variables (declare @Results as table . . .) rather than temporary tables.

(2) In general, it is probably better to write a single query rather than separate queries. So, you can eliminate the intermediate results tables anyway. SQL engines are designed to optimize execution paths. Give them a chance to work. That said, sometimes they get things wrong and intermediate tables are desirable/necessary.

(3) Your sort is ok, but you do need to be care. If Sort has duplicate values you run the risk of getting repeated values and different iterations will cause problems.

(4) Since you are really just returning results from a query, why not just define the query (perhaps as a view) and eliminate the stored procedure entirely? The stored procedure makes it unlikely that SQL Server will cache the results for paging purposes.

(5) I also wonder if you can remove the function calls in the from clause, since those can also negatively affect performance.