Efficiency Suggestions Greatly Appreciated

  • -- Here are the basic columns of the tables involved

    -- Ommitted extra columns, Contraints, Defaults, etc.. for simplicity

    CREATE TABLE [Transmittal] (

     [TransId] [int] IDENTITY (1, 1) NOT NULL ,

     [JobId] [int] NOT NULL ,

     [StaffId] [int] NOT NULL)

    Go

    CREATE TABLE [TransmittalRecipient] (

     [TransRecipId] [bigint] IDENTITY (1, 1) NOT NULL ,

     [TransId] [int] NOT NULL ,  -- FK constraint in place to Transmittal.TransId

     [TransRecipTypeId] [tinyint] NOT NULL , 

     [CompanyId] [int] NOT NULL ,

     [ContactId] [int] NULL)

    Go

    CREATE TABLE [TransmittalPlans](

     [TransPlanId] [bigint] IDENTITY (1, 1) NOT NULL ,

     [TransRecipId] [bigint] NOT NULL, -- FK to TransmittalRecipient.TransRecipId

     [TransPlanTypeId] [smallint] NOT NULL ,

     [TransPlanDescId] [smallint] NULL ,

     [Sets] [smallint] NOT NULL ,

     [Prints] [smallint] NOT NULL)

    Go

    Because of one to many identity FK relations between the three tables I can not just do straight insert/select based on TransId

    I need the identity values generated for each new TransmittalRecipient record to relate and copy the TransmittalPlans with

    What I normally would do is create a stored procedure to

    a) copy main Transmittal record for specific TransId value and grab scope_identity() for @iNewTransId

    b) create temp table #tmp for TransmittalRecipients and insert all existing records with the original TransId value

    c) process in a PK controlled loop to avoid using a {shudder} cursor

    (pardon quick syntax though I thought I would ask before coding this one!)

     While Count(PK) from #tmp >0

        { select top 1 PK = @pk From #tmp

          Copy TransmittalRecipient Where PK = @pk using @iNewTransId as new FK

          get Identity @NewPK

          Copy TransmittalPlans with TransRecipId = @pk using @NewPK as new FK

          delete #tmp where PK = @pk

        }

    $0.02 question: Is there a better/more_efficient way of doing this?


    In the beginning the Universe was created. This has made a lot of people very angry and has been widely regarded as a bad move.   Douglas Adams (1952-2001)

  • I may not have a full understanding of the Business Logic behind your operation, however, once you insert a record in [Transmittal] the @@identity will hold its last identity inserted. So, pass that value to the insert into [TransmittalRecipient]

    For example:

    INSERT INTO [TransmittalRecipient](...)

    VALUES(@@IDENTITY, @TransRecipTypeId,@CompanyId,@ContactId)

    ...and apply same login when inserting into [TransmittalPlans], replace the FK with the @@IDENTITY var.

  • Yes I am fully aware of @@Identity, which isn't the problem here (I use scope_identity() anyways because the last identity inserted isn't always the one I need!)

    I read back and realized that I didn't exactly say what I was doing in the first place - sorry.

    As background information, a Transmittal record has a number of Recipients, each of which have a number of Plans sent to them.  So the Recipients are related to the main Transmittal via TransId.  Each recipient will have their own set of Plans sent to them, and thus are related to the Recipient based on TransRecipId.  The user wants to be able to copy a complete transmittal and avoid having to re-enter all the data merely change an item or two.

    So I need to copy the main Transmittal record, related Recipient records, Plan records related to those Recipient records as a new Transmittal.


    In the beginning the Universe was created. This has made a lot of people very angry and has been widely regarded as a bad move.   Douglas Adams (1952-2001)

  • Here is a stripped down copy of the actual procedure which I would like possible improvement suggestions for.  I have removed extra columns, validations and error handling for display purposes:

    Alter Procedure spCopyTransmittal

    (

     @TransmittalId int,

     @ResCode int Output  -- New Transmittal Identity value or -1 for error

    )

    As

    Set NoCount On

      Declare @iNewTransId int

      Declare @iRowRecipId bigint

      Declare @iNewRecipId bigint

      Create Table #tRecipients(

         TransRecipId bigint,

         TransRecipTypeId int Null,

         CompanyId int Null,

         ContactId int Null

       )

       -- Copy the main Transmittal data

       Insert Into Transmittal

            ( JobId, StaffId, TransReasonId, TransDate)

       Select JobId, StaffId, TransReasonId, TransDate 

         From Transmittal

        Where TransId = @TransmittalId

      

       -- Use scope_identity to make sure always getting the correct PK

       Select @iNewTransId = scope_identity()

      

      -- Copy the recipient details using the new identity

      -- Note because each of these records has a PK Identity for

      --  subrecords in Plans, need to create individually by row

      Insert Into #tRecipients

           ( TransRecipId, TransRecipTypeId, CompanyId, ContactId )

      Select TransRecipId, TransRecipTypeId, CompanyId, ContactId

        From TransmittalRecipient

       Where TransId = @TransmittalId

      -- Loop thru each recipient and copy RecipientData, Get new Id, then Copy PlanData

      While (Select Count(TransRecipId) From #tRecipients) > 0

       Begin

          -- Process one recipient at a time

          Select Top 1

                 @iRowRecipId = TransRecipId

            From #tRecipients

          -- Get that recipient's data

          Insert Into TransmittalRecipient

               ( TransId, TransRecipTypeId, CompanyId, ContactId )

          Select @iNewTransId, TransRecipTypeId, CompanyId, ContactId

            From TransmittalRecipient

           Where TransRecipId = @iRowRecipId

          -- Retain the identity

          Select @iNewRecipId = scope_identity()

          -- Copy Plans using retained Recipient Identity as the FK for copied records

          Insert Into TransmittalPlans

               ( TransRecipId, TransPlanTypeId, TransPlanDescId, Sets, Prints)

          Select @iNewRecipId, TransPlanTypeId, TransPlanDescId, Sets, Prints

            From TransmittalPlans

           Where TransRecipId = @iRowRecipId

          -- Remove the current processed row from the temp Recipient table

          Delete #tRecipients

           Where TransRecipId = @iRowRecipId

          -- Keep processing for as long as have records left in #tRecipients

          Continue

       End

      -- Take out the trash

      Drop Table #tRecipients

       -- Done.  Return the main Transmittal generated Identity

       Select @ResCode=@iNewTransId

    Set NoCount Off

    Return

    Go


    In the beginning the Universe was created. This has made a lot of people very angry and has been widely regarded as a bad move.   Douglas Adams (1952-2001)

  • Do you create a new records for Recipient and Plan even if there are existing records with same values?

    I would suggest revising this approach. You must have logical unique key for each dataset, and IDENTITY must be just "shortcut" to that key.

    I use to use views with INSTEAD OF INSERT triggers for such complex inserts.

    You have whole inserted dataset enclosed in table INSERTED and you may join static tables to this temp table to retrieve new identities.

    Insert Into dbo.tblTransmittal

    ( JobId, StaffId, TransReasonId, TransDate)

    Select JobId, StaffId, TransReasonId, TransDate

    From INSERTED I

    WHERE NOT EXISTS (select 1 from dbo.tblTransmittal T where T.{key columns} = I.{key columns})

    Insert Into dbo.TransmittalRecipient

    ( TransId, TransRecipTypeId, CompanyId, ContactId )

    Select T.TransId, TransRecipTypeId, CompanyId, ContactId

    From INSERTED I

    INNER JOIN dbo.tblTransmittal T ON T.{key columns} = I.{key columns}

    WHERE NOT EXISTS (select 1 from dbo.TransmittalRecipient TR where TR.{keys} = {I+T}.{keys} )

    Etc.

    Hope you get the idea.

    _____________
    Code for TallyGenerator

  • Thank you for your response.  I do see what you are getting at though and do something similar on a different project.  The scope of this one is a bit different though.

    The whole point of this is for the user to be able to exactly copy an existing one and then modify it to send out Transmitals to clients again.  It's sort of like having a Transmittal template. 

    A lot of times the user is just going to change the date and resend the exact same plans to the exact same recipients, just on a different date.  I need to historically track ALL transmittals so once it is in the system and has been sent to the client the records are no longer edited.


    In the beginning the Universe was created. This has made a lot of people very angry and has been widely regarded as a bad move.   Douglas Adams (1952-2001)

  • So, you need just include date into the list of key columns.

    No problems here.

    The only question is: what does that record with old date represent?

    Why do you need deleted data in line with actual records?

    Just to kill performance and increase probability of mistakes? Or there is another point?

    _____________
    Code for TallyGenerator

  • This is bordering on getting off topic. The record with the old date represents the last group of files that were sent to a client which we retain the record of for legal purposes and thus never delete it.  So there is no deleted data in line with actual records.  The whole point is I am using previous data as template to begin another file to client process to save the user data entry time.  That's it.  Each Transmittal is it's own distinct legal process.

    Using the trigger method is not what I want to do as the number of times they will be copying an existing one as opposed to starting a new one is relatively low and I don't need to be firing a trigger for every normal insert of records. 

    I hope this has explained it well enough.  Any other ideas out there?


    In the beginning the Universe was created. This has made a lot of people very angry and has been widely regarded as a bad move.   Douglas Adams (1952-2001)

Viewing 8 posts - 1 through 7 (of 7 total)

You must be logged in to reply to this topic. Login to reply