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.