3

I have this stored procedure that I want the max startDate base on AgentId or without the AgentId. The way I am doing this is using a if else and wanted to see if there was a better way to do this?

  IF (@AgentId = 0)
      BEGIN
          SELECT top (1)max(cgv.StartDate) as AgentLatestPublishedDate,ag.Name
          FROM compendia.Agent ag
          JOIN compendia.DrugCompendium AS dc ON dc.AgentId = ag.OriginalAgentId 
          JOIN compendia.CompendiaGuidelineVersion cgv ON cgv.CompendiaGuidelineVersionId = dc.CompendiaGuidelineVersionId
          JOIN guideline.Disease AS d ON d.DiseaseId = dc.DiseaseId
          WHERE cgv.WorkFlowStatusId = 6 AND ag.EndDate IS NULL AND dc.IsNoLongerRecommended = 0
          group by ag.AgentId,ag.Name
      end

    ELSE
    BEGIN
          SELECT max(cgv.StartDate) as AgentLatestPublishedDate,ag.Name
          FROM compendia.Agent ag
          JOIN compendia.DrugCompendium AS dc ON dc.AgentId = ag.OriginalAgentId 
          JOIN compendia.CompendiaGuidelineVersion cgv ON cgv.CompendiaGuidelineVersionId = dc.CompendiaGuidelineVersionId
          JOIN guideline.Disease AS d ON d.DiseaseId = dc.DiseaseId
          WHERE cgv.WorkFlowStatusId = 6 AND ag.EndDate IS NULL AND dc.IsNoLongerRecommended = 0 and ag.AgentId = @AgentId
          group by ag.AgentId,ag.Name
    end
Improve this question
0

3 Answers 3

Reset to default
4

Current solution

Your version is certainly not the worst. Main problem is you have the same code twice, which has some maintenance overhead (i. e. all changes must be applied to both code blocks).

But T-SQL is not Java or C# - it doesn't give you as many tools for code deduplication (and many of the deduplication tools it does give you will make things more difficult for optimizer). Application programmers might say Don't Repeat Yourself, but as T-SQL programmer, sometimes you just have to.

One might reasonably argue to just leave it as it is.

Naive solution

You could use single block with slightly different filter: ... and (ag.AgentId = @AgentId OR @AgentId = 0). But the problem is, that ORs make things more difficult for optimizer. SQL Server creates execution plan for query once, and then reuses it for subsequent runs (until the plan gets push out of the memory). This is nice, because it save time on compilations. However, it also means that it always has to check whether ag.AgentId = @AgentId, because it doesn't know in advance what value will you pass into the parameter

Fine for small datasets, avoid otherwise.

Dynamic solution

This will make things easier for optimizer, and you will not have the same code twice. It relies on dynamic SQL to have only single copy of your SQL code, and alters it to either include or skip the @AgentId check.

DECLARE @sql NVARCHAR(MAX) = N'
SELECT max(cgv.StartDate) as AgentLatestPublishedDate,ag.Name
FROM compendia.Agent ag
JOIN compendia.DrugCompendium AS dc ON dc.AgentId = ag.OriginalAgentId 
JOIN compendia.CompendiaGuidelineVersion cgv ON cgv.CompendiaGuidelineVersionId = dc.CompendiaGuidelineVersionId
JOIN guideline.Disease AS d ON d.DiseaseId = dc.DiseaseId
WHERE cgv.WorkFlowStatusId = 6 
  AND ag.EndDate IS NULL 
  AND dc.IsNoLongerRecommended = 0 
  <<AGENT_FILTER_HERE>>  /*Notice the placeholder here*/
group by ag.AgentId,ag.Name
';

/*Now you either remove the placeholder, or replace it with actual condition.*/
IF @AgentId = 0
  SET @sql = REPLACE(@sql, N'<<AGENT_FILTER_HERE>>', N'');
ELSE
  SET @sql = REPLACE(@sql, N'<<AGENT_FILTER_HERE>>', N'and ag.AgentId = @AgentId');

/*Executing the parametrized dynamic SQL.*/
EXECUTE sp_executesql @sql, N'@AgentId INT', @AgentId = @AgentId;
Improve this answer
4

If you really want to avoid repating yourself (DRY), and you don't want to use dynamic SQL, then you could use an inline Table Valued Function.

The TOP (1) in the second branch seems extraneous.

CREATE OR ALTER FUNCTION dbo.GetAgentInfo (@HasAgentFilter bit, @AgentId bigint)
RETURNS TABLE
AS RETURN
    SELECT
      MAX(cgv.StartDate) as AgentLatestPublishedDate,
      ag.Name
    FROM compendia.Agent ag
    JOIN compendia.DrugCompendium AS dc ON dc.AgentId = ag.OriginalAgentId 
    JOIN compendia.CompendiaGuidelineVersion cgv ON cgv.CompendiaGuidelineVersionId = dc.CompendiaGuidelineVersionId
    JOIN guideline.Disease AS d ON d.DiseaseId = dc.DiseaseId
    WHERE cgv.WorkFlowStatusId = 6
      AND ag.EndDate IS NULL
      AND dc.IsNoLongerRecommended = 0
      AND (@HasAgentFilter = 1 AND ag.AgentId = @AgentId OR @HasAgentFilter = 0)
    GROUP BY
      ag.AgentId,
      ag.Name;

You still need the IF, so that you can pass a constant for the @HasAgentFilter parameter, that way you get the optimizer to elide the incorrect branch.

Do NOT pass a variable for the first parameter, only a constant, otherwise you are back to the same compiler issue that the other answer mentions (in Naive Solution).

IF @AgentId = 0
    SELECT a.*
    FROM dbo.GetAgentInfo(0, 0) a;
ELSE
    SELECT a.*
    FROM dbo.GetAgentInfo(1, @AgentId) a;
Improve this answer
1

Depending on your version of SQL you can do this syntax

In your ELSE block, update:

and ag.AgentId = @AgentId

to:

and(@AgentId = 0 OR ag.AgentId = @AgentId)

SQL server resolves this to not apply the AND filter if the first expression is true, OR it applies the AND filter

then you can delete the entire IF block and the ELSE, leaving only the query from your ELSE block

Improve this answer
2
  • Yeah, I don't know who downvoted...this is tested and works Commented Feb 1 at 0:40
  • There are idiots downvoting all the time, and wont leave a comment to explain. Commented Feb 1 at 9:44

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.