Another Way To Write a Stored Procedure

In the stored procedure below, there are 9 different if conditions being
checked. I was wondering if there is a more efficient way (or not) to write
this procedure, without having to write the same select criteria each time.
The only thing different is the where clause. Any suggestions are greatly
appreciated.
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_NULLS ON
GO
ALTER PROC ListParticipantsByRole(
@.testRoleEnum int,
@.errorID int = 0 OUTPUT)
AS
SET NOCOUNT ON
-- List Originators
IF (@.testRoleEnum = 1)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleOriginator = 1
ORDER BY P.DisplayName
-- List Validators
IF (@.testRoleEnum = 2)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleValidator = 1
ORDER BY P.DisplayName
-- List Screeners
IF (@.testRoleEnum = 3)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleScreener = 1
ORDER BY P.DisplayName
-- List SMEs
IF (@.testRoleEnum = 4)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleSME = 1
ORDER BY P.DisplayName
-- List Validation OPRs
IF (@.testRoleEnum = 5)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleValidationOPR = 1
ORDER BY P.DisplayName
-- List Validation Authorities
IF (@.testRoleEnum = 6)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleValidationAuthority = 1
ORDER BY P.DisplayName
-- List Officials
IF (@.testRoleEnum = 7)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleOfficial = 1
ORDER BY P.DisplayName
-- List Test Directors
IF (@.testRoleEnum = 8)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleTestDirector = 1
ORDER BY P.DisplayName
-- List Survey Participants
IF (@.testRoleEnum = 9)
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE T.TestRoleSurveyParticipant = 1
ORDER BY P.DisplayName
SET NOCOUNT OFF
GO
SET QUOTED_IDENTIFIER OFF
GO
SET ANSI_NULLS ON
GOSELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE 1 = CASE @.testRoleEnum
when 1 then T.TestRoleOriginator
when 2 then T.TestRoleValidator
...
end
ORDER BY P.DisplayName
"Mike Collins" <MikeCollins@.discussions.microsoft.com> wrote in message
news:C4C1B5B8-7E49-467A-83C7-4DEEED88B57B@.microsoft.com...
> In the stored procedure below, there are 9 different if conditions being
> checked. I was wondering if there is a more efficient way (or not) to
> write
> this procedure, without having to write the same select criteria each
> time.
> The only thing different is the where clause. Any suggestions are greatly
> appreciated.
> SET QUOTED_IDENTIFIER ON
> GO
> SET ANSI_NULLS ON
> GO
> ALTER PROC ListParticipantsByRole(
> @.testRoleEnum int,
> @.errorID int = 0 OUTPUT)
> AS
> SET NOCOUNT ON
> -- List Originators
> IF (@.testRoleEnum = 1)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleOriginator = 1
> ORDER BY P.DisplayName
> -- List Validators
> IF (@.testRoleEnum = 2)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidator = 1
> ORDER BY P.DisplayName
> -- List Screeners
> IF (@.testRoleEnum = 3)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleScreener = 1
> ORDER BY P.DisplayName
> -- List SMEs
> IF (@.testRoleEnum = 4)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleSME = 1
> ORDER BY P.DisplayName
> -- List Validation OPRs
> IF (@.testRoleEnum = 5)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidationOPR = 1
> ORDER BY P.DisplayName
> -- List Validation Authorities
> IF (@.testRoleEnum = 6)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidationAuthority = 1
> ORDER BY P.DisplayName
> -- List Officials
> IF (@.testRoleEnum = 7)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleOfficial = 1
> ORDER BY P.DisplayName
> -- List Test Directors
> IF (@.testRoleEnum = 8)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleTestDirector = 1
> ORDER BY P.DisplayName
> -- List Survey Participants
> IF (@.testRoleEnum = 9)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleSurveyParticipant = 1
> ORDER BY P.DisplayName
> SET NOCOUNT OFF
> GO
> SET QUOTED_IDENTIFIER OFF
> GO
> SET ANSI_NULLS ON
> GO
>|||Mike,
A redesign of TestParticipants is the best solution. Instead of
having a bunch of separate columns TestRoleThis, TestRoleThat,
TestRoleOther, ... you should have, depending on whether
a participant can have more than one role, either a column TestRole,
with possible values 'Validator', 'Screener', etc., or a separate table
TestParticipantRoles that links the participants with their roles.
Then your stored procedure becomes a single simple query like
this (case where there is a linking table):
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN TestParticipantRoles R ON T.PersonnelID = R.PersonnelId
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE R.RoleID = @.testRoleEnum
ORDER BY P.DisplayName
Short of that, you can still now do
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE CASE @.testRoleEnum
WHEN 1 THEN T.TestRoleOriginator
WHEN 2 THEN T.TestRoleValidator
..
END = 1
ORDER BY P.DisplayName
This latter solution may turn out to have poorer performance, however.
Steve Kass
Drew University
Mike Collins wrote:

>In the stored procedure below, there are 9 different if conditions being
>checked. I was wondering if there is a more efficient way (or not) to write
>this procedure, without having to write the same select criteria each time
.
>The only thing different is the where clause. Any suggestions are greatly
>appreciated.
>SET QUOTED_IDENTIFIER ON
>GO
>SET ANSI_NULLS ON
>GO
>ALTER PROC ListParticipantsByRole(
> @.testRoleEnum int,
> @.errorID int = 0 OUTPUT)
>AS
> SET NOCOUNT ON
> -- List Originators
> IF (@.testRoleEnum = 1)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleOriginator = 1
> ORDER BY P.DisplayName
> -- List Validators
> IF (@.testRoleEnum = 2)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidator = 1
> ORDER BY P.DisplayName
> -- List Screeners
> IF (@.testRoleEnum = 3)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleScreener = 1
> ORDER BY P.DisplayName
> -- List SMEs
> IF (@.testRoleEnum = 4)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleSME = 1
> ORDER BY P.DisplayName
> -- List Validation OPRs
> IF (@.testRoleEnum = 5)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidationOPR = 1
> ORDER BY P.DisplayName
> -- List Validation Authorities
> IF (@.testRoleEnum = 6)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidationAuthority = 1
> ORDER BY P.DisplayName
> -- List Officials
> IF (@.testRoleEnum = 7)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleOfficial = 1
> ORDER BY P.DisplayName
> -- List Test Directors
> IF (@.testRoleEnum = 8)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleTestDirector = 1
> ORDER BY P.DisplayName
> -- List Survey Participants
> IF (@.testRoleEnum = 9)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleSurveyParticipant = 1
> ORDER BY P.DisplayName
> SET NOCOUNT OFF
>GO
>SET QUOTED_IDENTIFIER OFF
>GO
>SET ANSI_NULLS ON
>GO
>
>|||Well, a more efficient design would store the numeric coefficient in a
single column, instead of having 9 different columns. It's hard to tell
from your requirements exactly what solution you need (e.g. can someone be a
screener and a roleSME?). But this is certainly not the best approach.
When you add or remove a role type, you have to change table structure and
all the code that references it. Blecch.
If a personnel member can only be in one role, then this design makes more
sense, IMHO:
CREATE TABLE dbo.TestRoles
(
TestRoleEnum INT PRIMARY KEY,
Description VARCHAR(32) UNIQUE
)
SET NOCOUNT ON;
INSERT dbo.TestRoles(Description) SELECT 'Originator';
...
CREATE TABLE dbo.TestParticipants
(
Personnelid INT /* FOREIGN KEY... */,
TestRoleEnum INT
)
Now your stored procedure can say:
ALTER PROCEDURE dbo.ListParticipantsByRole
@.testRoleEnum int,
@.errorID int = 0 OUTPUT
AS
BEGIN
SET NOCOUNT ON;
SELECT
P.DisplayName,
T.Personnelid
FROM
dbo.TestParticipants t
INNER JOIN
Common.dbo.vwPersonnelInfo p
ON
t.Personnelid = P.personellid
WHERE
t.TestRoleEnum = @.testRoleEnum
ORDER BY
P.DisplayName;
END
GO
And now you can add and remove test roles as you please, without having to
change any of the database structure. And as a bonus, your code becomes a
lot easier to read.
"Mike Collins" <MikeCollins@.discussions.microsoft.com> wrote in message
news:C4C1B5B8-7E49-467A-83C7-4DEEED88B57B@.microsoft.com...
> In the stored procedure below, there are 9 different if conditions being
> checked. I was wondering if there is a more efficient way (or not) to
> write
> this procedure, without having to write the same select criteria each
> time.
> The only thing different is the where clause. Any suggestions are greatly
> appreciated.
> SET QUOTED_IDENTIFIER ON
> GO
> SET ANSI_NULLS ON
> GO
> ALTER PROC ListParticipantsByRole(
> @.testRoleEnum int,
> @.errorID int = 0 OUTPUT)
> AS
> SET NOCOUNT ON
> -- List Originators
> IF (@.testRoleEnum = 1)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleOriginator = 1
> ORDER BY P.DisplayName
> -- List Validators
> IF (@.testRoleEnum = 2)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidator = 1
> ORDER BY P.DisplayName
> -- List Screeners
> IF (@.testRoleEnum = 3)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleScreener = 1
> ORDER BY P.DisplayName
> -- List SMEs
> IF (@.testRoleEnum = 4)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleSME = 1
> ORDER BY P.DisplayName
> -- List Validation OPRs
> IF (@.testRoleEnum = 5)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidationOPR = 1
> ORDER BY P.DisplayName
> -- List Validation Authorities
> IF (@.testRoleEnum = 6)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidationAuthority = 1
> ORDER BY P.DisplayName
> -- List Officials
> IF (@.testRoleEnum = 7)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleOfficial = 1
> ORDER BY P.DisplayName
> -- List Test Directors
> IF (@.testRoleEnum = 8)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleTestDirector = 1
> ORDER BY P.DisplayName
> -- List Survey Participants
> IF (@.testRoleEnum = 9)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleSurveyParticipant = 1
> ORDER BY P.DisplayName
> SET NOCOUNT OFF
> GO
> SET QUOTED_IDENTIFIER OFF
> GO
> SET ANSI_NULLS ON
> GO
>|||try this.. and see if it works faster...
writing for 2 ofthem.. u can add the rest..
Hope this helps.
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE @.testRoleEnum = 1 and T.TestRoleOriginator = 1
ORDER BY P.DisplayName
union all
SELECT P.DisplayName, T.PersonnelID
FROM TestParticipants T
JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
WHERE @.testRoleEnum = 2 and T.TestRoleValidator = 1
ORDER BY P.DisplayName|||Thank you all for the quick and great responses. I see now better ways of
designing my tables and how to better write my stored procedure. In this
scenario, a person can have more than one role.
"Mike Collins" wrote:

