Tuesday, March 20, 2012

Re: Stored procedure not executing correctly

Hi everyone,

I am having trouble with this stored procedure. The @.curr_type stores the first value of inspection type of the recordset , @.inspec_type stores the current record of the inspection type. @.inspec_type moves to the next record while @.curr_type's value remain the same for comparisons.

The error is in the Else statment. These variables,@.curr_type and @.inspec_type, values are = 3 the else statement should not be executed.

Is something wrong with my code or logic?
Thank you for any assistance.Mind posting some code ?|||Yeah, without seeing it you want us to guess? Well, I guess that the ELSE is not executed because a check for a value of the variable does not take into account a possibility of NULL. Good enough, hey?! ;)|||so Sorry! absent minded me.

Here is my code:

CREATE PROCEDURE dbo.spCheck_inspection_type
@.inspec_type smallint Out, @.curr_type smallint Out
AS--ve

declare insp_type cursor

For
Select inspection_type
From tblTmp_inspec
Where partial <> 0 or complete <> 0;
set @.curr_type = 0;

Open insp_type
Fetch Next From insp_type Into @.inspec_type

While @.@.FETCH_STATUS = 0
Begin
if @.curr_type = 0
set @.curr_type = @.inspec_type;
Else
--last inspection type is @.curr_type
Begin
if @.curr_type <> @.inspec_type
close insp_type
deallocate insp_type
return(1);
End

Fetch Next From insp_type Into @.inspec_type

End

close insp_type
deallocate insp_type

return(0);
GO

The ELSE statment executed eventhough both variables 's values is 3 . Why?

Thank you much!!|||Indeting is SO overlooked...

You have logic problems...I guess they don't call them scop termintors here...

BUT...for now, to make it easier to read, use a BEGIN and END for every logic block...AND make sure to match them up...

BUT...This is WAY to over done for what you're trying to do...which is?

Here, look at this

CREATE PROC spCheck_inspection_type
@.inspec_type smallint OUT, @.curr_type smallint OuT
AS
DECLARE insp_type CURSOR
FOR
SELECT inspection_type FROM tblTmp_inspec WHERE partial <> 0 or complete <> 0;
SET @.curr_type = 0;

Open insp_type
FETCH NEXT FROM insp_type INTO @.inspec_type

WHILE @.@.FETCH_STATUS = 0
BEGIN
IF @.curr_type = 0
SET @.curr_type = @.inspec_type;
ELSE
BEGIN
IF @.curr_type <> @.inspec_type
CLOSE insp_type
DEALLOCATE insp_type
RETURN 1
END
FETCH NEXT FROM insp_type INTO @.inspec_type
END

CLOSE insp_type
DEALLOCATE insp_type

RETURN 0
GO|||That's too many colors,Brett, andno comment on what they mean!

All the guy is missing is BEGIN...END on the test for @.curr_type <> @.inspec_type: if @.curr_type <> @.inspec_type begin
close insp_type
deallocate insp_type
return(1);
end|||fine...fine...fine...

BUT!

Would you do this?

I mean, what does it even mean?|||Yup, there are problems with the code. Most obvious one is that the cursor declaration does not take into account the order of records as they get returned. This would yield unpredictable results when the app is running on a single CPU machine while being tested, compared to a prod environment when it gets deployed onto a SMP system.

To avoid usage of cursor, while fixing the ORDERing issue, would be to do the following:

declare @.curr_type int, @.record_id int
select @.curr_type = inspection_type, @.record_id = <record_id> from (
select top 1 <record_id>, inspection_type from tblTmp_inspec
WHERE partial <> 0 or complete <> 0
order by <record_id>) x

if exists (select 1 from tblTmp_inspec where <record_id> > @.record_id
and partial <> 0 or complete <> 0 and inspection_type <> @.curr_type)
return 1
else
return 0|||Brett, that was the most beautiful piece of SQL I've ever seen.

I am reminded of the Old Testament story of Joseph and his "Code of many colors". It must have looked something like that.

:)|||Brett, that was the most beautiful piece of SQL I've ever seen.
:)

And you know that's saying something...because he's, well, blind

Thanks Dude...

Seriously, alicejwz, Lettuce know what you're doing, and we can hook you up.

No comments:

Post a Comment