Separate sprocs for Insert/Update or one Set sproc?

Question

So I came across an instance where a stored procedure which handled the Updates on a specific table was doing nothing because the record didn't exist [duh].

So what I decided to do was change the stored procedure to:

 UPDATE    Table
 SET       col1 = @var1
 WHERE     Type = @type AND UserId = @userId 
 IF @@ROWCOUNT = 0
 BEGIN
  INSERT INTO Table
---etc....
 END

So this obviously solved the problem and I renamed the sproc from UpdateTable to SetTable.

Is this a good idea, or should I do something at the application level where if the update returns 0 rows affected, call the insert procedure.

Answer 1

I like to have one for each. I makes the sproc perform a specific task instead of two, making it easier to maintain and extend.

Answer 2

I prefer the "upsert" that you have shown because it allows me to stay that much further away from persistence related logic. In the application I just want to work with my objects and "if I must" ask them to save / persist. With update and insert separately you are forced to think about this in the domain model. But the biggest advantage to persistence ignorance is that you can focus on real business problems instead of infrastructure concerns.

This design also helped me migrate to an ORM earlier this year because I didn't have a ton of "update" / "insert" calls, but instead a simple "save"

Answer 3

It Depends.

On some places in the application it will make sense to do an upsert because you cannot know, or don't care, if the current saved state should already contain a record. Yet on other places you'll want to know exactly and the pre-existance of a record on insert or the missing record for update should be an error.

Anyway, where you decide to do 'upsert' you have to be carefull though because of the inherent race conditions. Two separate connections may attempt to update, both see @@ROWCOUNT at 0 and then both try to insert, resulting in an primary key violation (if you're lucky) or worse in a inconsistent database state (duplicate record with different info). Getting such operations right is quite tricky. You must either ensure that the UPDATE locks in the state (ie. no other transaction can insert) or be prerared to hit a PK violation on insert and handle it accordingly. I preffer the later option, as the race conditions window is usually quite small and handling the exception is easier than preventing a concurent insert.

One avenue to consider is using the new SQL Server 2008 MERGE statement, it is designed to handle such cases (and finally catch up SQL Server with other vendors, both Oracle and MySQL been offering this functionality for years...).

Answer 4

It's fine if that's consistent with what the application expects.

If the business intention was only to update an existing record OR if the application was expecting the row to already exist and the stored proc silently hides the fact that it didn't, it could break other logic. What if another function/process deleted the row for a reason? What if columns not being updated in the row had information? The insert would possibly change the values of those columns unexpectedly.

Not saying this is the case. Only someone like you familiar with the application can tell whether this change is ok to make.

Answer 5

I like your upsert(although not stored procedures :)) But yea, why do logic on a method that has nothing to do with figuring out whether it needs to update or insert--just let the sproc figure it out.