Optimizing Update query - SQL Server Forums

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2008 Forums

Transact-SQL (2008)

Optimizing Update query

Author

Topic

alasse130
Starting Member

21 Posts

Posted - 2011-07-04 : 09:38:12

Can anyone help me on how to optimize this update statement?

DECLARE @BatchID INT
DECLARE @importid INT
DECLARE @isnew BIT
SET @ObjTypeID = 1
SET @BatchID = 1
SET @importid = 1
WHILE EXISTS (SELECT ImportID FROM Import
WHERE BatchID = @BatchID and ToUpdate = 1 AND ErrorMsg IS NULL)
BEGIN
SELECT @importid = MIN(ImportID)
FROM Import
WHERE BatchID = @BatchID and ToUpdate = 1 AND errorMsg IS NULL
SELECT @isnew = CASE WHEN ID ='new' THEN 1 ELSE 0 END
FROM Import
WHERE ImportID = @importid
BEGIN TRY
IF (@isnew = 0)
BEGIN
UPDATE Appointment
SET
InScope = CASE (Imp.InScope) WHEN 'FALSE' THEN 0 ELSE 1 END,
ScheduledDate = Imp.ScheduledDate,
PrimaryContact_ID = Imp.PrimaryContactId,
Plan_ID = CASE WHEN Imp.InScope = 'TRUE' THEN Imp.PlanId ELSE Plan_ID END,
FROM Import Imp
INNER JOIN Appointment A
ON Imp.AppointmentID = A.ID
WHERE Imp.ImportID =@importid
END
END TRY
BEGIN CATCH
UPDATE Import SET ErrorMsg = ERROR_MESSAGE()
WHERE ImportID = @importid
END CATCH
END

There are other field which I'm updating which I just didn't include here. Performance is very slow even when there are only 10 rows from the Import table which are marked as ToUpdate. Other info:

Import.ImportID - Primary Key
Import.BatchID - the same for rows which are imported at the same time

Please help me on this.

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-07-04 : 09:56:06

Performance is slow because you are doing the import 1 row at a time.

Why are you doing this?

What CATCH can possibly happen? Is it validation of dates etc from string types?

If it is then you need to do that first -- validate your Import before trying to update the Appointment table. Once you've validated the import fully then just do a simple UPDATE on every row of the import -- if you want to batch it then split your import into batches (I see there is allreay a batchID of 50 / 100 / whatever is performant) then update all the batches.

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION

alasse130
Starting Member

21 Posts

Posted - 2011-07-04 : 11:03:52

thanks for your reply!

Actually, the rows are already imported by the time this gets executed. The Import table has already been validated and only those which doesn't have any value in its ErrorMsg column is being updated. The Appointment table is what actually is being updated here based on what is in the Import table. I just added the catch just to be sure in case I missed validating anything.

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-07-04 : 11:15:40

yeah but look at what you are doing:


WHILE EXISTS (
	SELECT ImportID FROM Import 
	WHERE BatchID = @BatchID AND ToUpdate = 1 AND ErrorMsg IS NULL
	)
BEGIN
	SELECT @importid = MIN(ImportID) 
	FROM Import 
	WHERE BatchID = @BatchID AND ToUpdate = 1 AND errorMsg IS NULL

	SELECT @isnew = CASE WHEN ID ='new' THEN 1 ELSE 0 END
	FROM Import 
	WHERE ImportID = @importid

	BEGIN TRY
		IF (@isnew = 0) 
		BEGIN
			UPDATE Appointment
			SET
				InScope = CASE (Imp.InScope) WHEN 'FALSE' THEN 0 ELSE 1 END,
				ScheduledDate = Imp.ScheduledDate, 
				PrimaryContact_ID = Imp.PrimaryContactId, 
				Plan_ID = CASE WHEN Imp.InScope = 'TRUE' THEN Imp.PlanId ELSE Plan_ID END, 
			FROM
				Import Imp
				INNER JOIN Appointment A ON Imp.AppointmentID = A.ID 
			WHERE
				Imp.ImportID = @importid
		END 
	END TRY
	BEGIN CATCH
		UPDATE Import SET ErrorMsg = ERROR_MESSAGE()
		WHERE ImportID = @importid
	END CATCH
END

So:

1) You have a while loop that checks that there are rows in import with no message, and a batch ID
2) You loop over that set (checking each time which possible is a scan?)

for each iteration of the loop you:
2.1) Fetch the MINIMUM ID from Import where no message and batch id -- performing a scan and aggregate
2.2) For THAT ROW ONLY you work out if it is new or not (hopefully a seek...)
2.3) you start a try catch block
2.3.1) If it was new then you update Appointment (for that SINGLE ROW in import
2.3.2) If it wasn't new then you do nothing
2.4) Catch an error -- update Import if there was an error.

..... Hang on -- what happens if there was no catch? you don't do anything to the imported row so you will probably pick it up again at the top of your loop -- it has been updated but there is no error message so it will be picked up again. I think the same value will be picked up the 2nd, 3rd, nth time round the loop.

What error are you hoping to trap?

