Looking for a way to avoid a cursor

Question

I am trying to create a report that, among other things, lists how many new patients were seen in a particular month. A "new" patient is someone who has either 1) never been seen before, or 2) has been seen before, but had their last visit more than three years ago. I am currently trying to do this with a cursor that calls a table-valued function that reports the number of new patients seen on a given date. The cursor calls the function for each visit date in the last six years, and the execution time is abyssmal. Listed below are the cursor query logic, and below it the table-valued function definition. I also include the definition of the indexed view "vwKeptVisits" referenced by the function. Finally, I give some sample query results. These results are what I am trying to produce with the query, just looking for a more efficient way to do it... I am looking for ideas on how to do this without a cursor.

//CURSOR LOGIC//

DECLARE @BEGINYEAR  INT;
SET @BEGINYEAR = YEAR(GETDATE()) - 6;

DECLARE @TBLNewPts  TABLE(
VisitDate   SMALLDATETIME,
NewPtCount  INT
)

DECLARE @VisitDate  SMALLDATETIME

DECLARE TblCursor   CURSOR 
FAST_FORWARD

FOR
    SELECT DISTINCT VisitDate
    FROM dbo.Visits
    WHERE YEAR(VisitDate) > @BEGINYEAR 
    
OPEN TblCursor
FETCH NEXT FROM TblCursor INTO @VisitDate
WHILE @@FETCH_STATUS = 0

BEGIN

    INSERT @TBLNewPts 
    SELECT
    @VisitDate, 
    ISNULL(NewPts, 0)
            
    FROM dbo.fnNewPatientsByVisitDate(@VisitDate) 
    
FETCH NEXT FROM TblCursor INTO @VisitDate

END

CLOSE   TblCursor
DEALLOCATE TblCursor;

///////////////////////////////////////////////////////////////////////////////////////////////
CREATE FUNCTION [dbo].[fnNewPatientsByVisitDate]
(
@VisitDate  SMALLDATETIME
)
RETURNS 
@tbl TABLE 
(
NewPts  INT
)
WITH SCHEMABINDING

AS
BEGIN
    DECLARE @TBLNewPts  TABLE(
    VisitDate   SMALLDATETIME,
    PtID    INT
    )

    INSERT @TBLNewPts 
    --PATIENTS WHO HAVE NEVER BEEN SEEN BEFORE @VISITDATE
    SELECT 
    @VisitDate,
    PtID
    FROM dbo.vwKeptVisits 
    
    WHERE (VisitDate = @VisitDate)
          AND
          (PtID NOT IN 
                (SELECT PtID
                FROM dbo.vwKeptVisits
                WHERE VisitDate < @VisitDate)
                )   

    UNION

    --PATIENTS SEEN BEFORE, BUT MORE THAN THREE YEARS BEFORE @VISITDATE
    SELECT 
    @VisitDate,
    PtID    
    FROM dbo.vwKeptVisits 
    
    WHERE (VisitDate = @VisitDate)
          AND
          (PtID NOT IN 
                (SELECT PREVIOUS.PtID
                FROM dbo.vwKeptVisits AS PREVIOUS INNER JOIN
                    dbo.vwKeptVisits AS CURRNT ON
                    CURRNT.PtID = PREVIOUS.PtID AND
                    CURRNT.VisitNumber > PREVIOUS.VisitNumber 
                WHERE (PREVIOUS.VisitDate > DATEADD(YEAR,-3,@VisitDate)) 
                ));           

    INSERT @tbl
    SELECT
    COUNT(PtID)
    FROM @TBLNewPts ;

    RETURN 

END

/////////////////////////////////////////////////////////////////////////////////////////
/****** Object:  View [dbo].[vwKeptVisits]    Script Date: 6/18/2024 8:31:05 PM ******/

CREATE VIEW [dbo].[vwKeptVisits]
WITH SCHEMABINDING
AS
SELECT 
COUNT_BIG(*) AS ServiceCount,
V.VisitNumber,
V.PtID,
V.VisitDate,
V.MDID,
V.ApptID,
V.DX1,
V.DX2,
v.DX3,
v.DX4,
V.DX5,
V.POS 
FROM DBO.Visits V INNER JOIN
     dbo.[Visit Details] VD ON 
     V.VisitNumber = VD.VisitNumber
WHERE (V.Void = 0) AND 
(VD.[CPT CODE] <> 'No Show' AND
  VD.[CPT CODE] <> 'MD Cancel' AND
  VD.[CPT CODE] <> 'Pt Cancel') AND
  (V.ApptID IS NOT NULL)
GROUP BY V.VisitNumber, v.PtID , v.VisitDate, V.MDID, V.ApptID,
V.DX1, V.DX2, v.DX3, v.DX4, V.DX5, V.POS
GO

GRANT SELECT ON [dbo].[vwKeptVisits] TO [db_PracManUserRole] AS [dbo]
GO

SET ARITHABORT ON
SET CONCAT_NULL_YIELDS_NULL ON
SET QUOTED_IDENTIFIER ON
SET ANSI_NULLS ON
SET ANSI_PADDING ON
SET ANSI_WARNINGS ON
SET NUMERIC_ROUNDABORT OFF
GO

