Need help with a very simple stored procedure

Hello,

I am using SQL Server 2012. I tried to write a stored procedure for generating unique values to be used on barcodes. However, I really do not know much about stored procedures and I could not even create one using below SQL

create table barcodecounter(
  prefix varchar(6) not null,
  value int,
  constraint pk_barcodecounter primary key(prefix)
);

create procedure spCounter
  @param varchar(6)
as
  declare @counter int;

  begin transaction
  
  update barcodecounter
  set value = value + 1 
  where prefix = @param;

  commit transaction
  
  set @counter = (select value from barcodecounter where prefix = @param);
  select @param + right('00000000' + convert(varchar, @counter), 8) as 'count';
end

Error I am receiving is:

Msg 102, Level 15, State 1, Procedure spCounter, Line 16
Incorrect syntax near 'end'.

What I observe is that I get red squiggles under table name and column names in my update and select statements.

Any help is appreciated.

Thanks & Regards,
Ertan

All good things come to an end, but they need to begin

create procedure spCounter
  @param varchar(6)
as
begin --- <<< this
  declare @counter int;

  begin transaction
  
  update barcodecounter
  set value = value + 1 
  where prefix = @param;

  commit transaction
  
  set @counter = (select value from barcodecounter where prefix = @param);
  select @param + right('00000000' + convert(varchar, @counter), 8) as 'count';
end
1 Like

Yes, sloppy of me.

BTW, I just think about a situation that "what if prefix not exists?"

What would be a suggested way for such a situation? I came up with something as following, but not sure if that is the way it should be

create procedure spCounter
  @param varchar(6)
as
begin
  declare @counter int;

  begin transaction
  
  if not exists (select value from barcodecounter where prefix = @param)
  begin
    insert into barcodecounter(prefix, value) values(@param, 1)
  end
  else
  begin
    update barcodecounter
    set value = value + 1 
    where prefix = @param;
  end

  commit transaction
  
  set @counter = (select value from barcodecounter where prefix = @param);
  select @param + right('00000000' + convert(varchar, @counter), 8) as 'count';
end

Thanks & Regards,
Ertan

Both your SPs have concurrency problems.
With @param you are attempting to put at least 9 characters into varchar(6). Use a different variable.
When using varchar etc ALWAYS declare its length. This will save you a lot of pain.
I would be inclined to use an OUTPUT parameter instead of SELECTing the result.
Try something like:

SET ANSI_NULLS, QUOTED_IDENTIFIER ON;
GO
CREATE PROCEDURE dbo.spCounter
(
  @param varchar(6)
  ,@Result varchar(14) OUTPUT
)
AS

SET NOCOUNT, XACT_ABORT ON;

DECLARE @tResult TABLE
(
	Result varchar(8) NOT NULL
);

UPDATE dbo.barcodecounter
SET [value] = [value] + 1
OUTPUT RIGHT('00000000' + CAST(inserted.[value] AS varchar(8)), 8)
	INTO @tResult
WHERE prefix = @param;

IF @@ROWCOUNT = 0
BEGIN
	INSERT INTO dbo.barcodecounter
	OUTPUT RIGHT('00000000' + CAST(inserted.[value] AS varchar(8)), 8)
              INTO @tResult
	SELECT @param, 1
	WHERE NOT EXISTS
	(
		SELECT 1
		FROM dbo.barcodecounter WITH (UPDLOCK, SERIALIZABLE)
		WHERE Prefix = @param
			AND [value] = 1
	);

	IF @@ROWCOUNT = 0
	-- Another process could have inserted 1
	BEGIN
		UPDATE dbo.barcodecounter
		SET [value] = [value] + 1
		OUTPUT RIGHT('00000000' + CAST(inserted.[value] AS varchar(8)), 8)
			INTO @tResult
		WHERE prefix = @param;
	END
END

SET @Result = (SELECT @param + Result FROM @tResult);
GO
2 Likes

Thanks for advises and updated procedure.

Depending on what you are doing you may be better to use sequences:

Generated barcodes will be inserted in another master (header) table and a detail table.
Generally speaking, all generated values will be used. Meaning, has to be existed on that header table.
Customer would like to have cross check data at hand.
Upon request, we can count barcodes in header table for a particular week and compare them with the value in barcodecounter table.

This was the decision taken and I am trying to make it possible.

I am not totally convinced but the following shows the procedure seems to work.

SET ANSI_NULLS, QUOTED_IDENTIFIER, ANSI_PADDING ON;
GO
USE tempdb;
GO
-- Test Table
CREATE TABLE dbo.barcodecounter
(
	prefix varchar(6) NOT NULL
		CONSTRAINT PK_barcodecounter PRIMARY KEY
	,LastValue int NOT NULL
);
GO

