views:

788

answers:

2

I have this procedure which is inserting records from one table to another. the destination table is having an identity column called LeadId


Create Procedure prcInsertPrd
 As
  Begin
   Begin Transaction 
     Declare @Identity int
     Insert into Temp_ProductsArchive (column1,column2,column3)  select   
       (column1,column2,column3) 
    from Temp_Products

   if(@@Error=0)
     Begin
       Commit  
     End
   else
     Begin
       Rollback
     End
  End

Then I have written an Insert Trigger which will take LeadId from inserted table and Insert it to another table with other values as-


Alter TRIGGER trgInsertTopProducts
   ON  Temp_ProductsArchive
   AFTER INSERT
AS 
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;
     declare @LeadId as int
     Select @LeadId=LeadId from inserted 
     print @LeadId

Insert into Temp_ProductsTop(column4,LeadID,Column5,column6) Select column4,@LeadID 'LeadID',column 5,column6 from Temp_Products
END
GO

The problem is I am getting the First generated LeadiD not all the LeadIds. The LeadiD column in Temp_ProductsTop table is having only this value repeated for the number of records

A: 

"inserted" is a pseudotable containing a row for each item inserted. The trigger only fires once total, not once per entry inserted!

You'll probably need to use a cursor to loop over the records in inserted, processing them one at a time. Perhaps:

Alter TRIGGER trgInsertTopProducts
  ON Temp_ProductsArchive
  AFTER INSERT
AS
BEGIN
  -- SET NOCOUNT ON added to prevent extra result sets from
  -- interfering with SELECT statements.
  SET NOCOUNT ON;
  declare @LeadId as int
  declare c CURSOR READ_ONLY for
    select LeadId from inserted

  open c
  fetch next from c into @LeadId
  while @@FETCH_STATUS = 0
  BEGIN
    Insert into Temp_ProductsTop(ProductID,LeadID,ProductName,SupplierID,CategoryID,QuantityPerUnit,UnitPrice,UnitsInStock,UnitsOnOrder,ReorderLevel,Discontinued) Select ProductID,@LeadID 'LeadID',ProductName,SupplierID,CategoryID,QuantityPerUnit,UnitPrice,UnitsInStock,UnitsOnOrder,ReorderLevel,Discontinued from Temp_Products
    fetch next from c into @LeadId
  END
  close c
  deallocate c
END

(please note, I just typed this in here and it hasn't been syntax checked or tested)

rejj
advising people to use cursors when a set based solution is usual and appropriate, is not good advice IMO.
Mitch Wheat
You can do this set based by doing insert into.... select * from inserted will be many time faster, triggers should be as fast as possible
SQLMenace
+2  A: 

You have a major problem with your trigger. When you write a trigger you must assume that it might be called with multiple rows. i.e the inserted table might contain more than one row, which means you have to join to the inserted table.

I'm not sure what you are trying to achieve in your code, but you would need to rewrite something like this:

   Alter TRIGGER trgInsertTopProducts  
    ON  Temp_ProductsArchive 
    AFTER INSERT AS
    BEGIN    
      SET NOCOUNT ON;    
      insert into Temp_ProductsTop 
        (ProductID,LeadID,ProductName,SupplierID,CategoryID,QuantityPerUnit,UnitPrice,UnitsInStock,UnitsOnOrder,ReorderLevel,Discontinued) 
      Select ProductID, inserted.LeadID, ProductName,SupplierID,CategoryID,QuantityPerUnit,UnitPrice,UnitsInStock,UnitsOnOrder,ReorderLevel,Discontinued 
      from 
        Temp_Products
        inner join inserted on inserted.Leadid = ?
    END

GO

where ? represents your joined table.column. Do you want to add temp_products for each LeadId?

Mitch Wheat
This is the correct way, you can probably eliminate the join back to the Temp_Products since the inserted pseudotable will have all the values already unless it is an instead of trigger
SQLMenace
No I do not want to add Temp_Products for LeadId
@Badmate: perhaps you could explain a bit more?
Mitch Wheat