/****** Object:  Index [PK_vwKeptVisits]    Script Date: 6/18/2024 8:31:05 PM ******/
CREATE UNIQUE CLUSTERED INDEX [PK_vwKeptVisits] ON [dbo].[vwKeptVisits]
(
    [VisitNumber] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = ON, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, FILLFACTOR = 95, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [INDICESGROUP]
GO

SET ARITHABORT ON
SET CONCAT_NULL_YIELDS_NULL ON
SET QUOTED_IDENTIFIER ON
SET ANSI_NULLS ON
SET ANSI_PADDING ON
SET ANSI_WARNINGS ON
SET NUMERIC_ROUNDABORT OFF
GO

/****** Object:  Index [IX_vwKeptVisits_MDID]    Script Date: 6/18/2024 8:31:05 PM ******/
CREATE NONCLUSTERED INDEX [IX_vwKeptVisits_MDID] ON [dbo].[vwKeptVisits]
(
    [MDID] ASC
)
INCLUDE([PtID],[VisitDate]) WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = ON, SORT_IN_TEMPDB = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, FILLFACTOR = 95, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [INDICESGROUP]
GO

SET ARITHABORT ON
SET CONCAT_NULL_YIELDS_NULL ON
SET QUOTED_IDENTIFIER ON
SET ANSI_NULLS ON
SET ANSI_PADDING ON
SET ANSI_WARNINGS ON
SET NUMERIC_ROUNDABORT OFF
GO

/****** Object:  Index [IX_vwKeptVisits_PtID_VisitDate]    Script Date: 6/18/2024 8:31:05 PM ******/
CREATE NONCLUSTERED INDEX [IX_vwKeptVisits_PtID_VisitDate] ON [dbo].[vwKeptVisits]
(
    [PtID] ASC,
    [VisitDate] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = ON, SORT_IN_TEMPDB = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, FILLFACTOR = 96, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [INDICESGROUP]
GO

////SAMPLE QUERY RESULTS///////

VisitDate   NewPtCount
2019-01-02 00:00:00 0
2019-01-03 00:00:00 0
2019-01-04 00:00:00 0
2019-01-07 00:00:00 2
2019-01-08 00:00:00 3
2019-01-09 00:00:00 0
2019-01-10 00:00:00 4
2019-01-11 00:00:00 5
2019-01-14 00:00:00 1
2019-01-15 00:00:00 0
2019-01-16 00:00:00 0
2019-01-17 00:00:00 0
2019-01-18 00:00:00 0
2019-01-21 00:00:00 0
2019-01-22 00:00:00 1

The current query time is more than 35 seconds.

Rather than expecting us to reverse engineer your code to understand your business logic, please include a minimal reproducible example with both sample (initial) data and your desired results. — Dale K
– Dale K, Commented Jun 19, 2024 at 1:20
^^^ With emphasis on minimal what you've posted seems pretty complex, so you can't really expect people to trawl through that and make sense of it. And its unlikely to be necessary, because learning how to write fully set-based data retrieval instead of RBAR is more of a concept, so probably if you create a simple example which people can answer you'll be able to scale it up to your actual code. Thats the theory anyway. — Dale K
– Dale K, Commented Jun 19, 2024 at 1:27
Since fnNewPatientsByVisitDate is a table-valued function have you considered CROSS APPLYing it to the original query? — AlwaysLearning
– AlwaysLearning, Commented Jun 19, 2024 at 1:35
If it's a performance-sensitive task then consider converting your multi-statement table-valued function to an inline table-valued function. i.e.: rewrite it to be a single statement with set-based results instead of inserting into an intermediate @table variable. — AlwaysLearning
– AlwaysLearning, Commented Jun 19, 2024 at 1:37

shawnt00 · Accepted Answer · 2024-06-19 02:03:19Z

1

I'm not clear why the values are being tabulated for six full years of dates. All you need to do is check that no visit exists within the prior three years for the dates that fall within the report:

select VisitDate, count(*) as NewPtCnt
from dbo.vwKeptVisits v
where VisitDate between @reportStart and @reportEnd
  and not exists (
    select 1
    from dbo.vwKeptVisits v2
    where v2.PatId = v.PatId
      and v2.VisitDate between dateadd(year, -3, v.VisitDate) and dateadd(day, -1, v.VisitDate)
)
group by VisitDate;

You should use a date type rather than smalldatetime. Adjust the ends of the ranges as necessary for the values being stored.

edited Jun 19, 2024 at 2:03

answered Jun 19, 2024 at 1:55

shawnt00

18.1k3 gold badges23 silver badges25 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

JohnT Over a year ago

THANK YOU SHAWNT00!! I appreciate your taking the time to delve into what I was trying to do and offering such an elegant, logical and simple solution. And you are absolutely correct regarding what needed to be done to check if a patient visit was for a "new" patient. My function contained flawed logic. One question - why do recommend date versus smalldatetime for this?

shawnt00 Over a year ago

@JohnT Mostly because I don't think the time portion is relevant and also I believe smalldatetime is deprecated.

JohnT Over a year ago

Thanks again. The time portion is indeed irrelevant. I have smalldatetime throughout my databases as I am self-taught about this stuff and was attracted by the smaller byte footprint.

shawnt00 Over a year ago

@JohnT I understand. smalldatetime isn't deprecated quite yet. I was wrong there. And I recognize changing now could be a hassle. Just to note, date actually uses only three bytes so without the time component it's more compact.

Mike G · Accepted Answer · 2024-06-19 18:35:36Z

0

Here's a way. Because of the function(s) in the where clause your query is degraded.
I always refactor such pattern into OUTER APPLY(s) and performance improves significantly. You can then filter by columns from your outer apply in the where clause.

answered Jun 19, 2024 at 18:35

Mike G

8646 silver badges12 bronze badges

1 Comment

JohnT Over a year ago

Thank you for the input MikeG. I will consider OUTER APPLY in the future. In this particular case, I was able to drop the function altogether based on ShawnT00's input. Query time is now under one second!

Collectives™ on Stack Overflow

Looking for a way to avoid a cursor

2 Answers 2

4 Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related