Simple logic but not working as it should

Question

This is what i want to achieve:

So trigger fires on opportunities table when a record with opp_type = 0 is inserted.

The next part of the code just does the calculation which is to pick up the last used number from your custom table and add 1 to it. Stores new value in a variable.

The next part is to do the insert into the user field. Finally update the custom table to record the last used number.

I am getting the number to increment by one in the NEXTEXP1 table however the user field called O_Quote is not populating via the GUI.

is the code below doing what it should in terms of the explanation above?

by the steps in my trigger it seems the same but the user field is not populating with last number used:

alter TRIGGER [dbo].[Q2] ON  [dbo].[AMGR_opportunity_Tbl] AFTER INSERT
AS 
BEGIN

Declare @Opp_Type int

Select @Opp_Type = 0 from inserted

If @Opp_Type = 0

  BEGIN

    SET NOCOUNT ON;

    DECLARE @Client_Id varchar(24)
    DECLARE @Contact_Number int
    DECLARE @NewNumber varchar(250)
    DECLARE @NextQNo float
    DECLARE @UDFName varchar(50)
    DECLARE @GeneratorPrefix varchar(10)
    DECLARE @GeneratorLength float
    DECLARE @Opptype int
    DECLARE @Type_id int
    DECLARE @Oppid varchar (24)


    --select top 1 nextqno = nextqno from nextexp1
    SELECT @NewNumber = NextQno + 1 from dbo.NextEXP1

    ----insert into user field
    insert into O_Quote(Client_Id, Contact_Number, Type_Id, Code_Id, [O_Quote])
    values (@Client_Id,0,15,0,@NextQNo)

    -------update table with last number used
UPDATE [dbo].[NextEXP1] SET NextQNo = @NewNumber


    End

    End

GO

Answer 1

Looking into this, I think your logic is a bit off:

Select @Opp_Type = 0 from inserted

This will always evaluate as 0.

You want to use:

SELECT  @Opp_Type   =   i.Opp_Type 
FROM    inserted    AS  i;

Where i.Opp_Type is your column name.

Secondly, you declare a bunch of variables, but never set them:

    DECLARE @Client_Id varchar(24)
    DECLARE @Contact_Number int
    DECLARE @NewNumber varchar(250)
    DECLARE @NextQNo float
    DECLARE @UDFName varchar(50)
    DECLARE @GeneratorPrefix varchar(10)
    DECLARE @GeneratorLength float
    DECLARE @Opptype int
    DECLARE @Type_id int
    DECLARE @Oppid varchar (24)

And then go on to insert them. You need to set these, if you want to use them later. Should this be:

insert into O_Quote(Client_Id, Contact_Number, Type_Id, Code_Id, [O_Quote])
values (@Client_Id,0,15,0,@NewNumber)

Or do you need to set @NextQNo to be:

SELECT @NextQNo   = NextQno from dbo.NextEXP1;
SELECT @NewNumber = @NextQNo + 1;

And lastly, the way this is written will cause you issues if you insert more than one record at a time. You would need to think about a loop, to get that MaxID, which isn't ideal. Can you look at using IDENTITY columns instead?

Answer 2

@Leonidas199x is right with all points. I can also say that there are too many things that are unclear with that questions and lot's of the data is missing, however this is what I can suggest (this code handles bulk inserts also):

alter TRIGGER [dbo].[Q2] ON  [dbo].[AMGR_opportunity_Tbl] AFTER INSERT
AS 
BEGIN
   DECLARE @NewNumber varchar(250);
   SELECT @NewNumber = MAX(NextQno) FROM dbo.NextEXP1; -- I guess that's what you want

    insert into O_Quote(Client_Id, Contact_Number, Type_Id, Code_Id, [O_Quote])
    select Client_Id, Contact_Number, Type_Id, Code_Id, @NewNumber + row_num
    FROM (
            SELECT Client_Id, -- once again do not know where this value is taken from
                  0 Contact_Number,
                  15 Type_Id,
                   0 AS Code_Id,
            ROW_NUMBER() OVER(order by client_id) row_num
            FROM INSERTED WHERE Opp_Type = 0 --I guess that's the right column name
    ) a;

    SELECT @NewNumber = MAX(O_Quote) FROM O_Quote;

    UPDATE [dbo].[NextEXP1] SET NextQNo = @NewNumber;
END