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
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.These days I like to use non-materialized
CTE
s 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)
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:Then the Sproc would be a lot shorter: