Is this right? or there a better way....

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 2005 Forums

Transact-SQL (2005)

Is this right? or there a better way....

Author

Topic

doran_doran
Posting Yak Master

179 Posts

Posted - 2009-11-06 : 12:27:52

USE [SHARPIE]
GO
/****** Object: StoredProcedure [dbo].[updateSharpieData] Script Date: 11/06/2009 10:30:51 ******/
-- THIS STORED PROC PERFORMS INSERT AND UPDATE. FIRST IT CREATES SAME RECORDS WITH ACTIVE=1, NEXT IT UPDATES
-- OLD RECORDS'S ACTIVE STATUS TO FALSE
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

CREATE PROCEDURE [dbo].[updateSharpieData]
(
@User varchar(MAX),
@Date datetime,
@Comment varchar(MAX),
@ID INT,
@SharpieID INT,
@RAMType VARCHAR(50),
@RAM1 FLOAT,
@RAM1Error FLOAT,
@RAM2 FLOAT,
@RAM2Error FLOAT,
@RAM3 FLOAT,
@RAM3Error FLOAT,
@RAM4 FLOAT,
@RAM4Error FLOAT,
@RAM5 FLOAT,
@RAM5Error FLOAT,
@RAM6 FLOAT,
@RAM6Error FLOAT
)

AS
-- START THE TRANSACTION
DECLARE @IDSCOPE int

BEGIN TRY

BEGIN TRANSACTION

INSERT INTO SharpieDATA
(SharpieID, RAMType, RAM1, RAM1Error, RAM2, RAM2Error, RAM3,
RAM3Error, RAM4, RAM4Error, RAM5, RAM5Error, RAM6, RAM6Error, Active)
VALUES
(@SharpieID, @RAMType, @RAM1, @RAM1Error, @RAM2, @RAM2Error, @RAM3,
@RAM3Error, @RAM4, @RAM4Error, @RAM5, @RAM5Error, @RAM6, @RAM6Error, '1')
SET @IDSCOPE = SCOPE_IDENTITY()
END TRY

BEGIN CATCH
Insert into Logs
(
UserName,
LogDateTime,
TableName,
UserAction,
Comment
)
Values
(
@User,
@Date,
'SharpieDATA',
'Add',
'Attempt Failed'
)
RETURN 1
END CATCH

Insert into Logs
(
UserName,
LogDateTime,
TableName,
RowID,
UserAction,
Comment
)

Values
(
@User,
@Date,
'SharpieDATA',
@IDSCOPE,
'Add',
'Attempt Succeeded'
)

-- Rollback the transaction if there were any errors
IF @@ERROR <>0
BEGIN
ROLLBACK -- ROLLBACK THE TRANSACTION
RETURN 0
END

-- UPDATING PREVIOUS RECORD STATUS FROM ACTIVE TO INACTIVE
BEGIN TRY

UPDATE SharpieDATA
SET [active] = '0'
WHERE ID = @ID

IF @@ERROR <>0
BEGIN
ROLLBACK -- ROLLBACK THE TRANSACTION
RETURN 0
END

END TRY

BEGIN CATCH
Insert into Logs
(
UserName,
LogDateTime,
TableName,
UserAction,
Comment
)
Values
(
@User,
@Date,
'SharpieDATA',
'Update',
'Attempt Failed'
)
RETURN 1
END CATCH

Insert into Logs
(
UserName,
LogDateTime,
TableName,
RowID,
UserAction,
Comment
)

Values
(
@User,
@Date,
'SharpieDATA',
@IDSCOPE,
'Update',
'Attempt Succeeded'
)

COMMIT

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2009-11-06 : 12:28:52

What is your question?

Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog

"Let's begin with the premise that everything you've done up until this point is wrong."

doran_doran
Posting Yak Master

179 Posts

Posted - 2009-11-06 : 12:36:39

I mean is this the best way to achieve the goal? Could there be a better way?

Lamprey
Master Smack Fu Yak Hacker

4614 Posts

Posted - 2009-11-06 : 12:42:22

I would say no it doesn't look right. Your TRY-CATCH blocks are, probably, the main issue. You should look up how to do those properly.

doran_doran
Posting Yak Master

179 Posts

Posted - 2009-11-06 : 12:56:41

okay, i fixed the try catch block and now it's working. My real question was would this be the best way to do it (i mean code wise)?

Lamprey
Master Smack Fu Yak Hacker

4614 Posts

Posted - 2009-11-06 : 14:21:27

Did you also change your Transactions and such? They way you had it structured you could log something and then roll that logged event back out. Generally, I'd say you should Update then Insert, but since your working with a specific value you code is fine for the Insert and Updating.

doran_doran
Posting Yak Master

179 Posts

Posted - 2009-11-06 : 15:37:51

I updated the code (look @ the initial post). Can anyone suggest this is the best way to approach the business logic? (where the original record is marked with active=0 and similar record is create with active=1)

rohitkumar
Constraint Violating Yak Guru

472 Posts

Posted - 2009-11-06 : 16:00:12

[code]
UPDATE sharpiedata
SET [active] = '0'
WHERE id = @ID
[/code]

wont this update all the records (both old and NEW) inactive??
I would have used OUTPUT to get the new ID and updated old records by

...WHERE key = @key and ID <> @newID

doran_doran
Posting Yak Master

179 Posts

Posted - 2009-11-06 : 16:04:33

old ID is being supplied by the user (look in the parameter). Actually, I am looking for suggestion or Quality control to make sure my transaction is in right place, begin try and end try are all in the right place.

rohitkumar
Constraint Violating Yak Guru

472 Posts

Posted - 2009-11-06 : 16:10:52

I would still update with ID <> @newID, just to make sure all the earlier records are marked as inactive, i.e. there's only one active SharpieID

doran_doran
Posting Yak Master

179 Posts

Posted - 2009-11-06 : 16:16:09

rohit,

you are right. I have already made that adjustment. Can you go through the initially posted code and see if transaction, begin, try catch are all in right place.

UPDATE Sharpiedata
SET [active] = '0'
WHERE (id = @ID)
AND (ID <> @IDSCOPE)

rohitkumar
Constraint Violating Yak Guru

472 Posts

Posted - 2009-11-06 : 16:19:35

[code]UPDATE rammissiondata
SET [active] = '0'
WHERE ID <> @IDSCOPE
[/code]

WHERE id = xxx and id <> yyyy means nothing.

doran_doran
Posting Yak Master

179 Posts

Posted - 2009-11-06 : 16:23:27

okay. update statement is now looking good. did you check the rest of the code?

X002548
Not Just a Number

15586 Posts

Posted - 2009-11-06 : 16:53:43

quote:
Originally posted by doran_doran

did you check the rest of the code?

I can't I poked my eyes out

But before I did

1. What's with all the float datatypes
2. Your table is denormalized with all of those RAMnError Columns. Normalize your data
3. You should never supply a value for RETURN
4. A sproc should be simple fall through logic

1 BEGIN TRAN
1 COMMIT TRAN
1 ROLLBACK TRAN
1 RETURN
1 RAISE

Now I have a question for everyone

Does anyone see SIGNIFICANT Value of BEGIN TRY as compared to traditional error handling....lost seems the concept of user encountered (programming) errors...ah rowcount=0...everything must be ok

Brett

8-)

Hint: Want your questions answered fast? Follow the direction in this link
http://weblogs.sqlteam.com/brettk/archive/2005/05/25/5276.aspx

Add yourself!
http://www.frappr.com/sqlteam

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2009-11-07 : 14:01:12

I love TRY/CATCH over the old error handling. For instance you can actually catch a deadlock and retry whereas with the old way you couldn't.

Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog

"Let's begin with the premise that everything you've done up until this point is wrong."

X002548
Not Just a Number

15586 Posts

Posted - 2009-11-07 : 18:02:06

OK, gotta go back and look into it some more

You didn't re-engineer all error handling though, right?

cAN YOU POST A SAMPLE OF THAT TRY/CATCH DEADLOCK CODE?

Brett

8-)

Hint: Want your questions answered fast? Follow the direction in this link
http://weblogs.sqlteam.com/brettk/archive/2005/05/25/5276.aspx

Add yourself!
http://www.frappr.com/sqlteam

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2009-11-07 : 22:00:14

Here's an exmaple:


	BEGIN TRY
		BEGIN TRAN

		<DELETE statement>

		SELECT 
			@rowCount = @@ROWCOUNT,
			@totalRowsPurged = @totalRowsPurged + @rowCount, 
			@beginRow = @endRow + 1, 
			@endRow = @endRow + @batch,
			@retries = 0
			
		COMMIT TRAN
	END TRY
	BEGIN CATCH
		IF @@TRANCOUNT > 0
			ROLLBACK TRAN
			
		IF ERROR_NUMBER() = 1205 AND @retries < @maxRetries -- 1205 is deadlock error
		BEGIN
			SET @retries = @retries + 1
			WAITFOR DELAY '00:00:10'
		END
		ELSE -- some other error or done retrying
		BEGIN
			SELECT
				@errorMessage = ERROR_MESSAGE(),
				@errorSeverity = ERROR_SEVERITY(),
				@errorState = ERROR_STATE();
			
			SET @purgeSuccess = 0
			
			RAISERROR (@errorMessage, @errorSeverity, @errorState);
			
			RETURN
		END
	END CATCH

Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog

"Let's begin with the premise that everything you've done up until this point is wrong."

doran_doran
Posting Yak Master

179 Posts

Posted - 2009-11-09 : 10:08:59

Thank you TARA. This is exactly what I was looking for. More of a solid example. I can take this and build my solution around. How about this? Does this look good now? Thank you TARA for the sample code.

USE [SHARPIE]
GO
SET ansi_nulls ON
GO
SET quoted_identifier ON
GO

