views:

99

answers:

5

I have a stored procedure that will INSERT a record into a table. I have created a unique key constraint on the table to avoid duplicate records.

Regardless of the fact that there is a unique key constraint defined, I am using an IF() condition (prior to the INSERT statement) to check for a duplicate record that may already exist.

However, the conditional statement that I am using to check for a duplicate record is seemingly having no affect on whether or not the INSERT is executed. - i.e. when a duplicate record in submitted to the sproc, a "Violation of UNIQUE KEY constraint..." exception is thrown.

Here is a sample of my sproc:

BEGIN       
    if
        (SELECT Count(f1)
        FROM table
        WHERE f1 = @f1
        AND f2= @f2) 
    <= 0
    BEGIN
        INSERT INTO table
        (f1,f2)
        VALUES
        (@f1, @f2)

        RETURN @@IDENTITY
    END
END

Is there something wrong with my syntax? Or, maybe, I'm going about this the wrong way?

+2  A: 

This is more concurrency safe:

INSERT INTO table
SELECT @f1, @f2, @f3
  FROM TABLE WITH (UPDLOCK, HOLDLOCK)
 WHERE NOT EXISTS(SELECT NULL
                    FROM table
                   WHERE f1 = @f1
                     AND f2 = @f2
                     AND f3 = @f3) 

Or, if you want to keep the decision logic (but less concurrency safe), use NOT EXISTS:

IF NOT EXISTS(SELECT NULL
                FROM table
               WHERE f1 = @f1
                 AND f2 = @f2
                 AND f3 = @f3) 
BEGIN

    INSERT INTO table
      (f1,f2, f3)
    VALUES
      (@f1, @f2, @f3)

    RETURN @@SCOPE_IDENTITY

END
OMG Ponies
You should use SCOPE_IDENTITY() instead of @@IDENTITY, a trigger on the table might change @@IDENTITY
Jason Goemaat
@Jason Goemaat: Fixed.
OMG Ponies
It's still **NOT** concurrency safe, though. You must either use `WITH (UPDLOCK, HOLDLOCK)` in the select, or recover from the failed insert and perform an update instead.
Emtucifor
+3  A: 

Count(field) counts values that are not null.
This may be different to count(*), which counts rows.

But I would discourage using count here.
Have a look at this question.

GSerg
A: 
SET NOCOUNT ON  
SELECT * FROM table  
WHERE f1 = @f1
        AND f2= @f2  

IF @@ROWCOUNT = 0  
BEGIN  
 INSERT INTO table  
        (f1,f2)  
        VALUES  
        (@f1, @f2)  


    RETURN @@IDENTITY  
END  
FatherStorm
+3  A: 

If you're using SQL Server 2008 or newer then you should use the MERGE statement:

MERGE INTO table AS t
USING (VALUES (@f1, @f2)) AS s (f1, f2)
    ON s.f1 = t.f1
        AND s.f2 = t.f2
WHEN NOT MATCHED BY t THEN
    INSERT (f1, f2) VALUES (@f1, @f2)
LukeH
This still has concurrency problems...
Emtucifor
Emtucifor: What concurrency problem does `MERGE` have?
Gabe
+1  A: 

Thanks for your input OMG/Martin, but it turns out that the root of my problem was due to a parameter value that was null.

Specifically:

--sproc params
@f1 int,
@f2 nvarchar(30),
@f3 datetime = null --<<a null value will screw up the IF() condition

IF(SELECT COUNT(f1)
FROM table
WHERE f1 = @f1
AND f2 = @f2
AND f3 = @f3)
<1

BEGIN
  INSERT INTO....
END

Notice that the default value of the @f3 param is null. So, in the case where the caller of the sproc doesn't pass an @f3 parameter, the IF() condition will return 0 (zero) even if there is a matching @f1, @f2, and null @f3.

For example: Say the table already has a record of

f1   f2   f3
--------------
45   foo  NULL

Now a caller fires off the sproc by sending it:

@f1=45

@f2=foo

(Note the caller does not specify @f3)

When the IF() condition if fired, it will return 0 (zero). Strange, eh? Intuitively, I would think that the condition would return 1 since the params are an exact match as the existing values in the table.

To make matters a bit more confusing (for me, anyway), is that even though the IF() succeeds (returns zero), I receive a "Violation of UNIQUE KEY constraint.." exception. Actually, I'm not surprised that the exception is thrown since I consider the param values to be a perfect match to the existing record in the table - the confusing part, to me, is why does the IF() condition contradict the violation exception.

By the way, OMGPonies, I tried using your suggestion (EXISTS), and the same symptoms occur. Apparently, the NULL factor makes things funky.

Jed
If you'd have shown us your actual code I would have guessed that as an option! Using `Merge` and `Scope_Identity` are still both safer than what you are currently doing.
Martin Smith
For the record, the MERGE solution doesn't fix the NULL problem. In other words, in the ON join of my MERGE statement, if one (or more) of the fields have the NULL value, WHEN NOT MATCHED is always executed.
Jed
@Jed - Of course it won't because this was a logical error your end. No language feature will fix that. Merge will be safer under conditions of concurrency.
Martin Smith
Are you saying it is impossible to search for null records?
Jed