Stored procedure to insert a new person

Question

I have not written very many stored procedures before, and this is the first one I have written to be used in a real business application to write new data into the database, so I would like to get advice to improve it from more experienced SQL developers.

In short it is meant to insert a new person along with their contact information, if it is made available. Several fields allow null because the information is not necessarily required for our purposes.

I have tested all the data validations to make sure errors are thrown when validations fail.

For reference, this is using version Microsoft SQL Server 2014.

Here is the stored procedure create query for review:

if exists (
    select 
        * 
    from 
        INFORMATION_SCHEMA.ROUTINES
    where
        ROUTINE_NAME = 'usp_CreatePerson'
        and SPECIFIC_SCHEMA = 'PsychoProductions'
        and ROUTINE_TYPE = 'PROCEDURE'
)
drop procedure PsychoProductions.usp_CreatePerson;
go

create procedure PsychoProductions.usp_CreatePerson
    @PersonRoleId int,
    @Name varchar(500) = null,
    @Organization varchar(500) = null,
    @Website varchar(500) = null,
    @DefaultBillingMethodId int = 1, --unassigned
    @AddressTypeId int = null,
    @StreetAddress varchar(500) = null,
    @City varchar(200) = null,
    @State char(2) = null,
    @Zip varchar(10) = null,
    @PhoneTypeId int = null,
    @PhoneNumber varchar(20) = null,
    @EmailTypeId int= null,
    @EmailAddress varchar(200) = null
as
set nocount on;

begin transaction;

if not exists (select 1 from PsychoProductions.PersonRoles where Id = @PersonRoleId)
begin;
    throw 50001, 'Invalid PersonRoleId', 1
    rollback transaction;
end;
else 
begin;
    insert into PsychoProductions.Persons (
        PersonRoleId,
        Name,
        Organization,
        Website,
        DefaultBillingMethodId,
        IsActive
    ) values (
        @PersonRoleId,
        @Name,
        @Organization,
        @Website,
        @DefaultBillingMethodId,
        1
    );

    declare @NewPersonId int = @@IDENTITY;

    if @AddressTypeId is not null
    begin;
        if not exists (select 1 from PsychoProductions.AddressTypes where Id = @AddressTypeId)
        begin;
            throw 50001, 'Invalid AddressTypeId', 1;
            rollback transaction;
        end;
        else 
        begin;
            insert into PsychoProductions.PersonAddresses (
                PersonId,
                AddressTypeId,
                StreetAddress,
                City,
                St,
                Zip
            ) values (
                @NewPersonId,
                @AddressTypeId,
                @StreetAddress,
                @City,
                @State,
                @Zip
            );
        end;
    end;

    if @PhoneTypeId is not null
    begin;
        if not exists (select 1 from PsychoProductions.PhoneTypes where Id = @PhoneTypeId)
        begin;
            throw 50001, 'Invalid PhoneTypeId', 1;
            rollback transaction;
        end;
        else
        begin;
            insert into PsychoProductions.PersonPhones (
                PersonId,
                PhoneTypeId,
                PhoneNumber
            ) values (
                @NewPersonId,
                @PhoneTypeId,
                @PhoneNumber
            );
        end;
    end;

    if @EmailTypeId is not null
    begin;
        if not exists (select 1 from PsychoProductions.EmailTypes where Id = @EmailTypeId)
        begin;
            throw 50001, 'Invalid EmailTypeId', 1;
            rollback transaction;
        end;
        else
        begin;
            insert into PsychoProductions.PersonEmails (
                PersonId,
                EmailTypeId,
                Email
            ) values (
                @NewPersonId,
                @EmailTypeId,
                @EmailAddress
            );
        end;
    end;
    commit transaction;
end;

go

Here is an example valid procedure call:

DECLARE @RC int
DECLARE @PersonRoleId int = 2
DECLARE @Name varchar(500) = 'John Doe'
DECLARE @Organization varchar(500) = 'John Doe Inc'
DECLARE @Website varchar(500) = null
DECLARE @DefaultBillingMethodId int = 1
DECLARE @AddressTypeId int = 1
DECLARE @StreetAddress varchar(500) = '123 Main Street'
DECLARE @City varchar(200) = 'New York'
DECLARE @State char(2) = 'NY'
DECLARE @Zip varchar(10) = '12345'
DECLARE @PhoneTypeId int = 1
DECLARE @PhoneNumber varchar(20) = '(111)111-1111'
DECLARE @EmailTypeId int = 1
DECLARE @EmailAddress varchar(200) = '[email protected]'

EXECUTE @RC = [PsychoProductions].[usp_CreatePerson] 
   @PersonRoleId
  ,@Name
  ,@Organization
  ,@Website
  ,@DefaultBillingMethodId
  ,@AddressTypeId
  ,@StreetAddress
  ,@City
  ,@State
  ,@Zip
  ,@PhoneTypeId
  ,@PhoneNumber
  ,@EmailTypeId
  ,@EmailAddress

Results:

Id          PersonRoleId Name     Organization Website     DefaultBillingMethodId IsActive    CreatedDate
----------- ------------ -------- ------------ ----------- ---------------------- ----------- -----------------------
10          2            John Doe John Doe Inc NULL        1                      1           2016-06-17 17:35:56.813

Id          PersonId    AddressTypeId StreetAddress   City     St   Zip   CreatedDate
----------- ----------- ------------- --------------- -------- ---- ----- -----------------------
2           10          1             123 Main Street New York NY   12345 2016-06-17 17:35:56.812

Id          PersonId    PhoneTypeId PhoneNumber   CreatedDate
----------- ----------- ----------- ------------- -----------------------
6           10          1           (111)111-1111 2016-06-17 17:35:56.813

Id          PersonId    EmailTypeId Email               CreatedDate
----------- ----------- ----------- ------------------- -----------------------
5           10          1           [email protected] 2016-06-17 17:35:56.813

For reference, the table structure for these tables looks like this:

create table PsychoProductions.Persons (
    Id int identity(1,1) primary key,
    PersonRoleId int not null 
        references PsychoProductions.PersonRoles(Id),
    Name varchar(500) null,
    Organization varchar(500) null,
    Website varchar(500) null,
    DefaultBillingMethodId int not null
        references PsychoProductions.BillingMethods(Id),
    IsActive bit default 1,
    CreatedDate datetime default getdate()
);

create table PsychoProductions.PersonAddresses (
    Id int identity(1,1) primary key,
    PersonId int not null
        references PsychoProductions.Persons(Id),
    AddressTypeId int not null
        references PsychoProductions.AddressTypes(Id),
    StreetAddress varchar(500) null,
    City varchar(200) null,
    St char(2) null,
    Zip varchar(10) null,
    CreatedDate datetime default getdate()
);

create table PsychoProductions.PersonPhones (
    Id int identity(1,1) primary key,
    PersonId int not null
        references PsychoProductions.Persons(Id),
    PhoneTypeId int not null
        references PsychoProductions.PhoneTypes(Id),
    PhoneNumber varchar(20) not null,
    CreatedDate datetime default getdate()
);

create table PsychoProductions.PersonEmails (
    Id int identity(1,1) primary key,
    PersonId int not null
        references PsychoProductions.Persons(Id),
    EmailTypeId int not null
        references PsychoProductions.EmailTypes(Id),
    Email varchar(200) not null,
    CreatedDate datetime default getdate()
);

Answer 1

Per billinkc in the comments, rollback before throw would do what you intend.

Per Michael Green, avoid using @@IDENTITY where possible. I recommend using output instead. Here's an example:

-- ... blah blah blah ...

-- This table will store the Id of the person you inserted
declare @PersonId table (Id int)

-- ... blah blah blah ...

insert into PsychoProductions.Persons (
    PersonRoleId,
    Name,
    Organization,
    Website,
    DefaultBillingMethodId,
    IsActive
) 
output Inserted.Id into @PersonId (Id)
values (
    @PersonRoleId,
    @Name,
    @Organization,
    @Website,
    @DefaultBillingMethodId,
    1
);

-- ... blah blah blah ...

Now you've got the table @PersonId with a single column Id and a single row with the new ID you just inserted, so when you need to access that ID when inserting addresses and phone numbers, you can join or subquery @PersonId like you would any other table.

As far as I'm aware, variable scoping doesn't matter in SQL Server stored procedures – if you declare @PesronId inside the if block, it will still be accessible once the code has left that block. So you could declare the table right before the Persons insert to keep its use obvious, or near the top of the query like normal coding languages, to imply it is used not only in that if block.

Everything else looks good to me.