5

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
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
user1593175
  • 327
  • 5
  • 14
  • 1
    Can you elaborate on what your question is? – Gordon Linoff Dec 30 '12 at 20:44
  • @GordonLinoff As shown in the given example, 1st i am doing 3 `selects` and putting the result into 3 `views` then i am doing `union all` on the 3 `views`. So i want to know, is this the correct way or can i optimize it in a better way? – user1593175 Dec 30 '12 at 20:50
  • 1
    You're not putting the results in three views, you're putting them in three temporary tables. You're then copying that to a fourth temporary table, and copying *that* to a fifth temporary table. That does seem like there must be a simpler way. And you're copying all the records every time even though you'll only want 10 (adjustable) rows in the result set. As it is, though, it's hard to see what exactly you're looking for (`NEWID()` for a column `Sort`?), in order to be able to improve on your procedure. Can you show your tables, sample data, sample input parameters, and desired output? –  Dec 30 '12 at 21:00
  • @hvd kindly check my other question for the table structure,sample input and output.[http://stackoverflow.com/questions/14071312/combining-rows-from-multiple-tables-with-different-number-of-columns/](http://stackoverflow.com/questions/14071312/combining-rows-from-multiple-tables-with-different-number-of-columns/) – user1593175 Dec 30 '12 at 21:13
  • @user1593175 Ah, so you want a random 10 rows from any of the three tables? What are you doing with PageIndex? When you ask for page 1, and then ask for page 1 again, you won't get the same results, because you'll have re-shuffled them. That seems like it's what you want, but having PageIndex there at all seems unnecessary. I also have my doubts how you're selecting random rows, [this question](http://stackoverflow.com/questions/848872/select-n-random-rows-from-sql-server-table) has more details and alternatives for that part. –  Dec 30 '12 at 21:22
  • @hvd i send the pageindex value from my code..so for every new pageindex i fetch the next 10 records(row numbers)..in my actual table i have added a sort column permanently, so the guid values do not change and i get the same sort everytime. – user1593175 Dec 30 '12 at 21:29
  • are you using 2005 or 2008R2 ? – whytheq Dec 30 '12 at 21:30
  • @hvd one more thing..i do not want 10 random rows. first i combine all the results from all the 3 tables ...then sort them..then i retrieve the 10 rows for each page index. – user1593175 Dec 30 '12 at 21:31
  • @user1593175 If the procedure you're asking about isn't the procedure you're using, and comments on the procedure in your question don't apply, then you probably haven't asked what you wanted to. :) –  Dec 30 '12 at 21:31
  • @user1593175 I did get that part, not 10 rows from one table, 10 rows from the second, 10 rows from the third, but 10 rows total, each row coming from one of the three tables, correct? –  Dec 30 '12 at 21:33
  • @hvd sorry about the confusion..the question and procedure , both are correct..i have just made a small change..added the sort column in the table rather than a virtual column in the query – user1593175 Dec 30 '12 at 21:34

2 Answers2

3

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.

Gordon Linoff
  • 1,242,037
  • 58
  • 646
  • 786
3

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
Aaron Bertrand
  • 272,866
  • 37
  • 466
  • 490
whytheq
  • 34,466
  • 65
  • 172
  • 267
  • you have used Result3 twice and you also use Result4..but i guess you have not declared Result4 anywhere..is that correct? – user1593175 Dec 30 '12 at 22:15
  • typo - hopefully better now – whytheq Dec 30 '12 at 22:19
  • thanks for your time and effort. essentially there is no difference between my and your solution other than readability? – user1593175 Dec 30 '12 at 22:21
  • readability via `CTE`s and layout. I think I did this delaration in one `DECLARE @RecordCount INT = (SELECT COUNT(*) cnt FROM Results);` and I got rid of some of the table aliases as they didn't seem necessary. I added semi-colons as they will be required in the future. – whytheq Dec 30 '12 at 22:24
  • Great.Thanks.You are a nice man. +1 and accepting yours as answer. Is there any way i can skip the procedure and do the same thing in dynamic sql? – user1593175 Dec 30 '12 at 22:31
  • I spotted a couple of problems near the end of my answer and need to edit. – whytheq Dec 30 '12 at 22:33
  • ok.thanks.Is there any way i can skip the procedure and do the same thing in dynamic sql? – user1593175 Dec 30 '12 at 22:35
  • @user1593175 - what about trying Gordon Linoff's suggestion number (4) ? Why not make the three sections into one view and then you can just query the view? – whytheq Dec 30 '12 at 22:37
  • why do you need to use `Dynamic SQL` ....something you should only use if you really need to – whytheq Dec 30 '12 at 22:38
  • i have also asked him..i am not sure how to do that. – user1593175 Dec 30 '12 at 22:39
  • because i suppose dynamic/inline sql is faster and more performance efficient as compared to stored procedures.correcct me if i am wrong. – user1593175 Dec 30 '12 at 22:41
  • and how do i use this view with my stored proc? – user1593175 Dec 30 '12 at 22:51
  • i mean how do i integrate the view with the last part `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` – user1593175 Dec 30 '12 at 22:52
  • I just edited and added the new shorter Stored Procedure. Hopefully gordon will come back as I'm sure he'll be able to help a lot. – whytheq Dec 30 '12 at 22:55
  • i guess we cannot use `WHERE StoryID IN ( SELECT StringVal FROM funcListToTableInt(@whereStoryID) )` in the latest stored proc as i have a where clause for all the three tables...rather i guess i should use the where clause in th view – user1593175 Dec 30 '12 at 23:00
  • you won't be able to put that in the view because it requires a parameter '@whereStoryID' ....anything with parameters will need to stay in the stored procedure – whytheq Dec 30 '12 at 23:07
  • so how would i add the other two parameters `,@whereAlbumID VARCHAR(2000) ,@wherePictureID VARCHAR(2000)` i mean where clauses for the other two tables? – user1593175 Dec 30 '12 at 23:10
  • ah - I'd not spotted those differences in the `WHERE` clauses I'll edit my answer later today: the StoredProc will need to become a little more complex again! – whytheq Dec 31 '12 at 09:09
  • I've added a small section to the Stored Procedure - after `OR` - if you need to add other `OR`s you can just copy/paste and change the parameter – whytheq Jan 02 '13 at 10:17