TSQL CODE REVIEW ME PLEASE - 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 2000 Forums

SQL Server Development (2000)

TSQL CODE REVIEW ME PLEASE

Author

Topic

jesus4u
Posting Yak Master

204 Posts

Posted - 2004-07-19 : 11:39:24

Would you mind commenting on my code below?


BEGIN TRAN

--UPDATE INVENTORY TABLE BY ADDING BACK THE QUANTITIES FROM FAILED OR PENDING ORDERS.

UPDATE PortalStore_Inventory

SET PortalStore_Inventory.Inventory = PortalStore_Inventory.Inventory + PortalStore_OrderDetails.Quantity

FROM
	PortalStore_Orders INNER JOIN
	PortalStore_OrderDetails ON PortalStore_Orders.OrderID = PortalStore_OrderDetails.OrderID
WHERE
	(PortalStore_Orders.PaymentStatus = N'failed' OR  PortalStore_Orders.PaymentStatus = N'pending') 
	AND (PortalStore_Orders.StoreID = @StoreID) 
	AND (PortalStore_Inventory.ModelNumber=PortalStore_OrderDetails.SKU_UPC)

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

--CHECK IF ANY ROWS WERE UPDATED.

SELECT @@ROWCOUNT

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

IF @@ROWCOUNT > 0

--DELETE THOSE FAILED OR PENDING ORDERS.

	DELETE FROM 
		PortalStore_Orders 
	WHERE
		(PortalStore_Orders.PaymentStatus = N'failed' OR  PortalStore_Orders.PaymentStatus = N'pending') 
		AND (PortalStore_Orders.StoreID = @StoreID) 
	
	IF @@ERROR <> 0 
		BEGIN
			ROLLBACK TRANSACTION
			RETURN
		END
COMMIT TRAN

eyechart
Master Smack Fu Yak Hacker

3575 Posts

Posted - 2004-07-19 : 12:03:58

you could use RAISEERROR with different error messages in your error checking to indicate why a transaction has rolled back. Otherwise you will have no idea why that occured.

-ec

X002548
Not Just a Number

15586 Posts

Posted - 2004-07-19 : 12:42:25

I always try for 1 BEGIN, 1 COMMIT, 1 ROLLBACK, a common exit, 1 error handling section, 1 return (no values now though) and a common messaging system...

http://weblogs.sqlteam.com/brettk/archive/2004/05/25/1378.aspx

Brett

8-)

Kristen
Test

22859 Posts

Posted - 2004-07-19 : 13:08:34

"Would you mind commenting on my code below?"

Here you go then!:


DECLARE @intErrNo int, @intRowCount int
BEGIN TRAN

--UPDATE INVENTORY TABLE BY ADDING BACK THE QUANTITIES FROM FAILED OR PENDING ORDERS.

UPDATE dbo.PortalStore_Inventory

SET PortalStore_Inventory.Inventory = PortalStore_Inventory.Inventory + PortalStore_OrderDetails.Quantity

FROM
	dbo.PortalStore_Orders INNER JOIN
	dbo.PortalStore_OrderDetails ON PortalStore_Orders.OrderID = PortalStore_OrderDetails.OrderID
WHERE
	(PortalStore_Orders.PaymentStatus = N'failed' OR  PortalStore_Orders.PaymentStatus = N'pending') 
	AND (PortalStore_Orders.StoreID = @StoreID) 
	AND (PortalStore_Inventory.ModelNumber=PortalStore_OrderDetails.SKU_UPC)
SELECT @intErrNo = @@ERROR, @intRowCount = @@ROWCOUNT
IF @@ERROR@intErrNo <> 0 
	BEGIN
		ROLLBACK TRANSACTION
		RETURN
	END

--CHECK IF ANY ROWS WERE UPDATED.

SELECT @@ROWCOUNT

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

IF @@ROWCOUNT@intRowCount > 0

--DELETE THOSE FAILED OR PENDING ORDERS.

	DELETE FROM 
		dbo.PortalStore_Orders 
	WHERE
		(PortalStore_Orders.PaymentStatus = N'failed' OR  PortalStore_Orders.PaymentStatus = N'pending') 
		AND (PortalStore_Orders.StoreID = @StoreID) 
SELECT @intErrNo = @@ERROR, @intRowCount = @@ROWCOUNT
	IF @@ERROR@intErrNo <> 0 
		BEGIN
			ROLLBACK TRANSACTION
			RETURN
		END
COMMIT TRAN

I too would use IF @intErrNo <> 0 GOTO ErrorHandler rather than individual ROLLBACK and RETURN

I would try using an IN instead of the OR (both of them) to see if it performed better.

Kristen

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-07-19 : 13:23:07

and RETURN -1 when @intErrNo <> 0. RETURN 0 after COMMIT TRAN when success.

Tara

Kristen
Test

22859 Posts

Posted - 2004-07-19 : 13:46:31

Yup (or SELECT @intErrNo = 0 at the top and jsut return that - it will be non-zero for error - i.e. last value of @@ERROR to have failed)

Oh blast, now I'm off again ... we use two variables, @intRetVal and @intErrNo. @intErrNo is a temporary holder for @@ERROR and we set @intRetVal to be the "return" value:


