SQL Server Nation
For all of your SQL Server needs.

Help figureing out why this dosn't work.

rated by 0 users
Answered (Verified) This post has 1 verified answer | 13 Replies | 4 Followers

Tom Johnson posted on 21 Oct 2009 3:59 PM

DECLARE @ISPName VARCHAR(50)
SET @ISPName = 'ISP Name'
DECLARE @RecordID INT
BEGIN TRANSACTION
    SELECT @RecordID = (SELECT RecordID FROM l_ISPName (TABLOCKX) WHERE ISPName = @ISPName)
    
    --PRINT @@ROWCOUNT
    IF @@ROWCOUNT = 0
    BEGIN
        INSERT INTO l_ISPName (ISPName) VALUES (@ISPName)
        SELECT @RecordID = (SELECT RecordID from l_ISPName (TABLOCKX) WHERE ISPName = @ISPName)
    END
    COMMIT TRANSACTION
SELECT @RecordID

 

 

I'm really curious as to why if I remove the comment on the line "PRINT @@ROWCOUNT" the if statement will correctly evaluate if the prior select state returned 0 record and insert the information.  I changed the code to check the @RecordID and it works so it mainly is just trying to learn the error in my logic.

 

Answered (Verified) Verified Answer

The way you are setting @RecordID will result in @@rowcount being 1 no matter if there is a result or not. 

See this example:


declare @s int
select @s = (select 1 where 1=0)
select @@rowcount

The reason is because NULL is returned if the where clause is not met.  So your print @@rowcount was resetting @@rowcount to 0 which caused the insert to file.

See the full solution below.

 

DECLARE @ISPName VARCHAR(50),@rowcount int
SET @ISPName = 'ISP Name'
DECLARE @RecordID INT
BEGIN TRANSACTION
    SELECT @RecordID = RecordID FROM l_ISPName (TABLOCKX) WHERE ISPName = @ISPName
    set @rowcount = @@rowcount
    PRINT @ROWCOUNT
    IF @ROWCOUNT = 0
    BEGIN
        INSERT INTO l_ISPName (ISPName) VALUES (@ISPName)
        SELECT @RecordID = (SELECT RecordID from l_ISPName (TABLOCKX) WHERE ISPName = @ISPName)
    END
    COMMIT TRANSACTION
SELECT @RecordID

 

Co-Founder
SQL Server MVP
BrandonGalderisi
SQL Server Nation

All Replies

Because it must!

@@rowcount returns the number of rows effected by the last statement.  print statements will always have a 0 @@rowcount as shown by the sample SQL below.

 

select 1
select @@rowcount

print 'hi'
select @@rowcount

 

So if you want to display @@rowcount, you would be best served to declare a variable (ex. @rowcount), capture @@rowcount as @rowcount, then display and test @rowcount.

 

example:

 

DECLARE @ISPName VARCHAR(50),@rowcount int
SET @ISPName = 'ISP Name'
DECLARE @RecordID INT
BEGIN TRANSACTION
    SELECT @RecordID = (SELECT RecordID FROM l_ISPName (TABLOCKX) WHERE ISPName = @ISPName)
    select @rowcount = @@rowcount
    PRINT @ROWCOUNT
    IF @ROWCOUNT = 0
    BEGIN
        INSERT INTO l_ISPName (ISPName) VALUES (@ISPName)
        SELECT @RecordID = (SELECT RecordID from l_ISPName (TABLOCKX) WHERE ISPName = @ISPName)
    END
    COMMIT TRANSACTION
SELECT @RecordID

Co-Founder
SQL Server MVP
BrandonGalderisi
SQL Server Nation

A little back story and more info. 

I'm not wanting the print line to be there.  I was just trying to debug my code when I stuck it in there and it started working.

The table is 2 columns.  An identify column RecordID and a varchar(50) ISPName

The message result says that 1 row was affected which I'm guessing might be the select at the end.  The @RecordID is always null if the print line is commented which to me indicates 0 lines were selected so @@RowCount should be 0 and the if statement should be true.  However placing another print statement inside the if statement show this is always false unless the print line is uncommented.

 

I posted the code to create the table if someone wants to try recreating what I'm experiencing.

/****** Object:  Table [dbo].[l_ISPName]    Script Date: 10/22/2009 11:38:58 ******/
SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

SET ANSI_PADDING ON
GO

CREATE TABLE [dbo].[l_ISPName](
    [RecordID] [int] IDENTITY(1,1) NOT NULL,
    [ISPName] [varchar](50) NOT NULL
) ON [PRIMARY]

GO

SET ANSI_PADDING OFF
GO


The way you are setting @RecordID will result in @@rowcount being 1 no matter if there is a result or not. 

See this example:


declare @s int
select @s = (select 1 where 1=0)
select @@rowcount

The reason is because NULL is returned if the where clause is not met.  So your print @@rowcount was resetting @@rowcount to 0 which caused the insert to file.

See the full solution below.

 