Don't do this one at a time. I can't see any reason to do it this way.

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION

alasse130
Starting Member

21 Posts

Posted - 2011-07-04 : 11:22:44

Ah, I see your point. So I could do this?

UPDATE Appointment
SET
InScope = CASE (Imp.InScope) WHEN 'FALSE' THEN 0 ELSE 1 END,
ScheduledDate = Imp.ScheduledDate,
PrimaryContact_ID = Imp.PrimaryContactId,
Plan_ID = CASE WHEN Imp.InScope = 'TRUE' THEN Imp.PlanId ELSE Plan_ID END,
FROM Import Imp
INNER JOIN Appointment A
ON Imp.AppointmentID = A.ID
WHERE Imp.ImportID =@importid
END
END TRY
BEGIN CATCH
UPDATE Import SET ErrorMsg = ERROR_MESSAGE()
WHERE BatchID = @BatchID
and ErrorMsg is null

Is this correct?

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-07-04 : 11:31:48

well you could -- that would update -- every member of the batch you were trying to process with the error message if something went wrong. (is that what you wanted)?

Or did you want to know *which* row as causing an error (like trying to stuff 59 characters into a 58 length string for example)?

But it would certainly be faster!

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION

alasse130
Starting Member

21 Posts

Posted - 2011-07-04 : 11:47:05

Yes, that's what I want :)

I'll try it out. Thanks a lot!

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-07-04 : 11:50:48

ah -- your update statement should probably be this:


UPDATE a
SET
	InScope = CASE (Imp.InScope) WHEN 'FALSE' THEN 0 ELSE 1 END
	, ScheduledDate = Imp.ScheduledDate
	, PrimaryContact_ID = Imp.PrimaryContactId
	, Plan_ID = CASE WHEN Imp.InScope = 'TRUE' THEN Imp.PlanId ELSE Plan_ID END
FROM
	Import AS Imp
	INNER JOIN Appointment AS a ON Imp.AppointmentID = a.ID
WHERE
	Imp.[BatchID] = @batchID
	AND Imp.[ToUpdate] = 1

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION

alasse130
Starting Member

21 Posts

Posted - 2011-07-04 : 12:02:59

Thanks. Do you have any other suggestion how to improve the performance? Execution is much faster now but still not in the accepted amount of time for the users to wait. Should I make ImportID a primary key? I tried both where it is and it isn't but I didn't notice much difference.

Jason W
Starting Member

19 Posts

Posted - 2011-07-04 : 20:47:30

You can check your execution plan to see what is causing the slowness when the query runs. Here is some more info on the execution plan and how it works:
- http://www.sqloptimizationsschool.com/Pages/Basic%20Concepts/Execution%20Plans.aspx

Are tehre a lot of rows in the Import and Appointment tables? If so, it might help to index the AppointmentID and ID columns on each table. Here is some info on indexing:
- http://www.sqloptimizationsschool.com/Pages/Basic%20Concepts/Indexes.aspx

Jason
Webmaster at www.sqloptimizationsschool.com

Transact Charlie
Master Smack Fu Yak Hacker

3451 Posts

Posted - 2011-07-05 : 04:14:25

quote:
Originally posted by alasse130

Thanks. Do you have any other suggestion how to improve the performance? Execution is much faster now but still not in the accepted amount of time for the users to wait. Should I make ImportID a primary key? I tried both where it is and it isn't but I didn't notice much difference.

Depends on the relative sizes of the tables involved.

If the Import table is large then you want an index on *at least* AppointmentID (because it forms the join criteria to Appointment.

You also want an index on the Appointment.ID field (Which I'm guessing is the primary key and probably the CLUSTERED INDEX already)?

For this exact query:


UPDATE a
SET
	InScope = CASE (Imp.InScope) WHEN 'FALSE' THEN 0 ELSE 1 END
	, ScheduledDate = Imp.ScheduledDate
	, PrimaryContact_ID = Imp.PrimaryContactId
	, Plan_ID = CASE WHEN Imp.InScope = 'TRUE' THEN Imp.PlanId ELSE Plan_ID END
FROM
	Import AS Imp
	INNER JOIN Appointment AS a ON Imp.AppointmentID = a.ID
WHERE
	Imp.[BatchID] = @batchID
	AND Imp.[ToUpdate] = 1

Then what fields are being searched are on Import (and probably in order of importance) are:
AppointmentID
BatchID
ToUpdate

Therefore (for this *exact*) query the index I would suggest as the most important would be:


CREATE NONCLUSTERED INDEX IX_Import_Update_Appointment ON Import (
AppointmentID
, BatchID
, ToUpdate
)

IF you wanted to not perform a Key lookup after the index is searched then you could INCLUDE the columns from Import used in the SELECT portion also.

But you need to check the execution plan -- also post the *exact* code you are using now.

Charlie
===============================================================
Msg 3903, Level 16, State 1, Line 1736
The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION

alasse130
Starting Member

21 Posts

Posted - 2011-07-05 : 05:08:51

Thanks Jason, Charlie for all your help!

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources