# Stored procedure to describe the reason that a particular employee is unsuitable for a particular task

I used the following Stack Overflow questions as references for writing this code:

The idea behind my stored procedure is to give an explanation of why a particular worker is unsuitable to care for a particular patient under specific circumstances. For example, you obviously can't assign an RN to do physical therapy visits.

I have a Disciplines table, which has a structure that's kind of like this:

--------------------------------------------
| Discipline | Discipline ID | Other Field |
--------------------------------------------
| RN         | 1             | 2           |
--------------------------------------------
| PT         | 2             | 5           |
--------------------------------------------
| CNA        | 3             | 7           |
--------------------------------------------
| MD         | 4             | 23           |
--------------------------------------------


Obviously this is hypothetical data and I've modified the column names a bit (this is mostly for illustration).

What I want is to retrieve the list of Disciplines as a single string and concatenate it with an existing string. Workers might have several disciplines. (This is determined in a table that's not shown). An example of one possible output would be The worker does not have the appropriate disciplines. They have the following discipline(s): RN, PT (if that employee is both a RN and a PT - not too likely, obviously, but let's assume that they are).

I haven't included the entire stored procedure, but this is pretty representative and is the main part that I'm looking for a critique of:

 -- @IsAppropriate is a bit type
SELECT @IsAppropriate = dbo.fnCheckIfWorkerHasDisciplineForSvc(@BusinessUnitID, @WorkerID, @ServiceTypeID, GetDate(), GetDate())

IF @IsAppropriate = 0
BEGIN
SET @output = 'The worker does not have the appropriate disciplines. They have the following discipline(s):'

CREATE TABLE #Result
(
Discipline nvarchar(max),
IsPrimary bit,
EffectiveDate DateTime,
EndDate DateTime
)

-- Get which discipline a particular worker practices (PT, MD, RN, etc.)
INSERT #Result EXEC spGetWorkerDiscipline @WorkerID

DECLARE @disciplines nvarchar(max);

-- Get them as a nice list
SELECT @disciplines = coalesce(@disciplines + ', ', '') + Discipline from #Result

SET @output = @output + @disciplines

DROP TABLE #Result

-- Skip the rest of the stored procedure
-- See comments below - is this a terrible thing to do?
GOTO eom
END

-- Check some other reasons that the worker might be inappropriate...

-- Return the resulting string to the user
eom:
select @output


I verified that the query does, in fact, work. However, I have a few concerns:

• Is using GOTO like that in SQL really terrible? If so, is there a better way to accomplish the same thing? I'm almost embarrassed to post code that contains it, since I'm aware that, in general, GOTO is considered harmful.
• Will I take a performance hit for creating and dropping a temporary table like that? Could some of the other suggestions in the linked posts be a better choice here?
• Am I inappropriately mixing my architectural layers by creating a string for use on the front end in the database like this? (I'm using ASP.NET Web Forms).
• Your approach to string concatenation is not guaranteed. stackoverflow.com/questions/15138593/… – Martin Smith Mar 12 '17 at 19:16
• You could change GOTO eom to SELECT @output RETURN in each of your IF blocks. If your #Result table is the same for all IF blocks then just declare it outside of all of the blocks (once). Also, I'd check to see if it exists first with if object_id('tempdb..#Result') is not null drop table #Result or you may prefer a table variable. Lastly I think a function in your application layer could handle this but it's more preference than anything. – scsimon Mar 13 '17 at 16:09
• As an aside, using a if x then goto instead of a while can perform better than a typical while loop. I'm not sure if this is the case in more recent sql server versions but it's true in 05 and 08 at least. Also, have you tried using OPENQUERY in place of the temp table? – PenutReaper Apr 9 '17 at 3:35

• As mentioned in the comments, this is undefined behavior: SELECT @val = @val + ColumnName FROM AnyTable. As such, you should avoid it. If you need to, use STUFF or STRING_AGG instead
• When creating a temp table, (n)varchar columns should specify their collation (e.g. COLLATE DATABASE_DEFAULT ) to avoid errors in case server collation and database collation differ.
• Using RETURN or a chain of IF ELSE IF etc is going to raise fewer eyebrows than GOTO