1
\$\begingroup\$

I'd like to make this SQL scalar function more efficient. How can I do that?

CREATE FUNCTION [dbo].[fnFilterBySampleType]
(
    @context VARCHAR(10),
    @contextId INT,
    @sampleTypeId INT
)
RETURNS BIT
AS
BEGIN

    IF (
        @context = 'batch' AND @sampleTypeId = 247
        AND EXISTS(
            SELECT * 
            FROM batch 
            WHERE batchid = @contextId 
            AND (batch.sampletypeid = @sampleTypeId OR batch.sampletypeid IS NULL)
        ) 
    ) RETURN 1 

    IF (
        @context = 'batch' AND @sampleTypeId = 3301
        AND EXISTS(
            SELECT * 
            FROM batch 
            WHERE batchid = @contextId 
            AND batch.sampletypeid = @sampleTypeId
        ) 
    ) RETURN 1 

    IF (
        @context = 'batch' AND @sampleTypeId IS NULL
        AND EXISTS(
            SELECT * 
            FROM batch 
            WHERE batchid = @contextId 
            AND batch.sampletypeid IS NULL
        ) 
    ) RETURN 1 

    IF (
        @context = 'sample' AND @sampleTypeId = 247
        AND EXISTS(
            SELECT * 
            FROM sample
            WHERE sampleid = @contextId 
            AND ([sample].sampletypeid = @sampleTypeId OR [sample].sampletypeid IS NULL)
        ) 
    ) RETURN 1  

    IF (
        @context = 'sample' AND @sampleTypeId = 3301
        AND EXISTS(
            SELECT * 
            FROM sample
            WHERE sampleid = @contextId 
            AND sample.sampletypeid = @sampleTypeId
        ) 
    ) RETURN 1 

    IF (
        @context = 'sample' AND @sampleTypeId IS NULL
        AND EXISTS(
            SELECT * 
            FROM sample
            WHERE sampleid = @contextId 
            AND [sample].sampletypeid IS NULL
        ) 
    ) RETURN 1 

    RETURN 0

END
\$\endgroup\$
2
\$\begingroup\$

I am rewriting my answer to be more clear...

the way you have it written the exists clause will be executed many times...

You should move the exists clauses inside the if block so that in the cases that the other parts don't match it will be forced to short circuit. You can't depend on SQL server tsql statements to short circuit in the manner you expect.

So this will not execute the inner "IF exists" if the @context isn't 'batch' etc...

The way you have it written it may or may not execute the exists query.

IF (@context = 'batch' AND @sampleTypeId = 247)
begin

        if EXISTS(
            SELECT * 
            FROM batch 
            WHERE batchid = @contextId 
            AND (batch.sampletypeid = @sampleTypeId OR batch.sampletypeid IS NULL)
        ) RETURN 1  ELSE RETURN 0

end

for more reading on this subject check out this articles.

http://beingmarkcohen.com/?p=62

http://www.sqlmag.com/article/tsql3/short-circuit

http://blogs.msdn.com/b/bartd/archive/2011/03/03/don-t-depend-on-expression-short-circuiting-in-t-sql-not-even-with-case.aspx

\$\endgroup\$
  • \$\begingroup\$ sorry, I accidentally gave you an upvote. This is not correct though. Adding more tags so others will see it. \$\endgroup\$ – MacGyver Jan 30 '12 at 4:12
  • \$\begingroup\$ where did the 3301 and NULL checks go? \$\endgroup\$ – MacGyver Jan 30 '12 at 15:00
  • \$\begingroup\$ I didn't rewrite them all just the first one. \$\endgroup\$ – John Sobolewski Jan 30 '12 at 15:23
  • \$\begingroup\$ oh ok. is there a way I can combine more than 1 of them above to a single query? \$\endgroup\$ – MacGyver Jan 30 '12 at 22:50
  • \$\begingroup\$ there might be but I think doing so would make the code more confusing. \$\endgroup\$ – John Sobolewski Jan 31 '12 at 12:50

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.