0

I am building this where condition by concatenating the @whereClause variable in my stored procedure. At this point I am using the case to add additional values. if there a better way to do this?

SET @whereClause  = 
    ( CASE  
        -- APPType Clinical
         WHEN (@ApplicationType = 'Clinical') THEN @whereClause + ' AND vpgl.AppType = Clinical'
         -- APPType Patients
         WHEN (@ApplicationType = 'Patients') THEN @whereClause + ' AND vpgl.AppType = Patients'
          -- APPType Web
         WHEN (@ApplicationType = 'Web') THEN @whereClause + ' AND vpgl.AppType = null'
         ELSE @whereClause
       END  
    ) 
1
  • Shouldn't = Clinical be = 'Clinical'? Shouldn't = null be IS NULL? Why not just pass that in as a proper parameter? Commented Jun 17, 2020 at 16:32

2 Answers 2

2

I think this is a better, safer, more self-documenting, and more scalable approach:

IF @ApplicationType IN ('Clinical','Patients')
BEGIN
  SET @whereClause += N' AND vpgl.AppType = @ApplicationType';
END

IF @ApplicationType = 'Web'
BEGIN
  SET @whereClause += N' AND vpgl.AppType IS NULL';
END

...
SET @sql = @sql + @whereClause;

EXEC sys.sp_executesql @sql,
     N'@ApplicationType varchar(32)',
     @ApplicationType;

If you really, really want to use a CASE expression (which is not meant for control of flow), you can say:

SET @whereClause += CASE 

   WHEN @ApplicationType IN ('Clinical','Patients')
   THEN N' AND vpgl.AppType = @ApplicationType'

   WHEN @ApplicationType = 'Web'
   THEN N' AND vpgl.AppType IS NULL';

   ELSE N'' END;

...
SET @sql = @sql + @whereClause;

EXEC sys.sp_executesql @sql,
     N'@ApplicationType varchar(32)',
     @ApplicationType;
5
  • would I need to do something like this SET @whereClause += N' AND vpgl.AppType =' + @ApplicationType + ';
    – Jefferson
    Commented Jun 17, 2020 at 17:49
  • 1
    @Jefferson No, because you should be passing that in as a parameter. String concatenation is messy (you need to add doubled-up single quotes) and dangerous (it helps enable SQL injection). Commented Jun 17, 2020 at 18:42
  • right like this @ApplicationType as varchar(50) = NULL but will it just take the string value so it would look thisvpgl.AppType = @ApplicationType
    – Jefferson
    Commented Jun 17, 2020 at 19:44
  • 1
    @Jefferson did you try it? Maybe to clarify I'll show that the variable that holds @ApplicationType is different from the param you pass it into for the dynamic SQL. DECLARE @sql nvarchar(max), @AppVariable varchar(32) = 'from variable'; SET @sql = N'SELECT app_from_param_but_really_from_variable = @AppParam;'; EXEC sys.sp_executesql @sql, N'@AppParam varchar(32)', @AppParam = @AppVariable; or see this fiddle. Commented Jun 17, 2020 at 19:55
  • I understand what you mean now Thanks You
    – Jefferson
    Commented Jun 17, 2020 at 19:57
0

You can make it shorter:

SET @whereClause += (CASE @ApplicationType
                          -- APPType Clinical
                          WHEN 'Clinical' THEN ' AND vpgl.AppType = Clinical'
                          -- APPType Patients
                          WHEN 'Patients' THEN ' AND vpgl.AppType = Patients'
                          -- APPType Web
                          WHEN 'Web' THEN ' AND vpgl.AppType = null'
                          ELSE ''
                    END);

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.