Stored Procedure throws Violation of PRIMARY KEY constraint when called concurrently whit same parameters

  • Sorry to reply to a 3 year old post but it is pertinent to a problem I've currently got.

    I like Gail's article but the PK violation I'm getting is only a small part of a complex Stored Procedure, a great deal of which is in a Transaction.

    If I put XLOCK, HOLDLOCK on my destination table the exclusive lock will be held for the duration of the Transaction which I can't afford to do.

     INSERT dbo.PersonWithUnviewedAlerts ( PersonID )
    SELECT @PersonID
    WHERE NOT EXISTS (
    SELECT PersonID
    FROM dbo.PersonWithUnviewedAlerts WITH ( XLOCK, HOLDLOCK )
    WHERE PersonID = @PersonID )

    The same is true if I were to set Transaction Isolation Level Serializable, and in my tests it is also true if I take this code and put it in another Proc, which would be called from the main Proc.

    So I've encapsulated the Insert statement within a TRY CATCH block, the catch simply allowing the Code to continue.

    For clarity, the larger business logic is within a TRY CATCH where the catch rolls back the transaction and this would be a nested TRY CATCH.

    BEGIN TRY

    BEGIN TRAN

    Main Code...

    BEGIN TRY
    INSERT dbo.PersonWithUnviewedAlerts (PersonID)
    SELECT @PersonID
    WHERE NOT EXISTS ( SELECT PersonID FROM dbo.PersonWithUnviewedAlerts WHERE PersonID = @PersonID )
    END TRY
    BEGIN CATCH
    PRINT 'Attempt to insert duplicate PK in dbo.PersonWithUnviewedAlerts';
    END CATCH

    More Code...

    COMMIT TRAN

    END TRY

    BEGIN CATCH
    IF @@TRANCOUNT > 0
    ROLLBACK TRANSACTION;

    END CATCH?

    For MY business rules it works fine, but is there something wrong in implementing this?

    I do believe in prevention rather than cure, but is the NOT EXISTS in my WHERE clause no longer required as I can let my CATCH take care of it?

    Regards

    Giles

     

  • If you're not really interested in seeing whether a duplicate insert was attempted, consider adding IGNORE_DUP_KEY = ON to your target table definition.

    Your code then becomes much simpler.

    If you haven't even tried to resolve your issue, please don't expect the hard-working volunteers here to waste their time providing links to answers which you could easily have found yourself.

  • Why are you outputting this line, whatever the error?

    PRINT 'Attempt to insert duplicate PK in dbo.PersonWithUnviewedAlerts';

    What if it's caught due to a different error?

  • Jonathan AC Roberts, that's a very good point.

    I'm currently Unit testing Phil Parkin's solution, which fits my brief and is elegant to boot.

    Thanks for looking.

    Giles

  • giles.clapham wrote:

    Jonathan AC Roberts, that's a very good point.

    I'm currently Unit testing Phil Parkin's solution, which fits my brief and is elegant to boot.

    Thanks for looking.

    Giles

    Personally I'd just leave it with the NOT EXISTS in and remove the TRY CATCH around it.

  • Thanks Phil Parkin, it's worked very well.

    Jonathan AC Roberts, I agree with keeping the NOT EXISTS. If I can prevent the Insert then I should and IGNORE_DUP_KEY is only ON to cope with the occasions where atomically two processes are entering at the same time.

Viewing 6 posts - 31 through 35 (of 35 total)

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