views:

607

answers:

8

Hi,

I have a table:

Message (MessageID int, Subject nvarchar(100), Body nvarchar(max))

After a message is being updated on UI, I call a stored proc to update that table. In some cases user might update just subject, in other cases just body. I want this stored proc to only update what has changed, so I'm also passing flags showing whether subject or body has been updated:

create proc UpdateMessage(
  @MessageID int, 
  @Subject nvarchar(100), 
  @Body nvarchar(max),
  @SubjectChanged bit,
  @BodyChanged bit)

And now i'm confused how to build the conditional UPDATE statement. My first thought was to use CASE:

Update [Message] 
SET 
CASE WHEN @SubjectChanged = 1 THEN [Subject] = @Subject ELSE 1=1 END,
CASE WHEN @BodyChanged = 1 THEN Body = @Body ELSE 1=1 END,
WHERE MessageID = @MessageID

... but that doesn't seem to be a correct syntax as CASE has to be the right side of an assigment.

Any ideas how I could do that? (And keep in mind that in reality there are 6 parameters that can be updated, not two)

Thanks! Andrey

+1  A: 

Seems to me like you are wasting a lot of effort. If you retrieve the six values, display them to the user (in some user interface) and they can change some variable number of them and hit a "save" button - then just update all 6 fields every time, getting the new values from the user input fields.

Some may not have changed, but so what. Much simpler code that way.

Ron

Ron Savage
Keep in mind, the message body can be 5-10-50kb and if a user just updated subject, it's extremely wasteful to send the message body back and also perform that field's update to the db when its not necessary.The website is heavy load so I need to make sure I optimize where possible.
Andrey
Why not just use separate stored procedures then? Does it really need to be encompassed in a single sproc? You'll probably want to make some modifications in the future that will totally ruin your "awesome plan"
Joe Philllips
"Why not just use separate stored procedures then?" - to save on multiple db calls
Andrey
A: 

Use DEFAULT values for the stored procedure parameters.

create proc UpdateMessage(
  @MessageID int,  -- mandatory
  @Subject nvarchar(100) = NULL, 
  @Body nvarchar(max) = NULL)

Then, you can structure your update in this way:

Update [Message] 
SET 
[Subject] = ISNULL(@Subject, [Subject]),
Body = ISNULL(@Body, Body)
WHERE MessageID = @MessageID
Rodrigo
What if you're trying to update a field to NULL ?
Remus Rusanu
And also, when @Body is NULL, I'm updating a field with it's own value, which is wasting valuable resources (e.g. what if Message.Body is 1 Meg? It's a pretty heavy update, and unnecessary one).
Andrey
A: 
CREATE PROCEDURE UpdateMessage
  @MessageID int, 
  @Subject nvarchar(100), 
  @Body nvarchar(max),
AS
BEGIN
    if(@Subject is null or @Subject='')
        SELECT @Subject=Subject FROM Message WHERE MessageID=@MessageID
    if(@Body is null or @Body='')
        SELECT @Body=Body FROM Message WHERE MessageID=@MessageID
    UPDATE Message SET Subject=@Subject, Body=@Body WHERE MessageID=@MessageID
END
GO
byte
+4  A: 
update Message set
    Subject = (case when @SubjectChanged = 1 then @Subject else Subject end),
    Body = (case when @BodyChanged = 1 then @Body else Body end)

where MessageID = @MessageID

That should really be all you need. However, if you truly can't update the field if it hasn't changed, then you'll have to do it in separate statements.

if @SubjectChanged = 1 
    update Message set Subject = @Subject where MessageID = @MessageID
if @BodyChanged = 1 
    update Message set Body = @Body where MessageID = @MessageID
Adam Robinson
A: 

I am not sure if this is the best way to do it, but maybe you can try

IF @SubjectChanged = 1 THEN
   BEGIN
      UPDATE [Message]
      SET [Subject] = @Subject
      WHERE MessageID = @MessageID     
   END
END

IF @BodyChanged = 1 THEN
   BEGIN
      UPDATE [Message]
      SET Body = @Body
      WHERE MessageID = @MessageID
   END
END
Waleed Al-Balooshi
+2  A: 

Your best bet, by far, is to use explicit IF statements:

IF @subjectHasChanged = 1 and @bodyHasChanged = 1
 UPDATE Messages SET Subject = @subject, Body = @body 
   WHERE MessageId = @MessageId
ELSE IF @subjectHasChanged = 1
 UPDATE Messages SET Subject = @subject WHERE MessageId = @MessageId
ELSE IF @bodyHasChanged
 UPDATE Messages SET Body = @body WHERE MessageId = @MessageId

From a performance point of view, nothing beats this. Because SQL can see during query compilation that you only update Body, or Subject, or both, it can generate the appropriate plan, for instance not even bothering to open (for update) the non-clustered index you have on Subject (if you have one, of course) when you only update Body.

From a code code quality point of view, this is disaster, a nightmare to maintain. But acknowledging the problem is 80% solving the problem :) . You can use code generation techniques for instance to maintain such problem procedures.

Another viable approach is actually to use dynamic SQL, construct the UPDATE in the procedure and use sp_executesql. It has its own set of problems, as all dynamic SQL has. There are resources about dynamic SQL problems, and there are workarounds and solutions, see The Curse and Blessings of Dynamic SQL.

Remus Rusanu
A: 

I would highly recommend using Adam Robinson's method if you require this to be in a single stored procedure.

Even better would be to simply use separate stored procedures for each one of these updates.

Joe Philllips
A: 

The syntax required to create your statement is:

Update [Message] 
SET    [Subject] = CASE WHEN @SubjectChanged = 1 THEN @Subject ELSE [Subject] END,
       Body = CASE WHEN @BodyChanged = 1 THEN @Body ELSE Body END
WHERE  MessageID = @MessageID

if you still want to stick to it after all the suggestions.

N.b. if you leave out the ELSE [Subject] part of the CASE statements, instead of ignoring the UPDATE it sets the field to NULL.

Rafe Lavelle