... DO something ...
SELECT @intErrNo = @@ERROR, @intRowCount = @@ROWCOUNT
IF @intErrNo <> 0 OR @intRowCount <> 1 -- (@intRowCount test optional/variable)
    SELECT @intRetVal = 1 -- Unique location marker within this SProc
GOTO BailOut
ENDIF

Kristen

eyechart
Master Smack Fu Yak Hacker

3575 Posts

Posted - 2004-07-19 : 13:53:13

quote:
Originally posted by tduggan

and RETURN -1 when @intErrNo <> 0. RETURN 0 after COMMIT TRAN when success.

arent' return codes -1 through -14 for microsoft use only? Here is the table I have for these default values.

-1 Object missing
-2 Data type error occured
-3 Process chosen as deadlock victim
-4 Permission error
-5 Syntax error
-6 Miscellaneous user error
-7 Resource error
-8 Nonfatal internal error
-9 System limit reached
-10 Fatal internal inconsistency error
-11 Fatal internal inconsistency error
-12 Corrupt table or index
-13 Corrupt database
-14 Hardware error

-15 through -99 are reserved for future use. Because of this, I always use positive return codes 1-99, or negative return codes < -99.

-ec

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-07-19 : 13:57:09

-1 is the standard that I have seen used by a lot of people. I adopted this standard some time ago. I wouldn't say -1 through -14 are for MS use only. It isn't true for RETURN at least.

Tara

eyechart
Master Smack Fu Yak Hacker

3575 Posts

Posted - 2004-07-19 : 14:19:53

quote:
Originally posted by tduggan

-1 is the standard that I have seen used by a lot of people. I adopted this standard some time ago. I wouldn't say -1 through -14 are for MS use only. It isn't true for RETURN at least.

From the original BOL for SQL2K this text appears under the EXECUTE command:

SQL Server currently uses return values 0 through -14 to indicate the execution status of stored procedures. Values from -15 through -99 are reserved for future use. For more information about a list of reserved return status values, see RETURN.

In later revisions, this text has been edited out. Ken Henderson has also documented this info (that is where my table came from). I wonder if this no longer applies.

-ec

Kristen
Test

22859 Posts

Posted - 2004-07-19 : 14:26:51

Its down to what checks the return code on a user-defined SProc isn't it? In my case it's my code!

Kristen

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-07-19 : 14:28:41

[EDIT] Comments referring to ec's last post, not Kristen's. Should have quoted it. [/EDIT]

But even still, I wouldn't say for MS use only. They can be used by non-MSers. I checked Ken's stored procedure book and didn't see that table listing. His book just mentions that -1 through -14 are used for various failures. Maybe I don't have the same version as you.

Tara

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-07-19 : 14:31:30

quote:
Originally posted by Kristen

Its down to what checks the return code on a user-defined SProc isn't it? In my case it's my code!

Kristen

Exactly.

Tara

eyechart
Master Smack Fu Yak Hacker

3575 Posts

Posted - 2004-07-19 : 14:32:56

quote:
Originally posted by tduggan

But even still, I wouldn't say for MS use only. They can be used by non-MSers. I checked Ken's stored procedure book and didn't see that table listing. His book just mentions that -1 through -14 are used for various failures. Maybe I don't have the same version as you.

The table is on page 322 of The Guru's Guide to Transact-SQL.

-ec

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-07-19 : 14:36:11

quote:
Originally posted by eyechart

The table is on page 322 of The Guru's Guide to Transact-SQL.

[/quote]

Got it, thanks.

Tara

Kristen
Test

22859 Posts

Posted - 2004-07-19 : 14:36:23

Haven't got to that page yet, hope I don't have to rewrite all my code!

Kristen

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-07-19 : 14:36:37

quote:
Originally posted by eyechart

The table is on page 322 of The Guru's Guide to Transact-SQL.

Got it, thanks.

Tara

eyechart
Master Smack Fu Yak Hacker

3575 Posts

Posted - 2004-07-19 : 14:37:24

quote:
Originally posted by Kristen

Its down to what checks the return code on a user-defined SProc isn't it? In my case it's my code!

I don't know the answer to this, that is why I have always avoided the return codes between -1 and -99. The text in BOL said "reserved return status values".

Now, that text is no longer in the latest edition of BOL, but it was there for years.

-ec

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-07-19 : 14:38:05

quote:
Originally posted by Kristen

Haven't got to that page yet, hope I don't have to rewrite all my code!

Kristen

Yeah me either. But I still believe it's just how MS is using the return codes.

Tara

Kristen
Test

22859 Posts

Posted - 2004-07-19 : 14:44:14

"But I still believe it's just how MS is using the return codes."

I've taken it to mean that I can expect [maybe!] the MS SProcs to return those sort of values.

Kristen

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-07-19 : 14:46:46

jesus4u, at least use a negative number when a failure occurs. Which negative number is up to you. Use 0 when success.

Tara

X002548
Not Just a Number

15586 Posts

Posted - 2004-07-19 : 15:33:34

I already thought we had this discussion...and you shouldn't pass anything out, mostly because it could get overridden...use an OUTPUT variable

And if you code for -1 in a calling sproc you could be hosed....

Here's the thread

http://www.sqlteam.com/forums/topic.asp?TOPIC_ID=35642

Brett

8-)

Next Page

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources