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.
