Plase help

Hello,
I have a developer who developed the following stored procedure.
My question is since it started with a Begin Transaction and if the record
exists in the EXISTS statement and it will return an ID without commit /
rollback, does he need to either commit or rollback the transaction?
begin transaction
if EXISTS(select OpportunityId from Opportunity where OTN = @.OTN and
CustomerName = @.CustomerName and ResponseDate = @.ResponseDate and CreateDate
= convert(char(11),getdate(),106))
Begin
Select @.OpportunityId = OpportunityId from Opportunity where OTN = @.OTN
and CustomerName = @.CustomerName and ResponseDate = @.ResponseDate and
CreateDate = convert(char(11),getdate(),106)
Return @.OpportunityId
End
else
Begin
Insert into Opportunity (OTN, CustomerName, ResponseDate) VALUES (@.OTN,
@.CustomerName, @.ResponseDate)
If @.@.Error <> 0
Begin
Rollback Transaction
Return -1
End
RETURN SCOPE_IDENTITY( )
Commit Transaction
End
GOEd,
Try moving the BEGIN TRAN down in the code to just prior the DML (INSERT)
statment.
HTH
Jerry
"Ed" <Ed@.discussions.microsoft.com> wrote in message
news:BA6A49DF-BC78-412D-BC86-9D456BFDDBE1@.microsoft.com...
> Hello,
> I have a developer who developed the following stored procedure.
> My question is since it started with a Begin Transaction and if the
> record
> exists in the EXISTS statement and it will return an ID without commit /
> rollback, does he need to either commit or rollback the transaction?
>
>
> begin transaction
> if EXISTS(select OpportunityId from Opportunity where OTN = @.OTN and
> CustomerName = @.CustomerName and ResponseDate = @.ResponseDate and
> CreateDate
> = convert(char(11),getdate(),106))
> Begin
> Select @.OpportunityId = OpportunityId from Opportunity where OTN = @.OTN
> and CustomerName = @.CustomerName and ResponseDate = @.ResponseDate and
> CreateDate = convert(char(11),getdate(),106)
> Return @.OpportunityId
> End
> else
> Begin
> Insert into Opportunity (OTN, CustomerName, ResponseDate) VALUES (@.OTN,
> @.CustomerName, @.ResponseDate)
> If @.@.Error <> 0
> Begin
> Rollback Transaction
> Return -1
> End
> RETURN SCOPE_IDENTITY( )
> Commit Transaction
> End
>
> GO|||There is only one insert statement that I can see. Transaction is not
necessary at all in this case.
It is also not necessary to perform the same query two times in a row...
select the value once, then check to see if it is null.
SELECT @.OpportunityId = OpportunityId FROM Opportunity WHERE OTN = @.OTN
AND CustomerName = @.CustomerName AND ResponseDate = @.ResponseDate AND
CreateDate = convert(char(11),getdate(),106)
IF @.OpportunityID IS NOT NULL Return @.OpportunityId
ELSE
BEGIN
INSERT INTO Opportunity (OTN, CustomerName, ResponseDate) VALUES (@.OTN,
@.CustomerName, @.ResponseDate)
IF @.@.Error <> 0 RETURN -1
ELSE RETURN SCOPE_IDENTITY()
END
END
GO
John Scragg
"Ed" wrote:

> Hello,
> I have a developer who developed the following stored procedure.
> My question is since it started with a Begin Transaction and if the reco
rd
> exists in the EXISTS statement and it will return an ID without commit /
> rollback, does he need to either commit or rollback the transaction?
>
>
> begin transaction
> if EXISTS(select OpportunityId from Opportunity where OTN = @.OTN and
> CustomerName = @.CustomerName and ResponseDate = @.ResponseDate and CreateDa
te
> = convert(char(11),getdate(),106))
> Begin
> Select @.OpportunityId = OpportunityId from Opportunity where OTN = @.OTN
> and CustomerName = @.CustomerName and ResponseDate = @.ResponseDate and
> CreateDate = convert(char(11),getdate(),106)
> Return @.OpportunityId
> End
> else
> Begin
> Insert into Opportunity (OTN, CustomerName, ResponseDate) VALUES (@.OTN,
> @.CustomerName, @.ResponseDate)
> If @.@.Error <> 0
> Begin
> Rollback Transaction
> Return -1
> End
> RETURN SCOPE_IDENTITY( )
> Commit Transaction
> End
>
> GO|||However, the is a risk that the row is deleted after the SELECT and before t
he INSERT. To eliminate
that risk, isolation SERIALIZABLE should be used, which requires a transacti
on (to do its work)...
:-)
Also, the code doesn't catch if the SELECT returns several rows. This is why
I prefer SET @.var =
(SELECT ...). A run-time error will be raised if SELECT returns > 1 row.
Tibor Karaszi, SQL Server MVP
http://www.karaszi.com/sqlserver/default.asp
http://www.solidqualitylearning.com/
Blog: http://solidqualitylearning.com/blogs/tibor/
"John Scragg" <JohnScragg@.discussions.microsoft.com> wrote in message
news:969D558F-CCA2-4E38-A87F-2B52073A707D@.microsoft.com...
> There is only one insert statement that I can see. Transaction is not
> necessary at all in this case.
> It is also not necessary to perform the same query two times in a row...
> select the value once, then check to see if it is null.
> SELECT @.OpportunityId = OpportunityId FROM Opportunity WHERE OTN = @.OTN
> AND CustomerName = @.CustomerName AND ResponseDate = @.ResponseDate AND
> CreateDate = convert(char(11),getdate(),106)
> IF @.OpportunityID IS NOT NULL Return @.OpportunityId
> ELSE
> BEGIN
> INSERT INTO Opportunity (OTN, CustomerName, ResponseDate) VALUES (@.OTN,
> @.CustomerName, @.ResponseDate)
> IF @.@.Error <> 0 RETURN -1
> ELSE RETURN SCOPE_IDENTITY()
> END
> END
> GO
> John Scragg
> "Ed" wrote:
>|||Tibor, you make some good points. I am afraid I was not as clear and the
formatting of SQL in the newsgroup does not help :-) Let me try to be more
clear.
The original proc was apparently designed to perform 1 of 2 action. Either
(1) return the ID of an existing row, or (2) in the absance of a row, insert
a new row and return the ID of the new row.
If the row is deleted after the select it should not matter (as far as the
proc is concerned) since there is nothing else happening. If the select
returns a value, the proc returns the coulmn value selected and the INSERT
statement is ignored. The way it was written originally there were two
identical selects. First to check IF EXISTS and then the same query to get
the value. This did provide a tiny window for a row to be deleted between
the two selects, but my opinion was that it is simply ineffcient to query
twice for the same result :-)
As for the possible error on multiple rows, you are absolutely right. It
can error. It is the same as the developer's original code, and so I am
assuming that they have confirmed the values in the where clause will provid
e
a unique row (through constraints, rules, etc.). If not, then he has this
problem in his original code as well.
John
"Tibor Karaszi" wrote:

> However, the is a risk that the row is deleted after the SELECT and before
the INSERT. To eliminate
> that risk, isolation SERIALIZABLE should be used, which requires a transac
tion (to do its work)...
> :-)
> Also, the code doesn't catch if the SELECT returns several rows. This is w
hy I prefer SET @.var =
> (SELECT ...). A run-time error will be raised if SELECT returns > 1 row.
> --
> Tibor Karaszi, SQL Server MVP
> http://www.karaszi.com/sqlserver/default.asp
> http://www.solidqualitylearning.com/
> Blog: http://solidqualitylearning.com/blogs/tibor/
>
> "John Scragg" <JohnScragg@.discussions.microsoft.com> wrote in message
> news:969D558F-CCA2-4E38-A87F-2B52073A707D@.microsoft.com...
>|||That is a good point that I never think about...
How am I able to control/put the Transaction to even lock the Select
statement to make sure when the record/ID is returned, there is no one else
delete the record already?
Thanks
Ed
"Tibor Karaszi" wrote:

> However, the is a risk that the row is deleted after the SELECT and before
the INSERT. To eliminate
> that risk, isolation SERIALIZABLE should be used, which requires a transac
tion (to do its work)...
> :-)
> Also, the code doesn't catch if the SELECT returns several rows. This is w
hy I prefer SET @.var =
> (SELECT ...). A run-time error will be raised if SELECT returns > 1 row.
> --
> Tibor Karaszi, SQL Server MVP
> http://www.karaszi.com/sqlserver/default.asp
> http://www.solidqualitylearning.com/
> Blog: http://solidqualitylearning.com/blogs/tibor/
>
> "John Scragg" <JohnScragg@.discussions.microsoft.com> wrote in message
> news:969D558F-CCA2-4E38-A87F-2B52073A707D@.microsoft.com...
>|||Sorry, John. I misinterpreted the logic. It looked like me as if the row exi
sts, get the values
*and* do an insert. I didn't read the original code carefully enough. I catc
hed that you picked up
on the "read twice" issue, but thought that a hole was left as I assumed ano
ther logic. My bad...
Tibor Karaszi, SQL Server MVP
http://www.karaszi.com/sqlserver/default.asp
http://www.solidqualitylearning.com/
Blog: http://solidqualitylearning.com/blogs/tibor/
"John Scragg" <JohnScragg@.discussions.microsoft.com> wrote in message
news:7C58CD9A-0D47-4D4D-A68B-15F5D64D65B2@.microsoft.com...
> Tibor, you make some good points. I am afraid I was not as clear and the
> formatting of SQL in the newsgroup does not help :-) Let me try to be mor
e
> clear.
> The original proc was apparently designed to perform 1 of 2 action. Eithe
r
> (1) return the ID of an existing row, or (2) in the absance of a row, inse
rt
> a new row and return the ID of the new row.
> If the row is deleted after the select it should not matter (as far as the
> proc is concerned) since there is nothing else happening. If the select
> returns a value, the proc returns the coulmn value selected and the INSERT
> statement is ignored. The way it was written originally there were two
> identical selects. First to check IF EXISTS and then the same query to ge
t
> the value. This did provide a tiny window for a row to be deleted between
> the two selects, but my opinion was that it is simply ineffcient to query
> twice for the same result :-)
> As for the possible error on multiple rows, you are absolutely right. It
> can error. It is the same as the developer's original code, and so I am
> assuming that they have confirmed the values in the where clause will prov
ide
> a unique row (through constraints, rules, etc.). If not, then he has this
> problem in his original code as well.
> John
> "Tibor Karaszi" wrote:
>