July 23, 2003 at 7:06 am
Would someone be able to talk me through this stored proc. especially starting from the bit beginning DECLARE LinkID_cursor CURSOR...
thanks Mic.
/****** Object: Stored Procedure dbo.advertisers_AddListing Script Date: 2/2/2003 10:13:21 PM ******/
CREATE PROCEDURE advertisers_AddListing
@LinkTitle as varchar(255),
@LinkDescription as varchar(255),
@Keyword as varchar(255),
@AdvertiserID as int,
@SubCategoryID as int,
@LinkURL as varchar(255),
@price as money,
@Return as int output
AS
Declare @Exists as int
Set @Exists = (Select LinkID From Links Where AdvertiserID = @AdvertiserID and Keyword = @Keyword)
If NOT @Exists IS NULL
Begin
Set @Return = 1
End
Else
Begin
Declare @TopPrice as money
INSERT INTO Links
(LinkTitle, LinkDescription, Keyword, AdvertiserID, SubCategoryID, LinkURL, Status, Price, Rank, TopPrice)
VALUES
(@LinkTitle, @LinkDescription, @Keyword, @AdvertiserID, @SubCategoryID, @LinkURL, 1, @price, 1, 0.0)
Set @TopPrice = (Select Top 1 Price From Links Where Keyword = @Keyword order By Price DESC)
Update Links
Set TopPrice = @TopPrice
Where Keyword = @Keyword
DECLARE LinkID_cursor CURSOR
FOR
SELECT LinkID
FROM Links Where Status = 1 AND Keyword = @Keyword order by Price DESC
OPEN LinkID_cursor
DECLARE @TheLinkID sysname
DECLARE @@Counter as int
Set @@Counter = 1
FETCH NEXT FROM LinkID_cursor INTO @TheLinkID
WHILE (@@FETCH_STATUS <> -1)
BEGIN
IF (@@FETCH_STATUS <> -2)
BEGIN
SELECT @TheLinkID = RTRIM(@TheLinkID)
--EXEC ('SELECT ''' + @LinkID + ''' = LinkID FROM Links WHERE Keyword = ''server'' order by Price')
EXEC ('Update Links Set Rank = ''' + @@Counter + ''' WHERE LinkID = ''' + @TheLinkID + '''')
Set @@Counter = @@Counter + 1
END
FETCH NEXT FROM LinkID_cursor INTO @TheLinkID
END
CLOSE LinkID_cursor
DEALLOCATE LinkID_cursor
Set @Return = 0
Return
End
GO
July 23, 2003 at 7:38 am
I am torn between explaining what this procedure is doing, and explaining just how bad it is. That said, the part in question (starting with the DECLARE...CURSOR bit, is looping through a resultset of active links ordered descending by their price. It then, through a(n unnecessary) dynamic SQL statement, is updating the rank field in the links table for each link to the incrementing counter variable (@@counter) that represents its price ranking (with number 1 being the highest priced link).
--
To be honest, this procedure deserves to be completely rewritten. The author needs to get familiar with a) the EXISTS clause , b) how NOT to use cursors, c) how to use RETURN values, and d) how NOT to use dynamic SQL to do a simple UPDATE statement...
--
here's a proposed revision (and I'm sure you'll hear more ideas...):
CREATE PROCEDURE advertisers_AddListing
@LinkTitle as varchar(255),
, @LinkDescription as varchar(255),
, @Keyword as varchar(255),
, @AdvertiserID as int,
, @SubCategoryID as int,
, @LinkURL as varchar(255),
, @Price as money
AS
-- Exit if we find a similar link for this advertiser
IF EXISTS
(
SELECT LinkID FROM Links WHERE AdvertiserID = @AdvertiserID AND Keyword = @Keyword
) BEGIN
RETURN 1
END
-- Otherwise, update rankings
--
-- Insert the new listing
--
INSERT INTO Links
(LinkTitle, LinkDescription, Keyword, AdvertiserID, SubCategoryID, LinkURL, Status, Price, Rank, TopPrice)
VALUES
(@LinkTitle, @LinkDescription, @Keyword, @AdvertiserID, @SubCategoryID, @LinkURL, 1, @Price, 1, 0.0)
--
-- update the top price calculated field
-- for each link in this keyword grouping
DECLARE @TopPrice MONEY
--
SET @TopPrice = (SELECT MAX(Price) FROM Links WHERE Keyword = @Keyword ORDER BY Price DESC)
--
UPDATE Links
SET TopPrice = @TopPrice
WHERE Keyword = @Keyword
--
-- Build a temp table for the links, with prices descending
-- include an identity field which will serve as the rank
--
CREATE TABLE #Prices (RANK INT IDENTITY(1,1) NOT NULL, LinkID INT NOT NULL , Price MONEY NOT NULL)
INSERT INTO #Prices (LinkID , Price )
SELECT LinkID , Price
FROM Links
WHERE Status = 1
AND Keyword = @Keyword
ORDER BY Price DESC
--
-- Update the Links table rank with the identity of the
-- Price temp table, which represents the order of price
--
UPDATE l SET l.Rank = p.Rank
FROM Links l
INNER JOIN #Prices p ON l.LinkID = p.LinkID
--
DROP TABLE #Prices
--
RETURN 0
GO
The last thing you'd want to do is add some real error-handling and wrap it in a transaction...
--
HTH,
Jay
Edited by - jpipes on 07/23/2003 08:32:18 AM
Viewing 2 posts - 1 through 2 (of 2 total)
You must be logged in to reply to this topic. Login to reply