-- Test Procedure
SET ANSI_NULLS, QUOTED_IDENTIFIER ON;
GO
CREATE PROCEDURE dbo.GetNextBarcode
(
  @prefix varchar(6)
  ,@NextBarCode varchar(14) OUTPUT
)
AS
SET NOCOUNT, XACT_ABORT ON;

DECLARE @tResult TABLE (Result int NOT NULL);

UPDATE dbo.barcodecounter
SET LastValue = LastValue + 1
OUTPUT inserted.LastValue INTO @tResult
WHERE prefix = @prefix;

IF @@ROWCOUNT = 0
BEGIN
	INSERT INTO dbo.barcodecounter
	OUTPUT inserted.LastValue INTO @tResult
	SELECT @prefix, 1
	WHERE NOT EXISTS
	(
		SELECT 1
		FROM dbo.barcodecounter WITH (UPDLOCK, SERIALIZABLE)
		WHERE Prefix = @prefix
			AND LastValue = 1
	);

	IF @@ROWCOUNT = 0
	-- Another process could have inserted 1
	BEGIN
		UPDATE dbo.barcodecounter
		SET LastValue = LastValue + 1
		OUTPUT inserted.LastValue INTO @tResult
		WHERE prefix = @prefix;
	END
END
SET @NextBarCode = (SELECT @prefix + RIGHT('00000000' + CAST(Result AS varchar(8)), 8) FROM @tResult);
GO

-- Simple Test
truncate table dbo.barcodecounter;

DECLARE @NextCode varchar(14);
EXEC dbo.GetNextBarcode 'Test1', @NextCode OUTPUT;
print @NextCode;
EXEC dbo.GetNextBarcode 'Test2', @NextCode OUTPUT;
print @NextCode;
EXEC dbo.GetNextBarcode 'Test3', @NextCode OUTPUT;
print @NextCode;
EXEC dbo.GetNextBarcode 'Test1', @NextCode OUTPUT;
print @NextCode;
EXEC dbo.GetNextBarcode 'Test2', @NextCode OUTPUT;
print @NextCode;
GO

-- *** Basic concurrency test ***

--create dbo.fnTally from:
-- https://www.sqlservercentral.com/scripts/create-a-tally-function-fntally

truncate table dbo.barcodecounter;

CREATE TABLE ##Results1
(
	Barcode varchar(14) NOT NULL
);

CREATE TABLE ##Results2
(
	Barcode varchar(14) NOT NULL
);

/*
Now copy the following:

TRUNCATE TABLE ##Results2;
DECLARE @NextCode varchar(14);

and the results of:

WITH Prefixes
AS
(
	SELECT *
	FROM
	(
		VALUES ('Test1'),('Test2'),('Test3'),('Test4'),('Test5'),('Test6'),('Test7')
	) V (Prefix)
)
SELECT 'EXEC dbo.GetNextBarcode ''' + P.Prefix + ''', @NextCode OUTPUT;' +
	'INSERT INTO ##Results2 SELECT @NextCode;'
FROM Prefixes P
	CROSS JOIN dbo.fnTally(1, 1000) T

to another window. (Window2)

Then copy:

WAITFOR DELAY '00:00:01'; 
TRUNCATE TABLE ##Results1;
DECLARE @NextCode varchar(14);

and the results of:

WITH Prefixes
AS
(
	SELECT *
	FROM
	(
		VALUES ('Test1'),('Test2'),('Test3'),('Test4'),('Test5'),('Test6'),('Test7')
	) V (Prefix)
)
SELECT 'EXEC dbo.GetNextBarcode ''' + P.Prefix + ''', @NextCode OUTPUT;' +
	'INSERT INTO ##Results1 SELECT @NextCode;'
FROM Prefixes P
	CROSS JOIN dbo.fnTally(1, 1000) T

to yet another Window (Window1)

Start running Window1 and a second later start running window2

*/

-- When Window1 and Window2 have finished running
-- the following should return an empty set. ie no race condition.

WITH AllResults
AS
(
	SELECT Barcode FROM ##Results1
	UNION ALL
	SELECT Barcode FROM ##Results2
)
SELECT Barcode
FROM AllResults
GROUP BY Barcode
HAVING COUNT(*) > 1;


select * from ##Results1
select * from ##Results2;

-- Now close Window1 and Window2 and

--Tidy up
DROP TABLE IF EXISTS ##Results1;
DROP TABLE IF EXISTS ##Results2;
DROP FUNCTION IF EXISTS dbo.fnTally;
DROP PROCEDURE IF EXISTS dbo.GetNextBarcode;
DROP TABLE IF EXISTS dbo.barcodecounter;