January 17, 2014 at 12:24 pm
I can't understand what is happening inside the cursor when you set a variable to null: SET @strSellerNo2 = NULL. I am more confused because of the select statement immediately following: SELECT @strSellerNo2 = Seller_No. To me it looks like @strSellerNo1= SellerNo = @strSellerNo2.
USE [OutletRetail]
GO
/****** Object: StoredProcedure [Outlet].[sp_UpdateProductStatus] Script Date: 01/16/2014 19:58:47 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER OFF
GO
--Updates Product status codes to Available if NULL
ALTER PROCEDURE [Outlet].[sp_UpdateProductStatus]
AS
DECLARE @strProductNo varchar(20)
DECLARE @strSellerNo1 varchar(10)
DECLARE @strSellerNo2 varchar(10)
DECLARE UpdateProductCursor CURSOR FOR
SELECT Product_No, Seller_No
FROM Outlet.tblProductMaster
WHERE Product_Status IS NULL
OPEN UpdateProductCursor
FETCH NEXT FROM UpdateProductCursor INTO @strProduct_No, @strSellerNo1
WHILE @@FETCH_STATUS = 0
BEGIN
SET @strSellerNo2 = NULL
SELECT @strSellerNo2 = Seller_No
FROM Outlet.tblProductMaster
WHERE Product_No = @strProductNo
AND Seller_No <> @strSellerNo1
AND Product_Status = 'Available'
IF (@strSellerNo2 IS NULL)
BEGIN
UPDATE Outlet.tblProductMaster
SET Product_Status = 'Available'
WHERE Product_No = @strProductNo
AND Seller_No = @strSellerNo1
END
January 17, 2014 at 12:52 pm
To use a cursor, you basically need to use a while loop.
If you're assigning values to your variables inside the loopp using select statements, you might want to reinitialize your variables. That's because if no rows are returned from the select statement, the variable won't change its value.
To be clear, here's an example.
DECLARE @db int
--Initialize the variable
SET @db = 1
--Check the value
SELECT @db
--Try to assign value to a variable with a select that won't return rows
SELECT @db = object_id
FROM tempdb.sys.objects
WHERE 1 = 2
ORDER BY object_id
--Check the value
SELECT @db
--Assign value to a variable with a select that returns rows
SELECT @db = object_id
FROM tempdb.sys.objects
ORDER BY object_id
--Check the value
SELECT @db
In another set of ideas, your cursor might be causing slow performance and if you post the complete code with ddl and sample data for the tables involved, we can help you to change it to a solution that will outperform your current method.
January 17, 2014 at 1:20 pm
Thank you. I understand it somewhat more now. There are 2 more parts that confuse me.
What is the relationship between SET @strSellerNo2 = NULL and IF (@strSellerNo2 IS NULL).
Also how does Seller_No <> @strSellerNo1 when it seems that it was fetched as Seller_No=SellerNo1. I commented out the parts that confused me.
DECLARE @strProductNo varchar(20)
--DECLARE @strSellerNo1 varchar(10)
DECLARE @strSellerNo2 varchar(10)
DECLARE UpdateProductCursor CURSOR FOR
--SELECT Product_No, Seller_No
FROM Outlet.tblProductMaster
WHERE Product_Status IS NULL
OPEN UpdateProductCursor
--FETCH NEXT FROM UpdateProductCursor INTO @strProduct_No, @strSellerNo1
WHILE @@FETCH_STATUS = 0
BEGIN
SET @strSellerNo2 = NULL
SELECT @strSellerNo2 = Seller_No
FROM Outlet.tblProductMaster
WHERE Product_No = @strProductNo
--AND Seller_No <> @strSellerNo1
AND Product_Status = 'Available'
January 17, 2014 at 1:46 pm
I'll try to explain using your original code.
USE [OutletRetail]
GO
/****** Object: StoredProcedure [Outlet].[sp_UpdateProductStatus] Script Date: 01/16/2014 19:58:47 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER OFF
GO
--Updates Product status codes to Available if NULL
ALTER PROCEDURE [Outlet].[sp_UpdateProductStatus]
AS
DECLARE @strProductNo varchar(20)
--You declare 2 SellerNo.
--The first one is to fetch from the cursor
DECLARE @strSellerNo1 varchar(10)
--and the second one is to check for a different value for SellerNo
DECLARE @strSellerNo2 varchar(10)
DECLARE UpdateProductCursor CURSOR FOR
--This is your cursor definition, if you run this query, you'll know the values that will be used.
--Be aware that cursors defined with the default options are dynamic and affected by updates to the data.
SELECT Product_No, Seller_No
FROM Outlet.tblProductMaster
WHERE Product_Status IS NULL
OPEN UpdateProductCursor
--This retrieves the first values of the cursor.
--At the end of your while cycle, you'll have the same statement to navigate through the cursor.
FETCH NEXT FROM UpdateProductCursor INTO @strProduct_No, @strSellerNo1
WHILE @@FETCH_STATUS = 0
BEGIN
SET @strSellerNo2 = NULL
SELECT @strSellerNo2 = Seller_No
FROM Outlet.tblProductMaster
WHERE Product_No = @strProductNo
--Here, you're looking for rows with the same product_No but different Seller_No
--If there's more than one value for Seller_No for the same product, the variable will have a value assigned
--If not, it will stay NULL
AND Seller_No <> @strSellerNo1
AND Product_Status = 'Available'
--This is a validation to run the update only if the product has a single SellerNo.
IF (@strSellerNo2 IS NULL)
BEGIN
UPDATE Outlet.tblProductMaster
SET Product_Status = 'Available'
WHERE Product_No = @strProductNo
AND Seller_No = @strSellerNo1
END
I would still recommend that you post the complete details to improve the code and get rid of this cursor.
January 17, 2014 at 2:03 pm
This is an example of a replacement for all the code posted to use the cursor.
UPDATE t SET
Product_Status = 'Available'
FROM Outlet.tblProductMaster t
WHERE Product_Status IS NULL
AND NOT EXISTS( SELECT *
FROM Outlet.tblProductMaster x
WHERE x.Product_No = t.Product_No
AND x.Seller_No <> t.Seller_No
AND x.Product_Status = 'Available' )
Less than a third of code lines (and I'm not counting blank lines) and should perform a lot faster.
January 17, 2014 at 2:16 pm
Thanks so much for the help and clear explanation. I've read that cursors should be avoided. However, this procedure is actually connected to a larger DTS package which would be too much data to post. This is the complete code for this procedure.
I am expected to add another possibility for a "replacement" product so the status can be "replacement", "available", "not available". Not sure how I will write that.
USE [OutletRetail]
GO
/****** Object: StoredProcedure [Outlet].[sp_UpdateProductStatus] Script Date: 01/16/2014 19:58:47 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER OFF
GO
--Updates Product status codes to A if NULL
ALTER PROCEDURE [Outlet].[sp_UpdateProductStatus]
AS
DECLARE @strProductNo varchar(20)
DECLARE @strSellerNo1 varchar(10)
DECLARE @strSellerNo2 varchar(10)
DECLARE UpdateProductCursor CURSOR FOR
SELECT Product_No, Seller_No
FROM Outlet.tblProductMaster
WHERE Product_Status IS NULL
OPEN UpdateProductCursor
FETCH NEXT FROM UpdateProductCursor INTO @strProduct_No, @strSellerNo1
WHILE @@FETCH_STATUS = 0
BEGIN
SET @strSellerNo2 = NULL
SELECT @strSellerNo2 = Seller_No
FROM Outlet.tblProductMaster
WHERE Product_No = @strProductNo
AND Seller_No <> @strSellerNo1
AND Product_Status = 'Available'
IF (@strSellerNo2 IS NULL)
BEGIN
UPDATE Outlet.tblProductMaster
SET Product_Status = 'Available'
WHERE Product_No = @strProductNo
AND Seller_No = @strSellerNo1
END
UPDATEOutlet.tblProductMaster
SETProduct_Status = 'Not Available'
WHEREProduct_No = @strProductNo
AND Seller_No <> @strSellerNo2
AND Product_Status IS NULL
FETCH NEXT FROM UpdateProductCursor INTO @strProductNo, @strSellerNo1
END
CLOSE UpdateProductCursor
DEALLOCATE UpdateProductCursor
January 17, 2014 at 2:27 pm
I don't have the rules for the 'Replacement' option, but here's the code that should work for the query as you have it now.
UPDATE t SET
Product_Status = CASE WHEN EXISTS( SELECT *
FROM #tblProductMaster x
WHERE x.Product_No = t.Product_No
AND x.Seller_No <> t.Seller_No
AND x.Product_Status = 'Available' ) THEN 'Not Available'
ELSE 'Available' END
FROM #tblProductMaster t
WHERE Product_Status IS NULL
Be sure to test it before using it in production. My tests were limited to sample data that I created and might not reflect the actual data in your database.
By the way, I hope you can start migrating DTS packages to DTSX packages (aka SSIS packages). You'll have more functionality and won't have major problems if you plan on migrating to future versions of SQL Server (DTS packages won't work on Sql Server 2012).
January 17, 2014 at 3:01 pm
Thanks so much for the help and education Luis.
January 17, 2014 at 3:05 pm
And remember the first step towards the paradigm shift of writing Set Based code:
"Stop thinking about what you want to do to a row... think, instead, of what you want to do to a column."
Viewing 9 posts - 1 through 8 (of 8 total)
You must be logged in to reply to this topic. Login to reply