ALTER PROCEDURE [dbo].[UpdateSharpiedata](
@User VARCHAR(MAX),
@Date DATETIME,
@Comment VARCHAR(MAX),
@ID INT,
@SharpieID INT,
@RAMType VARCHAR(50),
@RAM1 FLOAT,
@RAM1Error FLOAT,
@RAM2 FLOAT,
@RAM2Error FLOAT,
@RAM3 FLOAT,
@RAM3Error FLOAT,
@RAM4 FLOAT,
@RAM4Error FLOAT,
@RAM5 FLOAT,
@RAM5Error FLOAT,
@RAM6 FLOAT,
@RAM6Error FLOAT)
AS
-- START THE TRANSACTION
DECLARE @IDSCOPE INT

BEGIN TRY
BEGIN TRANSACTION

INSERT INTO Sharpiedata
(Sharpieid,
ramtype,
ram1,
ram1error,
ram2,
ram2error,
ram3,
ram3error,
ram4,
ram4error,
ram5,
ram5error,
ram6,
ram6error,
active)
VALUES (@SharpieID,
@RAMType,
@RAM1,
@RAM1Error,
@RAM2,
@RAM2Error,
@RAM3,
@RAM3Error,
@RAM4,
@RAM4Error,
@RAM5,
@RAM5Error,
@RAM6,
@RAM6Error,
'1')

SET @IDSCOPE = Scope_identity()
END TRY

BEGIN CATCH
INSERT INTO logs
(username,
logdatetime,
tablename,
useraction,
COMMENT)
VALUES (@User,
@Date,
'SharpieDATA',
'Add',
'Attempt Failed')

RETURN 1
END CATCH

INSERT INTO logs
(username,
logdatetime,
tablename,
rowid,
useraction,
COMMENT)
VALUES (@User,
@Date,
'SharpieDATA',
@IDSCOPE,
'Add',
'Attempt Succeeded')

-- Rollback the transaction if there were any errors
IF @@ERROR <> 0
BEGIN
ROLLBACK -- ROLLBACK THE TRANSACTION

RETURN 0
END

-- UPDATING PREVIOUS RECORD STATUS FROM ACTIVE TO INACTIVE
BEGIN TRY
UPDATE Sharpiedata
SET [active] = '0'
WHERE (id = @ID) --AND (@ID <> @IDSCOPE)

IF @@ERROR <> 0
BEGIN
ROLLBACK -- ROLLBACK THE TRANSACTION

RETURN 0
END
END TRY

BEGIN CATCH
INSERT INTO logs
(username,
logdatetime,
tablename,
useraction,
COMMENT)
VALUES (@User,
@Date,
'SharpieDATA',
'Update',
'Attempt Failed')

RETURN 1
END CATCH

INSERT INTO logs
(username,
logdatetime,
tablename,
rowid,
useraction,
COMMENT)
VALUES (@User,
@Date,
'SharpieDATA',
@IDSCOPE,
'Update',
'Attempt Succeeded')

COMMIT

X002548
Not Just a Number

15586 Posts

Posted - 2009-11-09 : 10:50:10

quote:

Originally posted by tkizer


	BEGIN TRY
		BEGIN TRAN

		<DELETE statement>

		SELECT 
			@rowCount = @@ROWCOUNT,
			@totalRowsPurged = @totalRowsPurged + @rowCount, 
			@beginRow = @endRow + 1, 
			@endRow = @endRow + @batch,
			@retries = 0
			
		COMMIT TRAN
	END TRY
	BEGIN CATCH
		IF @@TRANCOUNT > 0
			ROLLBACK TRAN
			
		IF ERROR_NUMBER() = 1205 AND @retries < @maxRetries -- 1205 is deadlock error
		BEGIN
			SET @retries = @retries + 1
			WAITFOR DELAY '00:00:10'
		END
		ELSE -- some other error or done retrying
		BEGIN
			SELECT
				@errorMessage = ERROR_MESSAGE(),
				@errorSeverity = ERROR_SEVERITY(),
				@errorState = ERROR_STATE();
			
			SET @purgeSuccess = 0
			
			RAISERROR (@errorMessage, @errorSeverity, @errorState);
			
			RETURN
		END
	END CATCH

I'll have to try it...but how does it retry the execution with this?

And TRANCOUNT is going to be 1 isn't it?

Or does it skip the catch if there is no error

If it doesn't and hits the catch, trancount is 1...does it loop in the catch? How does it know to go back?

I'm confused

OK, so I'll open a book

Brett

8-)

Hint: Want your questions answered fast? Follow the direction in this link
http://weblogs.sqlteam.com/brettk/archive/2005/05/25/5276.aspx

Add yourself!
http://www.frappr.com/sqlteam

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2009-11-09 : 10:58:40

It doesn't loop, but it also doesn't bomb out like with a normal deadlock. That's the main purpose of TRY/CATCH or at least from my perspective. The looping part is the portion of code I didn't post. It's just a WHILE loop to purge data.

Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog

"Let's begin with the premise that everything you've done up until this point is wrong."

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources