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

Posted on


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 
    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

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

-- Return the resulting string to the user
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).


A handful of critiques:

  • 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
  • Don’t just blindly insert into a table; always specify a column list.
  • Its cheaper to use table variables than temp tables; SQL Server doesn’t need to do as much work for statistics and fewer latches of creation are required.
  • You generally don’t need to drop the temp table (assuming this is called from a stored procedure) as that’ll get done automatically by SQL Server
  • You could probably just return the list of disciplines + an error code, and let the consuming application handle presenting it in a friendly way. This also makes I18N someday in the future much easier.

Leave a Reply

Your email address will not be published. Required fields are marked *