> In the stored procedure below, there are 9 different if conditions being
> checked. I was wondering if there is a more efficient way (or not) to writ
e
> this procedure, without having to write the same select criteria each tim
e.
> The only thing different is the where clause. Any suggestions are greatly
> appreciated.
> SET QUOTED_IDENTIFIER ON
> GO
> SET ANSI_NULLS ON
> GO
> ALTER PROC ListParticipantsByRole(
> @.testRoleEnum int,
> @.errorID int = 0 OUTPUT)
> AS
> SET NOCOUNT ON
> -- List Originators
> IF (@.testRoleEnum = 1)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleOriginator = 1
> ORDER BY P.DisplayName
> -- List Validators
> IF (@.testRoleEnum = 2)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidator = 1
> ORDER BY P.DisplayName
> -- List Screeners
> IF (@.testRoleEnum = 3)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleScreener = 1
> ORDER BY P.DisplayName
> -- List SMEs
> IF (@.testRoleEnum = 4)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleSME = 1
> ORDER BY P.DisplayName
> -- List Validation OPRs
> IF (@.testRoleEnum = 5)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidationOPR = 1
> ORDER BY P.DisplayName
> -- List Validation Authorities
> IF (@.testRoleEnum = 6)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleValidationAuthority = 1
> ORDER BY P.DisplayName
> -- List Officials
> IF (@.testRoleEnum = 7)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleOfficial = 1
> ORDER BY P.DisplayName
> -- List Test Directors
> IF (@.testRoleEnum = 8)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleTestDirector = 1
> ORDER BY P.DisplayName
> -- List Survey Participants
> IF (@.testRoleEnum = 9)
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE T.TestRoleSurveyParticipant = 1
> ORDER BY P.DisplayName
> SET NOCOUNT OFF
> GO
> SET QUOTED_IDENTIFIER OFF
> GO
> SET ANSI_NULLS ON
> GO
>|||If you are planning to use a case statement, I would suggest going for a
union all
like I mentioned since it helps in better use of indexes, if any. If you
have not indexed the columns in the where clause, then go for the case. But
,
if you are redesining the table, thats the best way.|||That's a fairly straightforward approach. And here I was going to suggest
dynamic SQL.
Of course, the various suggestions to change the table structure are much
better in the long run.
"Raymond D'Anjou" <rdanjou@.canatradeNOSPAM.com> wrote in message
news:u9$RbXPeGHA.564@.TK2MSFTNGP02.phx.gbl...
> SELECT P.DisplayName, T.PersonnelID
> FROM TestParticipants T
> JOIN Common..vwPersonnelInfo P ON T.PersonnelID = P.PersonnelId
> WHERE 1 = CASE @.testRoleEnum
> when 1 then T.TestRoleOriginator
> when 2 then T.TestRoleValidator
> ...
> end
> ORDER BY P.DisplayName
> "Mike Collins" <MikeCollins@.discussions.microsoft.com> wrote in message
> news:C4C1B5B8-7E49-467A-83C7-4DEEED88B57B@.microsoft.com...
greatly
>|||"Jim Underwood" <james.underwoodATfallonclinic.com> wrote in message
news:O9EDeiPeGHA.3556@.TK2MSFTNGP02.phx.gbl...
> That's a fairly straightforward approach. And here I was going to suggest
> dynamic SQL.
> Of course, the various suggestions to change the table structure are much
> better in the long run.
I'm getting lazy. :-)|||Thanks for the additional input. Unfortunately it is not my decision to
redesign the database and I might have to use the case statements. I am
definitely going to give my two cents about redesigning the database.
"Omnibuzz" wrote:

> If you are planning to use a case statement, I would suggest going for a
> union all
> like I mentioned since it helps in better use of indexes, if any. If you
> have not indexed the columns in the where clause, then go for the case. Bu
t ,
> if you are redesining the table, thats the best way.sql