Cleanup My Logic

[mholbert]

I am working against a mssql 2005 db. I have largely tought myself sql, and as you will probably notice from the script below, I have lots to learn.

The following logic collects encounters that need to be deleted, and then uses a fetch statement to accomplish the deletions. It works, but it is pretty crude. I am curious to see how those of you far more skilled than I would rewrite this into a more logical and efficient script.

Thanks in advance for assistance.

/*collect all drug study encounters for deletion*/

/*collect all appointments that resulted in 'drug study' encounters*/
select a.appt_id,a.enc_id,a.appt_date
into #DR
from appointments a
inner join appointment_members am on am.appt_id = a.appt_id
where a.event_id = '12928339-AA0D-4A56-8F26-972517A237E4'
and a.enc_id is not null

/*Count the charges for each of the encounters above*/
select count(c.charge_id) as Count,#DR.enc_id
into #e
from charges c
right outer join #DR on #DR.enc_id = c.source_id
group by #DR.enc_id

/*collect together all the stuff on the encounters without any charges*/
select pe.enc_nbr,#E.enc_id,#DR.appt_id,#DR.appt_date,ep.payer_id
into #Delete
from #E
inner join patient_encounter pe on pe.enc_id = #E.enc_id
left outer join encounter_payer ep on ep.enc_id = pe.enc_id
inner join #DR on #DR.enc_id = pe.enc_id
where #E.count = 0

/*fetch, fetch, fetch*/
declare @enc_nbr char(8),
@enc_id uniqueidentifier,
@appt_id uniqueidentifier,
@appt_date char(8),
@payer_id uniqueidentifier

declare DeleteDrugStudy CURSOR for
select #Delete.enc_nbr,#Delete.enc_id,#Delete.appt_id,#Delete.appt_date,#Delete.payer_id
from #Delete

Open DeleteDrugStudy

Fetch Next From DeleteDrugStudy
into @enc_nbr, @enc_id, @appt_id, @appt_date, @payer_id

While @@FETCH_STATUS = 0

Begin
/*remove entry in appointment*/
Update appointments set enc_id = null where appointments.appt_id = @appt_id
/*remove attached payers*/
if @payer_id is not null delete from encounter_payer
where encounter_payer.enc_id = @enc_id
and encounter_payer.payer_id = @payer_id
/*delete encounter itself*/
Delete from patient_encounter where patient_encounter.enc_id = @enc_id

Fetch Next From DeleteDrugStudy
into @enc_nbr, @enc_id, @appt_id, @appt_date, @payer_id
End

Close DeleteDrugStudy
Deallocate DeleteDrugStudy

Go

 

[gmmastros]

There are a couple things I would suggest.

First, your cursor is deleting from 2 tables. One is based on @enc_id and the other is @enc_Id and @payer_id. This suggests to me that there is probably a relationship between these 2 tables. As such, I would encourage you to define a foreign key constraint between these 2 tables, and then enable cascade delete. With cascade delete enabled, rows from the child table will automatically be deleted when rows from the parent table are deleted. This will allow you to remove the cursor.

Cursors should rarely be used. SQL Server is optimized for processing data in a set based manner. Whenever you start looping over rows (either with a cursor or while loop) your performance will suffer tremendously.

After that, I would try to create a single query that can perform the deletes. Temp tables are cool, and they are sometimes required, but if you can re-write the query without them, your performance will improve.

If you don't already know about the HAVING clause, I encourage you to do a little research on it.

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[mholbert]

I cannot make any changes to the database structure perse, so i don't think i can enable cascade delete, but i will definately look into using Having instead of temp tables.

thx.

MH

 

[gmmastros]

If you can't enable cascade delete, then you may need a temp table anyway because there are multiple steps involved in the cursor.

You can still remove the cursor though.

There are 3 steps in your cursor.

1. Update appointments set enc_id = null where appointments.appt_id = @appt_id

Instead, you could do this:

Update Appointments
Set    enc_id = null
From   Appointments
       Inner Join #Delete
         On Appointments.appt_id = #Delete.appt_id

The code posted above would not need to be run inside the cursor. Basically, it's a set based update statement based on a join condition.

You can also delete from a table based on a join condition with another table. Like this:

Delete encounter_payer
From   encounter_payer
       Inner Join #Delete
         On  encounter_payer.enc_id = #Delete.enc_id
         and encounter_payer.payer_id = #Delete.payer_id

and this:

Delete patient_encounter
From   patient_encounter
       Inner Join #Delete
         on patient_encounter.enc_id = #Delete.enc_id

The complete code would look something like this:

/*collect all drug study encounters for deletion*/

/*collect all appointments that resulted in 'drug study' encounters*/
select    a.appt_id,a.enc_id,a.appt_date
into #DR
from    appointments a
inner    join appointment_members am on am.appt_id = a.appt_id
where    a.event_id = '12928339-AA0D-4A56-8F26-972517A237E4'
and    a.enc_id is not null

/*Count the charges for each of the encounters above*/
select    count(c.charge_id) as Count,#DR.enc_id
into    #e
from    charges c
right outer join #DR on #DR.enc_id = c.source_id
group by #DR.enc_id

/*collect together all the stuff on the encounters without any charges*/
select     pe.enc_nbr,#E.enc_id,#DR.appt_id,#DR.appt_date,ep.payer_id
into    #Delete
from    #E
inner    join patient_encounter pe on pe.enc_id = #E.enc_id
left outer join encounter_payer ep on ep.enc_id = pe.enc_id
inner    join #DR on #DR.enc_id = pe.enc_id
where    #E.count = 0

Update Appointments
Set    enc_id = null
From   Appointments
       Inner Join #Delete
         On Appointments.appt_id = #Delete.appt_id

Delete encounter_payer
From   encounter_payer
       Inner Join #Delete
         On  encounter_payer.enc_id = #Delete.enc_id
         and encounter_payer.payer_id = #Delete.payer_id

Delete patient_encounter
From   patient_encounter
       Inner Join #Delete
         on patient_encounter.enc_id = #Delete.enc_id

I STRONGLY encourage you to back up your database before running this code.

Then, double and triple check this code to make sure it does what you expect it to.

There's likely to be other performance gains you can get by re-writing the code. Most likely, those performance gains will pale in comparison to removing the cursor. (this assumes your tables are properly indexed.)

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom