patternsqlMinor
Stored procedure to insert a new person
Viewed 0 times
storedpersoninsertnewprocedure
Problem
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
```
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,
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,
Solution
Per billinkc in the comments,
Per Michael Green, avoid using
Now you've got the table
As far as I'm aware, variable scoping doesn't matter in SQL Server stored procedures – if you declare
Everything else looks good to me.
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.
Code Snippets
-- ... 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 ...Context
StackExchange Code Review Q#132339, answer score: 3
Revisions (0)
No revisions yet.