Updating a Table from a Stored Procedure

Question

I am trying to learn database on my own; all of your comments are appreciated. I have the following table.

CREATE TABLE AccountTable
(
    AccountId INT IDENTITY(100,1) PRIMARY KEY,
    FirstName NVARCHAR(50) NULL,
    LastName NVARCHAR(50) NULL,
    Street NVARCHAR(50) NULL,
    StateId INT REFERENCES STATETABLE(StateId) NOT NULL
)

I would like to write a Stored procedure that updates the row. I imagine that the stored procedure would look something like this:

CREATE PROCEDURE AccountTable_Update
       @Id          INT,
       @FirstName  NVARCHAR(20),      
  @LastName   NVARCHAR(20), 
  @StreetName NVARCHAR(20),
  @StateId  INT
  AS
BEGIN
UPDATE AccountTable 
  Set FirstName = @FirstName
  Set LastName = @LastName
  Set Street = @StreetName
  Set StateId = @StateId 
  WHERE AccountId = @Id
END

the caller provides the new information that he wants the row to have. I know that some of the fields are not entirely accurate or precise; I am doing this mostly for learning.

1. I am having a syntax error with the SET commands in the UPDATE portion, and I don't know how to fix it.
2. Is the stored procedure I am writing a procedure that you would write in real life? Is this an antipattern?
3. Are there any grave errors I have made that just makes you cringe when you read the above TSQL?

Solution 1

Are there any grave errors I have made that just makes you cringe when you read the above TSQL?

Not really "grave," but I noticed your table's string fields are set up as the datatype of NVARCHAR(50) yet your stored procedure parameters are NVARCHAR(20). This may be cause for concern. Usually your stored procedure parameters will match the corresponding field's datatype and precision.

Solution 2

#1: You need commas between your columns:

UPDATE AccountTable SET
    FirstName = @FirstName,
    LastName = @LastName,
    Street = @StreetName,
    StateId = @StateId
WHERE 
    AccountId = @Id

SET is only called once, at the very start of the UPDATE list. Every column after that is in a comma separated list. Check out the MSDN docs on it.

#2: This isn't an antipattern, per se. Especially given user input. You want parametized queries, as to avoid SQL injection. If you were to build the query as a string off of user input, you would be very, very susceptible to SQL injection. However, by using parameters, you circumvent this vulnerability. Most RDBMS's make sure to sanitize the parameters passed to its queries automagically. There are a lot of opponents of stored procedures, but you're using it as a way to beat SQL injection, so it's not an antipattern.

#3: The only grave error I saw was the SET instead of commas. Also, as ckittel pointed out, your inconsistency in the length of your nvarchar columns.

Comments

[Remus Rusanu]

Is good that you're trying to learn, but for things like syntax errors you should check first the MSDN: msdn.microsoft.com/en-us/library/ms177523.aspx. Even if the BNF notation can be challenging until you get used to it, is a must. As things will get more complicated you will have to resort to the documentation more and more often.

[MedicineMan]

I'm afraid I don't understand what you are saying in #2 and #3. Would you mind elaborating?