Member Avatar for ansari.wajid

Hi All

I am writting an stored procedure to get the recently inserted records primary key as follows:

ALTER    PROCEDURE [DBO].[AddEditUserMsgDetails]
(
@iJobID [INT]=NULL,
@iUserID [INT]=NULL,
@vFromName [VARCHAR](50)=NULL,
@vFromAddress [VARCHAR](50)=NULL,
@vSubject  [VARCHAR](50)=NULL,
@iListID [INT]=NULL,
@vActualText [VARCHAR](100)=NULL
)
AS
BEGIN
/* 
   Logic Comments: INSERTING OR UPDATING INTO THE TABLE ezlMailJobs
   EXEC AddEditUserMsgDetails null,505,'waa','waa@v-empower.com','Test',816,
   '<html><body>Hi &lt;&#33;FN&gt;<br>please ignore</body></html></body></html>'
   
*/
 
  IF @iJobId IS NULL OR @iJobId=0
     BEGIN
        INSERT INTO ezlMailJobs (iUserID,vFromName,vFromAddress,vSubject,iListID,vActualText)
        VALUES (@iUserID,@vFromName,@vFromAddress,@vSubject,@iListID,@vActualText)
	set @iJobID= @@Identity
	select @iJobID
     END
   IF @iJobId IS NOT NULL
      BEGIN
         UPDATE ezlMailJobs SET iUserID=@iUserID,vFromName=@vFromName,vFromAddress=@vFromAddress,
         vSubject=@vSubject,iListID=@iListID,vActualText=@vActualText
         WHERE iJobId=@iJobId
      END
   IF @@ERROR=0
      BEGIN
         COMMIT TRAN
         SELECT 1
      END
   ELSE
      BEGIN
         ROLLBACK TRAN
         SELECT -1
      END

As in the above stored procedure when ijobid is null or zero, It should execute insert statement and return recently inserted records ijobid else it should execute update statement. But as now it is executing insert statement and returning some garbage value. one more thing I have declared ijobid as identity.
I am unable to figure out the problem. Please help me.
Thanks
WAA

  • 2 Contributors
  • 1 Reply
  • 219 Views
  • 12 Hours Discussion Span
  • Latest Post Latest Post by dickersonka

Dani AI

Generated

Short answer: the “wrong” identity usually comes from using @@IDENTITY (affected by triggers and other scopes), from returning multiple result sets (your INSERT block does an early SELECT, then later you SELECT status), and from the logic flow — after the INSERT you set the id and then the separate UPDATE block runs because there is no ELSE. Also your code checks @@ERROR/COMMIT without a clear BEGIN TRAN / TRY–CATCH; that pattern is fragile.

Practical, minimal changes to make the procedure reliable:

  • Make the id parameter an OUTPUT parameter so the caller gets the new id reliably.
  • Suppress rowcount messages with SET NOCOUNT ON.
  • Capture the newly generated id with SCOPE_IDENTITY() (safe for the current scope) or use the INSERT ... OUTPUT clause if you prefer returning rows directly.
  • Ensure the INSERT and UPDATE are mutually exclusive (use IF...ELSE) so you don’t insert then immediately update the same row.

Small illustrative snippet (not the same code shown earlier in the thread):

-- declare the param as OUTPUT in the proc signature:
@iJobID INT OUTPUT

-- after a successful INSERT capture the id:
SET @iJobID = SCOPE_IDENTITY()

Better robustness: wrap the DML in TRY/CATCH with an explicit BEGIN TRAN; on success COMMIT, on error ROLLBACK and THROW/RAISERROR so callers get a clear failure. Do not rely on RETURN to deliver the new identity — RETURN is for status codes (as hinted), and @@IDENTITY is unsafe if triggers exist. Finally, ensure your procedure returns exactly one predictable result (either an OUTPUT param or a single SELECT), so client code won’t read a “garbage” value from an unexpected resultset.

Member Avatar for dickersonka

first you are missing an else on your first if checking jobid

after the insert use,

return @@IDENTITY

from codeside add a parameter with

ParameterDirection.ReturnValue;
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.