1

trying to get this stored procedure to work.

ALTER PROCEDURE [team1].[add_testimonial]
-- Add the parameters for the stored procedure here
@currentTestimonialDate char(10),@currentTestimonialContent varchar(512),@currentTestimonialOriginator varchar(20)
AS
BEGIN
DECLARE
@keyValue int

SET NOCOUNT ON;
--Get the Highest Key Value
SELECT @keyValue=max(TestimonialKey)
FROM Testimonial
--Update the Key by 1
SET @keyValue=@keyValue+1
--Store into table
INSERT INTO Testimonial VALUES (@keyValue, @currentTestimonialDate, @currentTestimonialContent, @currentTestimonialOriginator)

END

yet it just returns

Running [team1].[add_testimonial] ( @currentTestimonialDate = 11/11/10, @currentTestimonialContent = this is a test, @currentTestimonialOriginator = theman ).

No rows affected.
(0 row(s) returned)
@RETURN_VALUE = 0
Finished running [team1].[add_testimonial].

and nothing is added to the database, what might be the problem?

Tim Lehner
  • 14,813
  • 4
  • 59
  • 76
adangert
  • 36
  • 1
  • 5
  • 1
    What is the table structure for Testimonial. Also, make TestimonialKey an identity column and you don't have to --Get the Highest Key Value SELECT @keyValue=max(TestimonialKey) FROM Testimonial --Update the Key by 1 SET @keyValue=@keyValue+1 – Johnie Karr Nov 28 '11 at 02:52
  • Are there records in the table? Write out @keyvalue to make sure it has a value – Michael C. Gates Nov 28 '11 at 02:56
  • 1
    `SELECT @keyValue=max(TestimonialKey) FROM Testimonial --Update the Key by 1 SET @keyValue=@keyValue+1` well I hope you only have one user running this at any given time because this is a nasty race condition. – Conrad Frix Nov 28 '11 at 03:06
  • 2
    You need to do this in a transaction with a table lock or it won't work if more than one user is running the SP. I think you should just make the key autoincrementing. – Hogan Nov 28 '11 at 03:09
  • Based on how your output looks, you're running this through some code that you wrote (.NET or somesuch). What happens if you run the procedure through Management Studio instead? – Ben Thul Nov 28 '11 at 04:06
  • You are setting the row count to OFF in your stored proc: `SET NOCOUNT ON;` so even if any rows would be affected, you're not getting told that this is the case..... – marc_s Nov 28 '11 at 05:57

2 Answers2

1

There may have problems in two place:

a. There is no data in the table so, max(TestimonialKey) returns null, below is the appropriate way to handle it.

--Get the Highest Key Value
SELECT @keyValue= ISNULL(MAX(TestimonialKey), 0)
FROM Testimonial
--Update the Key by 1
SET @keyValue=@keyValue+1

b. Check your data type of the column currentTestimonialDate whether it is char or DateTime type, if this field is datetime type in the table then convert @currentTestimonialDate to DateTime before inserting to the table.

Also, check number of columns that are not null allowed and you're passing data to them.

If you're not passing data for all columns then try by specifying columns name as below:

--Store into table
INSERT INTO Testimonial(keyValue, currentTestimonialDate, 
                       currentTestimonialContent, currentTestimonialOriginator) 
              VALUES (@keyValue, @currentTestimonialDate, 
                     @currentTestimonialContent, @currentTestimonialOriginator)

EDIT:

After getting the comment from marc_s:

Make keyValue as INT IDENTITY, If multiple user call it concurrently that wont be problem, DBMS will handle it, so the ultimate query in procedure might be as below:

ALTER PROCEDURE [team1].[add_testimonial]
-- Add the parameters for the stored procedure here
@currentTestimonialDate char(10),
@currentTestimonialContent  varchar(512),@currentTestimonialOriginator varchar(20)
AS
BEGIN

  SET NOCOUNT ON;
  --Store into table
  INSERT INTO Testimonial VALUES (@currentTestimonialDate, 
        @currentTestimonialContent, @currentTestimonialOriginator)

END
Elias Hossain
  • 4,410
  • 1
  • 19
  • 33
  • And the **really** proper way to handle this `KeyValue` would be to make it an `INT IDENTITY` and let the database deal with incrementing it.... this approach here could fail under concurrent load with multiple clients getting the same "new" `KeyValue` ... – marc_s Nov 28 '11 at 05:57
0

Two issues that I can spot:

SELECT @keyValue=max(TestimonialKey)

should be

SELECT @keyValue=ISNULL(max(TestimonialKey), 0)

To account for the case when there are no records in the database

Second, I believe that with NOCOUNT ON, you will not return the count of inserted rows to the caller. So, before your INSERT statement, add

SET NOCOUNT OFF
competent_tech
  • 44,465
  • 11
  • 90
  • 113