September 2, 2011 at 9:47 am
Hi,
I'm trying to research some performance issues that came up when we went to Win 7. I decided, after finding nothing, to use SQL Profiler to see what the commercial, third-party application was pulling from the database. Besides finding a LOT of "SELECT *"s (that's bad, right?) I found things like the snippet below. I've been away from intense SQL for a while, but have I lost some SQL chops, or is this a loop that won't ever loop and the update is a set-based update anyway? I've found this structure repeating in other sprocs, too. I tend to think there must be a reason for this and I'm not seeing it - please point out what I'm not seeing......
/* Update tableA */
Select @Count = count(*)
from tableA
where Id = @Id and
ColA <= convert(char(10), getdate(),101) and
ColB = 'x' and ColC > 0 and ColD= 'A'
While @Count <> 0
Begin
Update tableA
Set ColA = dateadd(wk, ColC, SomeDate)
Where Id = @Id and
ColA <= convert(char(10), getdate(),101) and
ColB = 'x' and ColC > 0 and ColD = 'A'
Select @Count = count(*)
from tableA
where Id = @Id and
ColA <= convert(char(10), getdate(),101) and
ColB = 'x' and ColC > 0 and ColD= 'A'
End
Thanks!
September 2, 2011 at 10:49 am
On face-value, it seems like the while-loop will only be executed once...unless there is something else that happens between the update (maybe triggers) and the select inside the loop.
Not sure why it would be done that way...seems a little impractical if there are no update-triggers on the table.
September 2, 2011 at 10:57 am
It's adding a number of weeks (ColC) to ColA, until ColA is caught up with the current date.
So, if ColA started out as a year ago, and ColC = 1, then it will loop 52 times. At that point, ColA > getdate(), and it will exit the loop.
There are easier ways to do this, and ones that are less CPU expensive, but it does look like it makes some sense potentially, depending on starting values of ColA and ColC.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
September 2, 2011 at 11:01 am
GSquared (9/2/2011)
It's adding a number of weeks (ColC) to ColA, until ColA is caught up with the current date.So, if ColA started out as a year ago, and ColC = 1, then it will loop 52 times. At that point, ColA > getdate(), and it will exit the loop.
There are easier ways to do this, and ones that are less CPU expensive, but it does look like it makes some sense potentially, depending on starting values of ColA and ColC.
Good catch...I missed that completely 🙂
September 2, 2011 at 11:48 am
Martin Schoombee (9/2/2011)
GSquared (9/2/2011)
It's adding a number of weeks (ColC) to ColA, until ColA is caught up with the current date.So, if ColA started out as a year ago, and ColC = 1, then it will loop 52 times. At that point, ColA > getdate(), and it will exit the loop.
There are easier ways to do this, and ones that are less CPU expensive, but it does look like it makes some sense potentially, depending on starting values of ColA and ColC.
Good catch...I missed that completely 🙂
Yes, me too - thanks! The software (and associated db) are baffling to me. No foreign keys, it seems to do "SELECT *"s on tables with 75+ columns, if you're searching for records on Person A at Location X, it first retrieves ALL Persons at Location X, then details for the first Person record in that ordered list, then moves on to get that same information for the Person you're actually querying on......I've done some SQL Profiling before, but all these (to me) unnecessary queries makes me wonder if I'm misreading the results-like I did for the loop.....
Thanks again!
September 6, 2011 at 6:24 am
Feel free to post queries here, and we'll try to see if we can figure out what/why on them.
Considering the first one, it looks like the code was probably built by someone much more comfortable with procedural code than with set-based solutions. There are patterns to watch out for if that's the case. Expect loops. Expect "premature optimization". Expect data integrity items like foreign keys, column/table constraints, possibly even strong data types, to be handled by the application (possibly in a business rules layer, possibly in a data access layer, hopefully not in the front-end). Also expect a lot of dynamic SQL, with the all the possibilities of injection; possibly "prevented" by searching inputs for "the forbidden characters". (Most procedural coders have yet to realize that this means the complex search functions they build will never be able to look up "O'Mally's Bar & Grill", since it'll strip out the apostophes and possibly even the ampersand. They also don't understand that it doesn't actually prevent SQL injection. It just makes it a tiny bit more complex.)
Those are the usual patterns that go along with code like your first sample. Easy enough to find and fix, but it does mean you'll have your work cut out for you. Good job security if you're good at fixing it!
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
Viewing 6 posts - 1 through 5 (of 5 total)
You must be logged in to reply to this topic. Login to reply