Why Do I Keep Seeing This Mistake?

I really enjoy seeing variable declarations inside the loop, since BEGIN...END doesn't change the scope for variables:

BEGIN

 DECLARE @message VARCHAR(25);

 SET @message = REPLICATE( '-', 25 )

END

SET @message = 'abcdefg';

The declarations don't really affect performance much, but the code is just plain confusing to anyone coming from any other language on the planet!

The code

SET  @counter = 0, @separator = REPLICATE( '-', 25 )

should be

SELECT @counter = 0, @separator = REPLICATE( '-', 25 )

Frankly, I am at a loss to explain why you would use loops at all for this, when it can easily be done in a set-based manner:

--====== SET-Based:

SET NOCOUNT ON

;WITH cteNumbers as (

SELECT TOP(100)

ROW_NUMBER() OVER(ORDER BY object_id) as N

FROM sys.system_columns

ORDER BY object_id

)

SELECT Str( N, 10 ) + Str( SQRT( CONVERT( FLOAT, N ) ), 10, 4 )

FROM cteNumbers

WHERE N <= 100

Or, if you really need the separator lines:

--====== SET-Based w separators:

SET NOCOUNT ON

DECLARE @separator VARCHAR(25)

SET  @separator = REPLICATE( '-', 25 )

;WITH cteNumbers as (

SELECT TOP(200)

ROW_NUMBER() OVER(ORDER BY object_id) as N

FROM sys.system_columns

ORDER BY object_id

)

SELECT

CASE WHEN N%2=1 THEN @separator

ELSE Str( N/2, 10 ) + Str( SQRT( CONVERT( FLOAT, N/2 ) ), 10, 4 )

END

FROM cteNumbers

WHERE N <= 200

select CONVERT(varchar(28), getdate(), 121) as [End Set2]

Both of these take 0 msec on my system, so that seems like an even better optimization.

Hi Kevin,

It does not work for me:

DECLARE @i INT, @j INT;

SET @i=1, @j=2;

Msg 102, Level 15, State 1, Line 2

Incorrect syntax near ','.

SQL 2005

Hey Kevin,

Madhivanan and Alexander are right; it's a typo in your second example.  You can't use SET to assign values to multiple variables in T-SQL (at least not since SQL 7).

As to why experienced programmers make this mistake, I think Barry's response is an indicator; good T-SQL programmers are unaccustomed to working with loops.  They try to find ways around them as best they can, and when they succumb to using a looping method, it's not easy to see the obvious errors.  Forest for the trees sort of thing.  At least that's my opinion.

Good post.

Stu

I'm saying the fact that BEGIN...END doesn't define a scope for variables can be extremely confusing, especially when you're coming from a language with code-grouping constructs that do define scope ("{"..."}" in C-style languages, for instance).  I wouldn't expect variable declarations to affect performance by any noticeable amount, but the fact that you can put variable declarations in a BEGIN...END construct and access them outside of the BEGIN...END can make for some confusing code.

You have my vote on the set based solution; however, may I add, since we are on the topic of best programming practices, that we create a table of numbers for computational purposes such as this, instead of going to the sys.system_columns for an object_id, which has no relevance to the solution at hand.

Leading on from what Mike said about scope , try this.

This had me scratching my head for a while on a bug

declare @LoopCount integer

Select @LoopCount = 0

while(@LoopCount <10)

begin

  Declare @TestTab table(

     col integer not null

  )

  insert into @TestTab values(@LoopCount)

  Select * from @TestTab

  Select @LoopCount = @LoopCount+1

end

Back to you main point , though , long gone is the thought that "Every cycle is sacred".  More prevelant is the attitude that the computer is so powerful you dont have to worry about it.

Still it seperates the men from the boys <G>

Although I understand that the original intent of this post was to explain how to optimize loops a bit, I'm with Barry... the best way to optimize a loop is to get rid of the loop.  Then you don't have to worry about errors like using SET for more than one variable assignement ;-)

Here's the set based version complete with separator lines...

SELECT '-------------' + CHAR(10)

     + STR(t.Number,3,0)

     + STR(SQRT(t.Number),10,4) AS [Square roots for 1 to 100]

  FROM Master.dbo.spt_Values t --A Tally table would work here

 WHERE t.Number BETWEEN 1 and 100

   AND t.Type = 'P'

Also, sys.SysColumns isn't being used as a source of "ready made numbers" in Barry's code.  sys.SysColumns is nothing more than a source of rows for the Row_Number to work against.  There is no guarantee that the numbers in sys.SysColumns will be contiguous and that's why the Row_Number was needed.  It's a time honored way of genning a Tally table on the fly.

As to why people continue to make the mistake of including what should be a static operation in a loop, consider this... people still use loops for such simple things... did you expect them to actually be able to optimize the loop when they can't avoid the loop to begin with? ;-)

I agree that the example uses a loop when a set based solution is entirely better.  That's not Kevin's point.  Having redundant initialization inside a loop, any loop, is a mistake.  It happens in all languages.

Could be cust-and-paste error or not knowing the scope rule for you language.  Still Kevin has a good point and is getting chewed for choosing an iconic example.

I do agree ditchig the loop is the better solution.

But I also recognise the fact that the compiler should be smarter and simple be able to do those 'place out of loop', 'vanish loop' operations by itself. Just like C/C++ compilers are able to do, I also very much dislike the current query cache plan implementation and lack of control over the optimizer. It can be made SO much better.

For pointers follow what is done in compilers and in processors branch prediction and caching logic. If it can't be quickly done in the first pass, have a low priority background thread optimize it several more passed, like hotspot optimizations do for JIT compilers.

If a batch or statement is executed 1000 times over a day, surely spending a bit of time optimizing in the background instead of using the initial plan would be preferable and help overal performance. There is nothing magic to it, the functional meaning however bad written is fixed and thus must be transformable to better code.

If we humans can do it, the CPU can do it better with the insane logic capacities and speed they have these days. This is no excuse to write code that does not express function clearly, but today its often the other way around. People having to test several solutions in order to pick the one the optimizer handles best. Experience helps in getting it done faster, but it is still an excersise that distracts from the real thing, writing functional clear and precise code and have the computer do the rest.

Before I jump onto the Goals and Themeword meme started by my buddy, Thomas LaRock ( blog | twitter ),