Problem locating rows with dynamic SQL

Question

I'm new at SQL in general, so I'm have a few problems with some of my stored procedures I'm designing. My procedure takes in 3 parameters. A location (or table name basically, a varchar), a room number (varchar), and a phone number (char) in the form of 'xxx-xxxx'. Here is the code for the procedure.

ALTER PROCEDURE [dbo].[AddPhoneNumberToRoom]
 @Location varchar(25),
 @Room varchar(10),
 @PhoneNumber char(8)
AS
BEGIN
SET NOCOUNT ON;
 DECLARE @String varchar(100);
 SELECT @String = ' UPDATE ' + @Location + 
 ' SET PhoneNumber = ' + @PhoneNumber +
 ' WHERE Room = ' + @Room;
 EXEC(@String);
END

The error I receive is:

The UPDATE statement conflicted with the FOREIGN KEY constraint "FK_Apartments_PhoneNumbers". The conflict occurred in database "CMPhone", table "dbo.PhoneNumbers", column 'PhoneNumber'."

The phone number is in the PhoneNumber table, so when I do:

UPDATE Apartments SET PhoneNumber='123-4567' WHERE Room='2'

It works just fine. The PhoneNumbers table also looks like this, just has the one number in it.

PhoneNumber
-----------
123-4567

Answer 1

PRINT @String; will yield that it says PhoneNumber = 123-4567 (which treats it like an expression, 123 subtract 4567, which equals -4444), not PhoneNumber = '123-4567' like in the example you hard-coded. So you should be escaping the values you're appending with two single-quotes:

SELECT @String = ' UPDATE ' + @Location + 
 ' SET PhoneNumber = ''' + @PhoneNumber + ''' 
 WHERE Room = ''' + @Room + ''';';

But better yet, to better protect yourself from SQL injection (see here, here, and here), you should have (a) check that @Location is a valid table, (b) QUOTENAME() it anyway, and (c) pass in the other two values as properly typed parameters rather than appending them to the string:

IF OBJECT_ID(N'dbo.' + @Location) IS NOT NULL
BEGIN
 DECLARE @sql NVARCHAR(MAX);
 SET @sql = N'UPDATE dbo.' + QUOTENAME(@Location)
 + N' SET PhoneNumber = @PhoneNumber
 WHERE Room = @Room;';
 EXEC sys.sp_executesql @sql, 
 N'@PhoneNumber CHAR(8), @Room VARCHAR(10)',
 @PhoneNumber, @Room;
END

And even better still, you shouldn't have a separate table for each Location IMHO - Location should be a column. Then you wouldn't need dynamic SQL at all.

• user3268054: Ok, so I put the first section in (the escaping the values one) and I get an incorrect syntax near ' WHERE Room = ' ' error. Sorry, like I said, i'm new to SQL. The second one does work, but I need to work through the syntax so I know what's going on.
• Aaron Bertrand: @user3268054 Try it now, just missed a string delimiter (again, this is another reason you should be learning the second form - less string escaping).