I am using GoTo for Closing and Deallocating the Cursor. Is there a better way to write my code? Note, this code is for learning how to use Cursors and output Parameters.
I am new to TSQL, so feel free to comment on other items. For example, I was looking for a stored Procedure that will close a Cursor if it is Open but I didn't find anything via Google Search.
Thank you
-- Find the MAX, MIN, Avg for each Color in the Products table using Cursor
USE [AdventureWorks2019]
GO
/****** Object: StoredProcedure [dbo].[WithCursor] Script Date: 5/29/2024 11:30:17 AM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROC [dbo].[WithCursor]
(
@color VARCHAR(20) = NULL,
@MinStock INT OUTPUT,
@MaxStock INT OUTPUT,
@AvgStock INT OUTPUT
)
AS
SET NOCOUNT ON
BEGIN TRY
Declare @recordCount int = ( SELECT Count(*)
FROM Production.Product
WHERE (@color IS NULL AND Color IS NULL) OR (Color = @color))
-- ******************** NO Records Exist ********************
IF @recordCount = 0
Begin
DECLARE @errorMessage NVARCHAR(200);
IF @color IS NULL
SET @errorMessage = N'There are no products with the color NULL';
ELSE
SET @errorMessage = CONCAT(N'There are no products with the color ', @color);
RAISERROR(@errorMessage, 11, 1);
RETURN -2; -- no such color
End
-- ******************** Open the Cursor for Products ********************
DECLARE @level int;
IF EXISTS (
SELECT 1
FROM sys.dm_exec_cursors(0)
WHERE name = 'pCursor'
)
BEGIN
CLOSE pCursor
DEALLOCATE ProductCursor;
END
DECLARE pCursor CURSOR FOR
SELECT SafetyStockLevel
FROM Production.Product
WHERE (@color IS NULL AND Color IS NULL) OR (Color = @color);
--Open the Cursor. We know there is at least 1 record from the above check.
OPEN pCursor
FETCH NEXT FROM pCursor INTO @level
-- ******************** 1 Record Exist ********************
IF @recordCount = 1
Begin
Set @MinStock = @level
Set @MaxStock = @level
Set @AvgStock = @level
GoTo Cleanup
End
-- ******************** 2 or More Records Exist ********************
Declare @SumLevel int
Declare @MinLevel int
Declare @MaxLevel int
Set @SumLevel = @level
Set @MinLevel = @level
Set @MaxLevel = @level
--Move to the second record
FETCH NEXT FROM pCursor INTO @level
While @@FETCH_STATUS = 0
Begin
Set @SumLevel = @SumLevel + @level
If @level < @MinLevel
Set @MinLevel = @level
If @level > @MaxLevel
Set @MaxLevel = @level
FETCH NEXT FROM pCursor INTO @level
End
Set @MinStock = @Minlevel
Set @MaxStock = @Maxlevel
Set @AvgStock = @SumLevel / @recordCount
GoTo Cleanup
END TRY
BEGIN CATCH
THROW;
--Cleanup
IF EXISTS (
SELECT 1
FROM sys.dm_exec_cursors(0)
WHERE name = 'pCursor'
)
BEGIN
CLOSE pCursor
DEALLOCATE ProductCursor;
END
RETURN -1;
END CATCH;
--Success
Cleanup:
CLOSE pCursor
DEALLOCATE pCursor
Return 0
I can duplicate the code and that will remove the GoTo but I am trying not to Duplicate code as much as possible.
I refactored the code with everyone's suggestions. Thank you everyone. It was very helpful to read your feedback. Going forward I will use Local Cursors.
USE [AdventureWorks2019];
GO
/****** Object: StoredProcedure [dbo].[WithCursor] Script Date: 5/29/2024 11:30:17 AM ******/
SET ANSI_NULLS ON;
GO
SET QUOTED_IDENTIFIER ON;
GO
CREATE OR ALTER PROC [dbo].[WithCursor]
(
@color VARCHAR(20) = NULL,
@MinStock INT OUTPUT,
@MaxStock INT OUTPUT,
@AvgStock INT OUTPUT
)
AS
SET NOCOUNT ON;
BEGIN TRY
-- ******************** Open the Cursor for Products ********************
DECLARE @level INT;
--Local Cursor doesn't need to be deallocated
DECLARE @pCursor CURSOR;
SET @pCursor = CURSOR READ_ONLY FORWARD_ONLY
FOR
SELECT SafetyStockLevel
FROM Production.Product
WHERE Color = @color OR (@Color IS NULL AND Color IS NULL);
-- Open the Cursor.
OPEN @pCursor;
FETCH NEXT FROM @pCursor INTO @level;
-- ******************** Empty Recordset ********************
IF @@FETCH_STATUS <> 0
BEGIN
DECLARE @errorMessage NVARCHAR(200);
SET @errorMessage = N'There are no products with the color ' + COALESCE(@color, N'NULL');
RAISERROR(@errorMessage, 11, 1);
RETURN -2; -- no such color
END;
-- ******************** 1 or More Records Exist ********************
DECLARE @SumLevel INT = 0;
DECLARE @MinLevel INT = @level;
DECLARE @MaxLevel INT = @level;
DECLARE @recordCount INT = 0;
WHILE @@FETCH_STATUS = 0
BEGIN
SET @SumLevel = @SumLevel + @level;
IF @level < @MinLevel
SET @MinLevel = @level;
IF @level > @MaxLevel
SET @MaxLevel = @level;
FETCH NEXT FROM @pCursor INTO @level;
SET @recordCount = @recordCount + 1;
END;
--Set the Output Parameters
SET @MinStock = @MinLevel;
SET @MaxStock = @MaxLevel;
SET @AvgStock = @SumLevel / @recordCount;
END TRY
BEGIN CATCH
THROW;
RETURN -1;
END CATCH;