SQL Server: INSERT trigger failing for one of two columns of composite PK.

If you're wanting to manually set the value, why use an identity column ?

Why not just use an integer column?  Wouldn't that solve the issue you're having ?

I agree. Identity columns are for columns you do not want to care about. The identity value will get incremented by each new record - there is no relation between SERIAL and the identity.

Hi,

This is not a good solution. The proper way to do it would be if you had two tables, one as a master and the other as a detail.

In the first one you would keep your [SERIAL] which would be autoincrement, and the second one will have the [PROFILE] which will be autoincrement  and the [SERIAL] from the first table.

This is the best way to distinct all profiles for each customer.

Let me know if anything.

Regards,
Igor

Hello all

Thank you for the comments. The thing with handling the identity column is that in some business cases, some records are deleted. Then the identity continues from where it was leaving gaps.  Example:

Records in table: 123-0, 123-1, 123-2, 123-3, 123-4 and 123-5. You delete 123-2. The next identity you get (the two numbers are two different fields, together are PK) is 123-5 leaving a 'gap' for 123-2

The objective of the trigger is not to allow this to happen so is trying to control that the second field (identity) does not have gaps.

Additional explanation on id gaps:

You can experiment with this SQL, which, on an identity column will give you the FIRST AVAILABLE number and not the NEXT number which is the case here:

SELECT MIN(t1.ID)+1 FROM MY_TABLE t1
     LEFT JOIN MY_TABLE t2
     ON t1.ID = t2.ID-1
WHERE t2.ID IS NULL

Select allOpen in new window

(MY_TABLE being your table and ID is a normal identity column, say, seed 1 increment 1)

If you follow this SQL, for example if you put in MY_TABLE some records for ID identity column, 1,2,3,4 and 5, normally the column will offer you number 6 as next ID (which is normal and acceptable). Now delete record No 4 and try again: identity (again as expected) is offering number 7 which is correct. But now you have a gap, your records in MY_TABLE are 1,2,3,5,6,7.
If you run the SQL immediately above, you will notice that it returns No 4, and furthermore, if you do enter a record with ID = 4 without deleting anything, then it will continue with number 8, 9, 10... etc as  a normal identity column would.

So, that is the purpose of this question, how to offer the FIRST available number and not the NEXT available number, for an identity column that, is part of a composite key and not alone in the table.


Thank you for reading this, and a big thanks for your efforts,

Nick

Additional reply to experts comments:

Dear sjwales, Qlemo
Thank you for your reply.

The problem is that with the identity column if a record is deleted the column continues from the last number and in my case, for business logic reasons, this cannot happen. Check my comment above (ID: 38718104) for an elaborate explanation.



Dear igordevelop
Thank you for your reply.

Actually the idea of two tables crossed my mind, but unfortunately there are already too many tables alread dependent on the PK ([SERIAL],[PROFILE]) on the one hand, and on the other the problem is still that a [PROFILE] number may be ommitted resulting in a gap that unfortunately cannot be accepted.
So unfortunately I must keep this table as one, having primary key the [SERIAL],[PROFILE] pair of fields (both integer data types) and have a trigger that would offer the next available [PROFILE] number without continuing from the last one.


Thank you all very much

Comment twoards a solution:

I put the trigger which is shown in the question on the top, in a table with just a single identity column. After renaming the table name and column name, it was accepted. Maybe the problem is that the trigger now refers to an identity column that is part of a composite key with another identity column?

I tried to rewrite the trigger this time including the [SERIAL] column but I have got the same error message again.

Just letting you know. Thanks.

Didn't think in depth about it, but wouldn't it be better to just set the [PROFILE] to a default value of .e.g. 999 (without idenity), and let the trigger do its magic?

Dear Qlemo
Thank you for your reply.

Unfortunately can't do this - the [SERIAL] column together with the [PROFILE] are PK. The business logic behind it (bit simplified) is that a customer has a number and a contact profile - the contact profile is the table where PK is the pair [SERIAL] (FK to CUSTOMER) and [PROFILE] (FK to another table). So when, for example, customer No 123 has two services, requests communication of the first service to be directed to contact profile No 1 (SERIAL=123 PROFILE=1 description=HOME) and communication for a second service to directed to contact profile No 2 (SERIAL=123 PROFILE=2 description=WORK)

Thanks again

Mechanism,

I ran through the below test, which seems to do exactly as you are describing, without using an identity

