2

Option 1:

CREATE PROCEDURE [dbo].[GetStudents]
@MinimumAge int = NULL
AS

select * from Students s where @MinimumAge is null or s.Age >= @MinimumAge

Option 2:

CREATE PROCEDURE [dbo].[GetStudents]
@MinimumAge int = NULL
AS

IF @MinimumAge IS NULL
BEGIN
    select * from Students s
END
ELSE
BEGIN
    select * from Students s where s.Age >= @MinimumAge
END

Will option 1 be slower than option 2 because it has extra WHERE clause? Or will SQL Server take care of that ?

I think option 1 is good as I don't have to duplicate the code, unless it's slower. The original procedure has many lines of code. So I don't want to duplicate all code just for one condition, unless it's the only good option.

There is indexing on the age column and it does not allow nulls.

The problem is, Actual SP is having many lines of code - So I don't want to duplicate all code just to add one where condition, unless it's the only good option.

4

Community wiki answer:

You could also try:

WHERE Age >= COALESCE(@MnimumAge, 0)

Assuming 0 is not a valid Age. Otherwise, you could use -1 as a default instead of 0.

This will allow a seek on an index keyed on Age.


The other main option is to add OPTION (RECOMPILE) to the statement:

CREATE PROCEDURE [dbo].[GetStudents]
    @MinimumAge int = NULL
AS
BEGIN
    SELECT S.* 
    FROM dbo.Students AS S
    WHERE @MinimumAge IS NULL
    OR S.Age >= @MinimumAge
    OPTION (RECOMPILE);
END;

This adds a small overhead on each call as the statement (not whole procedure) is recompiled, but it allows the parameter embedding optimization, so you will get an optimal execution plan for the specific value of @MinimumAge on each execution. See Parameter Sniffing, Embedding, and the RECOMPILE Options by Paul White for more information.

You might also like to read #BackToBasics : An Updated "Kitchen Sink" Example by Aaron Bertrand and review the related Q & A SQL Server--If logic in stored procedure and the plan cache

3

From my experience, the best solution is a third one, which follows the rule for 'one procedure for one task'. Your procedure currently serves 2 tasks - returning all employees, and returning a sub set based on age. 2 different things, in one procedure, is a source of future grief. Probably not with a simplified statement like you gave, but very likely with more complex code.

Here is how I would do it:

CREATE PROCEDURE [dbo].[GetAllStudents]
AS
BEGIN
    SELECT  * 
    FROM    Students;
END;
GO

CREATE PROCEDURE [dbo].[GetOldStudentsByAge] 
@MinimumAge INT
AS
BEGIN
    SELECT * 
    FROM Students AS a 
    WHERE s.Age >= @MinimumAge;
END;
GO

CREATE PROCEDURE [dbo].[GetStudents]
@MinimumAge INT = NULL
AS
BEGIN
    IF @MinimumAge IS NULL 
        EXECUTE dbo.[GetAllStudents];
    ELSE 
        EXECUTE dbo.[GetOldStudentsByAge] @MinimumAge;
END;

Now, every procedure has a single purpose, uses a simple statement with no application logic in it, it is much clearer and easier to read and maintain IMHO, and you will have no parameter / caching issues due to subsequent executions using different logic, as each sub-procedure will only have one good plan. You can still run into parameter issues if @MinimumAge values differ significantly enough to require a different plan. That can be solved too, but is a separate issue.

  • So do I have to duplicate the code in two SPs ? And use 3rd SP as a wrapper ? The problem is, Actual SP is having many lines of code - So I don't want to duplicate all code in 2 SPs, unless it's the only good option. – Jay Shah Jun 7 '18 at 10:01
  • Well, the designer of the application can change this to make separate calls for each task, in which case the 3rd wrapper won't be needed... – SQLRaptor Jun 7 '18 at 16:12
2

Yes, it can slow down the query if the column is indexed.

SELECT * FROM Students  WHERE @MinimumAge IS NULL OR Age >= @MinimumAge --Index Scan

vs

SELECT * FROM Students  WHERE Age > @MinimumAge --Index Seek

The IS NULL does have a performance impact, you can get an Index Scan rather than an Index Seek, so there is an additional IO overhead.

If the additional IO overhead is negligible then I'd go with Option 1 and just keep an eye on it. With Option 2 you're complicating the SP and opening yourself up to Parameter Sniffing issues, and that's an issue that can hit you out of the blue with no warning and lead to a miserable day at the office.

1

I think this will work

select * from Students a where @MinimumAge is null  
union all
select * from Students a where s.Age >= @MinimumAge
  • Can you please explain that how is it better than option 1 or option 2 ? – Jay Shah Jun 6 '18 at 22:39
  • @JayShah I suggest you test it out. It is basically option 2 with less syntax. – paparazzo Jun 6 '18 at 22:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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