1

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
4

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.

  • 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. – user3268054 Jul 7 '15 at 17:53
  • @user3268054 Try it now, just missed a string delimiter (again, this is another reason you should be learning the second form - less string escaping). – Aaron Bertrand Jul 7 '15 at 18:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.