-- Create test data
use demo
go
create table test1 (Serial int, profile tinyint);
go
insert into test1 values(123, 0);
insert into test1 values(123, 1);
insert into test1 values(123, 2);
insert into test1 values(123, 3);
insert into test1 values(123, 4);

-- create gap in the code
delete from test1 where serial = 123 and profile = 2

-- create trigger

CREATE TRIGGER [dbo].[CUSTOMER_CONTACT_STD_Insert_PROFILE] 
   ON  [dbo].[test1] 
   FOR INSERT
AS 
BEGIN
	-- SET NOCOUNT ON added to prevent extra result sets from
	-- interfering with SELECT statements.
	SET NOCOUNT ON;

    UPDATE dbo.test1
	SET PROFILE = (
					SELECT MIN(t1.PROFILE)+1 
					FROM test1 t1
						LEFT JOIN test1 t2 
						ON t1.PROFILE = t2.PROFILE-1
					WHERE t2.PROFILE IS NULL
				   )
	FROM dbo.test1 INNER JOIN INSERTED ins
	ON test1.PROFILE = ins.PROFILE
	
END

-- Check existing data, there is a gap in row #2

select * from test1                                            

Serial      profile
----------- -------
123         0
123         1
123         3
123         4

(4 row(s) affected)

-- new insert comes along, using #5, Trigger should fire to update the gap

insert into test1 values (123, 5);

-- Check data after insert, the 5 is now a 2

select * from test1               

Serial      profile
----------- -------
123         0
123         1
123         2
123         3
123         4

(5 row(s) affected)

Select allOpen in new window

However, it appears to me that it might be easier to modify the insert  statement, rather than have a trigger ?  I am guessing that the insert probably can use the most recent value so you don't have to work out what your next value is at insert time, but if the trigger is going to do that work anyway, why not just be up front and do it during the insert ?

Let's assume we have the same base table and the data is in this state:

DROP TRIGGER [dbo].[CUSTOMER_CONTACT_STD_Insert_PROFILE] ;
go
-- create gap in the code
delete from test1 where serial = 123 and profile = 2
go

select * from test1                                            

Serial      profile
----------- -------
123         0
123         1
123         3
123         4

(4 row(s) affected)

-- Do the insert, fill the gap during the insert, not afterwards in a trigger

declare @prof tinyint;
SELECT @prof = MIN(t1.PROFILE)+1 
FROM test1 t1
	LEFT JOIN test1 t2 
ON t1.PROFILE = t2.PROFILE-1
WHERE t2.PROFILE IS NULL;

insert into test1 values( 123, @prof)
go

select * from test1;

Serial      profile
----------- -------
123         0
123         1
123         2
123         3
123         4

(5 row(s) affected)

Select allOpen in new window

It would need some tweaking for inserting the first profile for a serial (because if there's no data there it returns NULL), but the main point here is get around how to have sequential profile numbers.

Would that not work ?

The PROFILE column will never do what you want as an IDENTITY column.  The nature of IDENTITY columns is to leave gaps.

Given your requirements, I think you really require an INSTEAD OF INSERT trigger.

The INSTEAD OF trigger could take a logical lock on the SERIAL key values, then assign the proper PROFILE values prior to INSERT.  That reduces the overhead dramatically, since you don't have to DELETE and reINSERT all the rows.

A standard AFTER DELETE trigger could compress any gaps created by a DELETE(s) of an intermediate PROFILE value.

Dear sjwales
Thank you for your reply.

I thought quite alot on your proposal and sounds interesting and reasonable. Indeed, I do not care much where the control will be, so if it is on the INSERT statement, that would be fine with me. As of pure luck, I supposed to run on the same 'gap' problem with the [SERIAL] column, but luckily I am not, because the [SERIAL] column is being given by the CUSTOMER table and I do not have to worry about gaps (I treat gaps on the CUSTOMER table). My problem is just the [PROFILE] column.

Sugestion: if you could provide me (1) a stored procedure XXX with argument the [SERIAL] field(EXEC sp_XXX yyy) and (2) making sure that the stored procedure returns no NULL (the [PROFILE] field has seed 0 and incr. 1 so the minimum value would be 0, for example if it's the first profile ever created for that [SERIAL]) I would accept as a good standing solution.


Dear ScottPletcher
Thank you for your reply.

True, the IDENTITY column never gives.. promises of not leaving gaps, it's rather a business requirement than a db best practice. As with the previous expert, I have no problem either with an INSTEAD OF INSERT trigger but I didn't quite get the logic with the AFTER DELETE and the INSTEAD OF trigger together. Would that be ok if I ask for an example?

(Code temporarily removed found bug after posting!)

Dear ScottPletcher - Dear sjwales

As it is, the [SERIAL] number will be known, and the question is to return the first available [profile] number. So, what seems to facilitate the situation is to remove the IDENTITY feature from [SERIAL] and set it to the column that passes the value to this table. In this case, the column can be without the identity set.

[For ScottPletcher]
If I follow the ScottPletcher approach, does this mean that I can remove the IDENTITY set from [PROFILE] as well and stil be sure that I will have the no-gaps approach when inserting and deleting records?

[For all]
I modified the SQL and this seem to return the minimum profile for a given [SERIAL], I thought this would help

SELECT t1.serial s, MIN(t1.profile)+1 p FROM [CUSTOMER_CONTACT_STD] t1
LEFT JOIN [CUSTOMER_CONTACT_STD] t2 
ON (t1.profile = t2.profile-1 AND t1.serial = t2.serial)
WHERE t2.profile IS NULL 
--AND t1.serial = whatever number is the user trying to insert, assumption is that it's known
GROUP BY t1.serial

Select allOpen in new window

An INSTEAD INSERT could help (no AFTER DELETE is needed if I understand correctly this approach, since every time the INSTEAD INSERT trigger will be filling up the missing profile numbers)

Thank you in advance for your efforts

Here's the code I came up with that seems to work:

drop procedure myinsert
go
create procedure myinsert (@serial int)
as
  set nocount on
  declare @prof tinyint;
  SELECT @prof = (SELECT MIN(t1.PROFILE)+1 
                  FROM test1 t1
	                  LEFT JOIN test1 t2 
                  ON t1.PROFILE = t2.PROFILE-1 and t1.SERIAL = t2.SERIAL
                  WHERE t2.PROFILE IS NULL)
  FROM test1 
  WHERE SERIAL = @Serial;
  
if @prof is NULL set @prof = 0;

insert into test1 values( @serial, @prof)
go


truncate table test1;

exec myinsert 123

select * from test1;

Serial      profile
----------- -------
123         0

(1 row(s) affected)

exec myinsert 123

select * from test1;

Serial      profile
----------- -------
123         0
123         1

(2 row(s) affected)


exec myinsert 124
exec myinsert 124
select * from test1;

Serial      profile
----------- -------
123         0
123         1
124         0
124         1

(4 row(s) affected)

exec myinsert 125
exec myinsert 125
exec myinsert 125
select * from test1;

Serial      profile
----------- -------
123         0
123         1
124         0
124         1
125         0
125         1
125         2

(7 row(s) affected)

Select allOpen in new window

Dear sjwales
Thank you for your reply.

I tried to replicate but seems I'm doing something wrong (your SQL seems ok but maybe it's something on my side).

This is the table I use:

SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

SET ANSI_PADDING ON
GO

CREATE TABLE [dbo].[test1](
	[serial] [int] NOT NULL,
	[profile] [tinyint] NOT NULL,
	[name] [varchar](50) NULL,
	[surname] [varchar](100) NULL,
 CONSTRAINT [PK_customer] PRIMARY KEY CLUSTERED 
(
	[serial] ASC,
	[profile] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

GO

SET ANSI_PADDING OFF
GO

Select allOpen in new window

This is what I am executing:

create procedure myinsert (@serial int)
as
  set nocount on
  declare @prof tinyint;
  SELECT @prof = (SELECT MIN(t1.PROFILE)+1 
                  FROM test1 t1
	                  LEFT JOIN test1 t2 
                  ON t1.PROFILE = t2.PROFILE-1 and t1.SERIAL = t2.SERIAL
                  WHERE t2.PROFILE IS NULL)
  FROM test1 
  WHERE SERIAL = @Serial;
  
if @prof is NULL set @prof = 0;

insert into test1 values( @serial, @prof)
go

Select allOpen in new window

This is the error message:
Msg 213, Level 16, State 1, Procedure myinsert, Line 15
Column name or number of supplied values does not match table definition.

Can you lend a hand?

I only provided the outline / skeleton of the code that should work.

You will need to fill out the rest of the values in your table:

insert into test1 values (@serial, @prof .... and then values for name & surname.

You could pass them as parameters when you call the proc:

create procedure myinsert (@serial int, @name varchar(50), @surname varchar(100)
as

Select allOpen in new window

and then change the insert statement to:

insert into test1 values( @serial, @prof, @name, @surname)

Select allOpen in new window

What about multiple INSERTs at once, as the initial q stated/implied?

What about DELETEs from the table?

Since the core of the code is the original poster's code, it should find the gap and fill it after a delete.    For multiple inserts, call the proc once per insert required.

Without knowing the details of the enveloping processing, can really only generalizes solution

Deal sjwales
Thank you for your reply.

I am testing your proposed solution as per comment [38719418]

True, as [ScottPletcher] said, there could be considerations on multiple INSERTS. Any advice on this?

Nevertheless however, the question straight from the beginning asked for an INSERT on two columns both identity.
I will change the SQL as per sjwales comment [38719418] and see if this works.


Dear ScottPletcher
Thank you for your reply. You think an INSTEAD OF INSERT trigger could do the job?

Thanks all, I will be posting a code based on [sjwales] comment.

Thank you

Dear sjwales
Thank you for your reply.

I use this simple table for testing:

SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

SET ANSI_PADDING ON
GO

CREATE TABLE [dbo].[test1](
	[serial] [int] NOT NULL,
	[profile] [tinyint] NOT NULL,
	[name] [varchar](50) NULL,
	[surname] [varchar](100) NULL,
 CONSTRAINT [PK_customer] PRIMARY KEY CLUSTERED 
(
	[serial] ASC,
	[profile] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

GO

SET ANSI_PADDING OFF
GO

Select allOpen in new window

I manually entered six values so that we have a set ready:

serial	profile	name	surname
1	0	nick	mar
2	0	john	johnson
2	1	john	johnson
2	3	john	johnson
3	0	jack	daniels
3	2	jack	daniels

Select allOpen in new window

and this is the modified stored procedure:

CREATE PROCEDURE test1_INSERT (@serial int, @name varchar(50), @surname varchar(100))
AS
  SET NOCOUNT ON
  DECLARE @profile tinyint;
  SELECT @profile = (SELECT MIN(t1.PROFILE)+1 
                  FROM test1 t1
	                  LEFT JOIN test1 t2 
                  ON t1.PROFILE = t2.PROFILE-1 and t1.SERIAL = t2.SERIAL
                  WHERE t2.PROFILE IS NULL)
 FROM test1 
  WHERE SERIAL = @Serial;
  
IF @profile is NULL set @profile = 0;

INSERT INTO test1 values( @serial, @profile, @name, @surname)
GO

Select allOpen in new window

Examining the values for serial No 2, I have [serial, profile] keys 2.0, 2.1 and 2.3 so I made a test for inserting 2.2:
EXEC test1_INSERT 2, 'John', 'Johnson'

but I got error indicating that the profile passed was 1 (ie 2.1) instead of the correct 2.2 (the 'gap' one):

Msg 2627, Level 14, State 1, Procedure test1_INSERT, Line 17
Violation of PRIMARY KEY constraint 'PK_customer'. Cannot insert duplicate key in object 'dbo.test1'. The duplicate key value is (2, 1).
The statement has been terminated.

Can you help me correct the SELECT in parenthesis?

Dear sjwales
Thank you for your reply.

Indeed, it worked perfectly, thank you for sharing.

A a matter of fact, I even added my log columns at the end (inputing functions, like user_name() etc) and I made the two columns IDENTITY (both SERIAL and PROFILE). With a small edit in the INSERT and a "SET IDENTITY_INSERT test1 ON" and "OFF" enclosing the INSERT, it worked very nicely.
I don't know about concurrent inputs and deletes, I think I will have to postpone this on a second stage.
I am also a bit more confident now that this should work fine with UPDATE and DELETE as well, give or take some edit.


Dear ScottPletcher

It would be nice to see and compare an INSTEAD OF INSERT trigger based on the above; as of now, sjwales answer (ID: 38722170) is the closest possible to a viable solution.

Dear all
Thank you for your replies.
If you have any final comments, kindly provide them.

Dear sjwales
Thank you for your contribution, seems that there is no clear-cut way other than the one we worked on. I accept your working solution as final.

Dear all
Thank you for your time and contributions.

It is really a good solution. The stored procedure is placed instead of trigger (an INSTEAD OF trigger was discussed but not suggested by other members) and it works as is for non-IDENTITY columns.
Implementation suggestion: YES (modify only for IDENTITY if required)