DECLARE @ISPName VARCHAR(50),@rowcount int
SET @ISPName = 'ISP Name'
DECLARE @RecordID INT
BEGIN TRANSACTION
    SELECT @RecordID = RecordID FROM l_ISPName (TABLOCKX) WHERE ISPName = @ISPName
    set @rowcount = @@rowcount
    PRINT @ROWCOUNT
    IF @ROWCOUNT = 0
    BEGIN
        INSERT INTO l_ISPName (ISPName) VALUES (@ISPName)
        SELECT @RecordID = (SELECT RecordID from l_ISPName (TABLOCKX) WHERE ISPName = @ISPName)
    END
    COMMIT TRANSACTION
SELECT @RecordID

 

Co-Founder
SQL Server MVP
BrandonGalderisi
SQL Server Nation

declare @s int
select @s = (select 1 where 1=0)
select @@rowcount

Thats what the issue was.  Didn't realize it would never return any thing other then 0

SELECT @RecordID = RecordID FROM l_ISPName (TABLOCKX) WHERE ISPName = @ISPName

That is the line that solved my problem, didn't know you could do an assignment inside a select statement.  I'll try to keep that in mind for future use.

 

 

You almost always want to do it that way.  Because if for some reason there were ever two records that matched your ISPName, you would get the following error.

Msg 512, Level 16, State 1, Line 3
Subquery returned more than 1 value. This is not permitted when the subquery follows =, !=, <, <= , >, >= or when the subquery is used as an expression.

Here is a statement that will show the error.

declare @s int
select @s = (select 1 union all select 0)

 

Co-Founder
SQL Server MVP
BrandonGalderisi
SQL Server Nation

Guess I've not ever come across that issue before.  I went back and reviewed the select vs set portion of my sql book because I knew that one of the differences between select vs set.  Turns out it did show select being used like you said.  Guess I just forgot some of the basics after a few years. :)

No problem.  That's why we're here to help.  Keep us in mind and tell your co-workers :).

Co-Founder
SQL Server MVP
BrandonGalderisi
SQL Server Nation

Hello.  I see that an answer has already been given but I think there are a few additional points to be made that can be helpful.

  1. From a logical point of view it might make more sense to test for the value of @RecordID that comes back from the SELECT (a direct indicator of success / failure) rather than testing for @@ROWCOUNT (an indirect indicator).  The value of @RecordID is NULL upon declaration and will be NULL if no rows are returned by the current SELECT syntax OR if a single row with a NULL value is returned by your original SELECT syntax.  Hence doing IF (@RecordID IS NULL) will be more reliable than IF (@@ROWCOUNT = 0).  However, given that doing a SELECT @Var = (SELECT ...) is a bit awkward, I would still recommend using the SELECT @Var .. FROM Table syntax that Brandon recommended. 
  2.  I would highly recommend using SCOPE_IDENTITY() to get the newly created @RecordID value as opposed to doing a full SELECT on the table since the value will be the same and SCOPE_IDENTITY() is already in memory and does not require going back to disk which will take longer.  Since you are already doing an exclusive lock on the table, the SELECT will only cause the transaction to take longer increasing the chances for blocking on the locked resource I_ISPName. 
  3. Minor point but starting in SQL Server 2005, Table Hints should use the WITH keyword, as in: WITH (TABLOCKX)

 

So, the modified solution is as follows (note I removed the @rowcount variable as it is no longer needed):

DECLARE @ISPName VARCHAR(50)
SET @ISPName = 'ISP Name'
DECLARE @RecordID INT

BEGIN TRANSACTION
    SELECT @RecordID = RecordID FROM l_ISPName WITH (TABLOCKX) WHERE ISPName = @ISPName

    IF (@RecordID IS NULL)
    BEGIN
        INSERT INTO l_ISPName (ISPName) VALUES (@ISPName)
        SET @RecordID = SCOPE_IDENTITY()
    END
COMMIT TRANSACTION

SELECT @RecordID AS [RecordID]

 

Take care, Solomon...

1. Testing NULL for @RecordID will require that you set it to NULL after every insert.

 

2. You are correct.  It would be much faster to use scope_identity().  When looking at the original question, I merely explained why removing the print fixed the test of @@rowcount.  I didn't even pay attention to how they were retreiving @recordID after insert.

Co-Founder
SQL Server MVP
BrandonGalderisi
SQL Server Nation

2)  In my production code I did use @@IDENTITY vs the select statement I used in the example.  I was unaware of the SCOPE_IDENTITY().  Since its a better tool for what I wan't I'll go ahead and make the change in my code.

 

Thanks

You definitely want to use scope_identity() over @@identiy.  @@Identity can return the value of an identity column from a different table if triggers are used for auditing.

Co-Founder
SQL Server MVP
BrandonGalderisi
SQL Server Nation

:)  I looked up the difference when Solomon suggested I use that instead.  I havn't put any triggers in yet but could see my self scratching my head trying to figure out why it broke when I put a trigger on the table if I ever did.  As you can probably tell db development is not my primary field but I've had fun working on this project as I've got to put to practice some of what I've read and subsequently forgoten about.

Page 1 of 1 (14 items) | RSS
Copyright SQL Server Nation 2010
Powered by Community Server (Non-Commercial Edition), by Telligent Systems