Problem with stored procedure
-
hi
here is the code
create PROCEDURE StProc_ProcessCustomer
(
@CustID VarChar(10)
@x_Email VarChar(75),
@x_First_Name VarChar(20),
@x_Last_Name VarChar(30),
@x_address VarChar(50),
@x_City VarChar(50),
@x_state VarChar(30),
@x_Zip VarChar(10),
@x_Country VarChar(50),
@x_phone VarChar(30),
@x_fax VarCHar(30) = NULL,
@Password VarChar(20),
@SearchPhone VarChar(20),
@FullName VarChar(50)
)
AS BEGIN
SET NOCOUNT ON
DECLARE @CustID VarChar(10)
-- Make sure this is a new customer
IF EXISTS(SELECT CustID FROM TblCustomers
WHERE EmailAddress = @x_Email)
Set @CustID = CustID
BEGIN
UPDATE TblCustomers SET
FirstName = @x_first_name, LastName = @x_last_name,
Addressline1 = @x_address, City = @x_city,
State = @x_state, Zip = @x_zip,
Country = @x_country, Phone = @x_phone,
FaxNumber = @x_fax, CustPassword = @password,
SearchPhone = @SearchPhone
WHERE CustID = @CUstID
END -- IF EXISTS(SELECT @CustID FROM TblCustomers ...
ELSE
BEGIN
-- get new customer number and insert new record
Select NextCustNo AS CustID FROM TblMisc
Set @CustID = NextCustNo
Update TblMisc SET NextCustNo = (NextCustNo + 1)
INSERT TblCustomers (
CustID, FirstName, LastName, Addressline1, City, FullName,
State, Zip, Country, Phone, ShipAddr1,
ShipCity, ShipState, ShipZip,
ShipCountry, FaxNumber, SearchPhone, EmailAddress,
NoOfSales, GrossSales, DiscountsUsed, Premiums, OneQuickClick,
HasOtherShipping, NoOfVisits, NoOfDownloads, AvaMP3,
SubsType, SubsCurrRenew, SubsNextRenew, SubsStart, CustPassword )
VALUES (
@CustID, @x_first_name, @x_last_name, @x_address, @x_city, @FullName,
@x_state, @x_zip, @x_country, @x_phone, @x_address,
@x_city, @x_state, @x_zip,
@x_country, @x_fax, @searchPhone, @x_email,
0, 0, 0, 0, 0,
0, 0, 0, 0,
'N', NULL, NULL, NULL, @Password )
END -- IF EXISTS(SELECT @CustID FROM TblCustomers ...
-- Return Results
SELECT CustID, EmailAddress
FROM TblCustomers
WHERE CustID = @CustID
END -- InsertCustomer
GO
I'm not getting the existing custID ... I'm real new to stored procedures (this is my third). I'm trying to pass the fields I need to update customer if he exists or to create a new record if he does not. I need it to return the CustomerID and email address. I didn't get a syntax error, just no return and no update. Could someone point me in the right direction?
Thanx
-
A couple of problems I can see immediately:
The code:
IF EXISTS(SELECT CustID FROM TblCustomers
WHERE EmailAddress = @x_Email)
Set @CustID = CustID
BEGIN...assume the previous SELECT returns custID, but it doesn't, it simply checks existence.
Also, the stuff from BEGIN onwards will always be executed, because the SET statement is the only one dependent on the IF.
An alternative is:
SELECT @CustID = CustID FROM TblCustomers
WHERE EmailAddress = @x_Email
IF @@ROWCOUNT > 0 -- Yes it exists and @CustID now contains the CustID
BEGIN
...
ENDCheers,
- Mark
Cheers,
- Mark -
Hi
it works 🙂 ... BUT it doesn't return anything. I run it and it says "no recordset returned" what am I missing??
thanks, Lynda
quote:
A couple of problems I can see immediately:The code:
IF EXISTS(SELECT CustID FROM TblCustomers
WHERE EmailAddress = @x_Email)
Set @CustID = CustID
BEGIN...assume the previous SELECT returns custID, but it doesn't, it simply checks existence.
Also, the stuff from BEGIN onwards will always be executed, because the SET statement is the only one dependent on the IF.
An alternative is:
SELECT @CustID = CustID FROM TblCustomers
WHERE EmailAddress = @x_Email
IF @@ROWCOUNT > 0 -- Yes it exists and @CustID now contains the CustID
BEGIN
...
ENDCheers,
- Mark
-
Can you re-post the current code?
Cheers,
- Mark
Cheers,
- Mark -
It's fixed ... I had a return before my last select (the one that returns my current values) I pulled out the return and it works.
here is the current code:
create PROCEDURE dbo.StProc_ProcessCustomer
(
@x_Email VarChar(75),
@x_First_Name VarChar(20),
@x_Last_Name VarChar(30),
@x_address VarChar(50),
@x_City VarChar(50),
@x_state VarChar(30),
@x_Zip VarChar(10),
@x_Country VarChar(50),
@x_phone VarChar(30),
@x_fax VarCHar(30) = NULL,
@Password VarChar(20),
@SearchPhone VarChar(20),
@FullName VarChar(50)
)
AS BEGIN
SET NOCOUNT ON
DECLARE @CustID VarChar(10)
-- Make sure this is a new customer
SELECT @CustID = CustID FROM TblCustomers
WHERE EmailAddress = @x_Email
IF @@ROWCOUNT > 0
-- Yes it exists and @CustID now contains the CustID
BEGIN
UPDATE TblCustomers SET
FirstName = @x_first_name, LastName = @x_last_name,
Addressline1 = @x_address, City = @x_city,
State = @x_state, Zip = @x_zip,
Country = @x_country, Phone = @x_phone,
FaxNumber = @x_fax, CustPassword = @password,
SearchPhone = @SearchPhone
WHERE CustID = @CUstID
END -- IF @@ROWCOUNT > 0 ...
ELSE
BEGIN
DECLARE @NextCustNo VarChar(10)
-- get new customer number and insert new record
Select @NextCustNo = NextCustNo FROM TblMisc
Set @CustID = @NextCustNo
Update TblMisc SET NextCustNo = (NextCustNo + 1)
INSERT TblCustomers (
CustID, FirstName, LastName, Addressline1, City, FullName,
State, Zip, Country, Phone, ShipAddr1,
ShipCity, ShipState, ShipZip,
ShipCountry, FaxNumber, SearchPhone, EmailAddress,
NoOfSales, GrossSales, DiscountsUsed, Premiums, OneQuickClick,
HasOtherShipping, NoOfVisits, NoOfDownloads, AvaMP3,
SubsType, SubsCurrRenew, SubsNextRenew, SubsStart, CustPassword )
VALUES (
@CustID, @x_first_name, @x_last_name, @x_address, @x_city, @FullName,
@x_state, @x_zip, @x_country, @x_phone, @x_address,
@x_city, @x_state, @x_zip,
@x_country, @x_fax, @searchPhone, @x_email,
0, 0, 0, 0, 0,
0, 0, 0, 0,
'N', NULL, NULL, NULL, @Password )
END -- IF EXISTS(SELECT @CustID FROM TblCustomers ...
-- Return Results
SELECT CustID, EmailAddress
FROM TblCustomers
WHERE CustID = @CustID
SET NOCOUNT OFF
END -- ProcessCustomer
GO
quote:
Can you re-post the current code?Cheers,
- Mark
-
Good to see.
An observation though... you're using a separate table for the "next customer id". This has the potential to get out of sync or to cause locking/blocking problems, as I've seen happen.
A couple of alternatives:
a) Have TblCustomers.CustID as an INTEGER IDENTITY field, so the next value is automatically assigned.
b) Have your INSERT code select and increment the max CustID from the TblCustomers table itself, rather than another table. I would assume CustID is indexed (because you do an UPDATE .... WHERE CustID = @CUstID), so selecting MAX(CustID) should be fast.
c) Tell me to mind my own business
Cheers,
- Mark
Cheers,
- Mark -
A couple more comments. What is the reasoning behind using a varchar field for the NextCustNo and CustID fields? As Mark stated it seems these fields should be INTEGERS and at the very least the NextCustNo might be better as an identity column.
I would also make the @CustID parameter an output parameter so that you can pass up to the calling procedure/app the new CustID.
One more thing is that I'm surprised that you don't have this wrapped in a transaction. It would seem to me that you would want to roll this back if you have an error so you might want to check into transactions for this.
Gary Johnson
Microsoft Natural Language Group
DBA, Sr. DB Engineer
Gary Johnson
Microsoft Natural Language Group
DBA, Sr. DB EngineerThis posting is provided "AS IS" with no warranties, and confers no rights. The opinions expressed in this post are my own and may not reflect that of my employer.
-
I know, I know ... now. This is a system that is 4 years old and started life as an access database that I ported to SQL Server. It is also been a education for me. I just started using stored procedures, this is my third ... the first two were simple select procedures. This one was built because I saw the improved performance with just the two simple ones I did before. It aslo provided much more control over what was going on and a simpler way to code the ASP pages that use it. And you guys know all of this.
Yes, the CustID shold be an integer, and yes it should have been an Identity field. I have no clue how to take my existing data (145,000 customer records) and make it all good. I can't change the custID because these folks all have cookies that reference that. I'm trying to learn here and you guys have been wonderful.
quote:
A couple more comments. What is the reasoning behind using a varchar field for the NextCustNo and CustID fields? As Mark stated it seems these fields should be INTEGERS and at the very least the NextCustNo might be better as an identity column.I would also make the @CustID parameter an output parameter so that you can pass up to the calling procedure/app the new CustID.
One more thing is that I'm surprised that you don't have this wrapped in a transaction. It would seem to me that you would want to roll this back if you have an error so you might want to check into transactions for this.
Gary Johnson
Microsoft Natural Language Group
DBA, Sr. DB Engineer
-
oh, yeah ... I changed this procedure and added transaction code:
DECLARE @NextCustNo VarChar(10)
-- get new customer number and insert new record
BEGIN TRAN
Select @NextCustNo = NextCustNo FROM TblMisc
Set @CustID = @NextCustNo
Update TblMisc SET NextCustNo = (NextCustNo + 1)
COMMIT
quote:
A couple more comments. What is the reasoning behind using a varchar field for the NextCustNo and CustID fields? As Mark stated it seems these fields should be INTEGERS and at the very least the NextCustNo might be better as an identity column.I would also make the @CustID parameter an output parameter so that you can pass up to the calling procedure/app the new CustID.
One more thing is that I'm surprised that you don't have this wrapped in a transaction. It would seem to me that you would want to roll this back if you have an error so you might want to check into transactions for this.
Gary Johnson
Microsoft Natural Language Group
DBA, Sr. DB Engineer
-
I understand your problem of working with legacy systems. Sometimes there's a few warts that one just has to live with.
One final suggestion... that all of the update code (update Next Customer Number, and Insert new Customer row) is bound by the transaction.
Cheers,
- Mark
Cheers,
- Mark
Viewing 10 posts - 1 through 9 (of 9 total)
You must be logged in to reply to this topic. Login to reply