July 23, 2012 at 5:42 pm
Hi,
I'm just looking for either justification that I'm not going crazy and know a little bit about SQL, or either a lesson that's there's sometimes a good reason for some SQL that I think is giving us a problem.
This is a third party app, I got the SQL from running Profiler while using the function that one of my users identified as running extremely slowly after an application upgrade. First thing that popped out at me was this (a.submitted is a datetime column):
I'm not sure how they're getting the date string, it's part of the record that's displayed on the screen. You'd think they could use the PK of the row..... But anyway.....
a.) converting a date string literal to a varchar (with a date format to boot). Sort of redundant?
b.) converting a date to varchar to compare with another varchar instead of converting literal to datetime - not efficient?
c.) using a function on left side of operator - db can't use index, if one exists, on that column.
Running the query in Management Studio as is took 41 seconds. Taking out the first convert (on the left side) and casting the literal to datetime took <1 second. Their support wrote back to me with a script to add some indexes on some columns. Of course, the columns weren't in this particular query, but hey, adding an index must be good, huh? Then they said I should run my maintenance plans more often and archive the data so there's no so much to search through.
Is there something in a front-end that would require this, something that makes it impossible to convert the literal to datetime? This isn't a stored procedure, it's coming from the front-end, so I can't change it. Or even create an index, since the datetime column is converted to varchar.
So - is it me or is it them? 🙂
Thanks,
Pat
July 23, 2012 at 5:47 pm
It's them.:cool:
Jason...AKA CirqueDeSQLeil
_______________________________________________
I have given a name to my pain...MCM SQL Server, MVP
SQL RNNR
Posting Performance Based Questions - Gail Shaw[/url]
Learn Extended Events
July 23, 2012 at 5:48 pm
For the record, take the query changes with the timing comparisons and show them outright. They probably won't change anything but you will at least have documentation at the attempt to get them to change it to something better.
Jason...AKA CirqueDeSQLeil
_______________________________________________
I have given a name to my pain...MCM SQL Server, MVP
SQL RNNR
Posting Performance Based Questions - Gail Shaw[/url]
Learn Extended Events
July 23, 2012 at 6:00 pm
declare @datetime datetime = '2011-7-13 2:39:39 PM'
if convert(varchar,@datetime,120) = convert(varchar, '2011-7-13 14:39:39', 120)
print 1
else print 2
declare @datetime2 datetime = '2011-7-13 14:39:39'
if @datetime2 = convert(varchar, '2011-7-13 14:39:39', 120)
print 3
else
print 4
Hmmmmm ????
July 23, 2012 at 9:59 pm
There's just no reason to convert the DATETIME based column to VARCHAR to do the comparison. Worse yet, the conversion makes the WHERE clause non-SARGable. For those that don't know exactly what that means, it means that it won't use and can't use an INDEX SEEK. The best it'll do is an INDEX SCAN in this case.
--Jeff Moden
Change is inevitable... Change for the better is not.
July 24, 2012 at 1:30 am
SQLRNNR (7/23/2012)
It's them.:cool:
+1
Need an answer? No, you need a question
My blog at https://sqlkover.com.
MCSE Business Intelligence - Microsoft Data Platform MVP
July 24, 2012 at 1:58 am
I had a similair situation here.
I created a test table with roughly 3 million random dates in it and then ran the existing code and revised code against this. Then sent an email out to the other developers with the examples and statistics. The revised code was 6 - 12 times faster in all cases.
Having proved that formatting anywhere other than in the SELECT area is a bad idea (and we have to have it there bacause the interface doesn't do any formatting of dates) we should see less and less of this and the speed of the processes should improve.
When making efficiency claims, always provide examples with proof (and be polite!). It shows that you know how to present your case and that your code is an improvement over what is currently in use.
July 24, 2012 at 7:20 am
Well thanks, everyone, I appreciate the responses! While I'd like to do database stuff all day long, working at a small business means in one day I can go from delving into SQL to figuring out Group Policy, troubleshooting roaming profiles and Active Directory, trying to keep up with updates on Windows Server Update Services and swapping out printers. So I know my SQL skills haven't been exercised as much as they should, and wasn't sure if I might have missed something.
I did send screen shots of the original and modified query results in SSMS with the query times shown. However, the only contact I have is the standard customer support line. How far up the line any question or request goes depends on whichever support person you happen to get, some are more responsive than others. It's like calling your Internet Provider - you can be a crack telecommunications engineer and know exactly where the problem is, but when you call you still get the same "OK, is your computer plugged in to the modem?" treatment. :crazy:
July 25, 2012 at 10:58 am
miapjp (7/24/2012)
Well thanks, everyone, I appreciate the responses! While I'd like to do database stuff all day long, working at a small business means in one day I can go from delving into SQL to figuring out Group Policy, troubleshooting roaming profiles and Active Directory, trying to keep up with updates on Windows Server Update Services and swapping out printers. So I know my SQL skills haven't been exercised as much as they should, and wasn't sure if I might have missed something.I did send screen shots of the original and modified query results in SSMS with the query times shown. However, the only contact I have is the standard customer support line. How far up the line any question or request goes depends on whichever support person you happen to get, some are more responsive than others. It's like calling your Internet Provider - you can be a crack telecommunications engineer and know exactly where the problem is, but when you call you still get the same "OK, is your computer plugged in to the modem?" treatment. :crazy:
Since this issue was introduced after a recent upgrade, then it should be fresh on the developer's mind why they did it that way. Even if the pupose of the query is to return rows that were inserted between a window of time, there is no reason to convert the dates to strings for the comparison. Their advice about adding indexes or updating statistics will not improve this query; it is non-indexable and will always perform a table scan.
"Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho
July 25, 2012 at 11:45 am
The reason they're doing what they're doing is that they don't know how to properly deal with times in DateTime columns. Format 120 strips the time out. They probably don't know the correct ways to deal with that kind of data.
If the database were SQL 2008 or later, they could convert both sides to Date instead of DateTime, and it would do what they need. I've tested, and that still ends up SARGable, regardless of the function-use. If you're on SQL 2005, as per the forum you posted in, that solution isn't available, but others are.
The simplest solution in SQL 2000/2005 is to use >= on the prior midnight, and < the following midnight, on any DateTime value. Then you get the same day, and it can use indexes properly. I suggest letting them know that.
Simply leaving the DateTime value from the UI alone, with a time-value on it, and doing an equality comparison, won't actually get the same results as what their query is designed to get. It'll be faster, but it'll be wrong, and that's worse than right-but-slow.
- 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
July 25, 2012 at 12:02 pm
Where convert(varchar, a.submitted, 120) = convert(varchar, '2011-07-07 14:39:39', 120)
Style 120 actually doesn't strip out the time portion of the datetime, so what they're doing makes no sense. I doubt the code would even consistently return the intended result. Aside from poor performance, it's probably a bug.
select convert(varchar,getdate(),120);
2012-07-25 13:56:14
"Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho
July 25, 2012 at 12:17 pm
Eric M Russell (7/25/2012)
Where convert(varchar, a.submitted, 120) = convert(varchar, '2011-07-07 14:39:39', 120)
Style 120 actually doesn't strip out the time portion of the datetime, so what they're doing makes no sense. I doubt the code would even consistently return the intended result. Aside from poor performance, it's probably a bug.
select convert(varchar,getdate(),120);
2012-07-25 13:56:14
You're right! I was thinking of 112, not 120.
- 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
August 6, 2012 at 1:16 pm
GSquared (7/25/2012)
The reason they're doing what they're doing is that they don't know how to properly deal with times in DateTime columns. Format 120 strips the time out. They probably don't know the correct ways to deal with that kind of data..
.
.
Simply leaving the DateTime value from the UI alone, with a time-value on it, and doing an equality comparison, won't actually get the same results as what their query is designed to get. It'll be faster, but it'll be wrong, and that's worse than right-but-slow.
Hey GSquared (or anyone who knows the answer) - I have a question about the last line in your response. Do you mean I could get the wrong answers by casting the literal with the time value as a datetime and comparing it to the database data ? Would that be because the literal doesn't have the milliseconds part? (for the record, they're all "000" in the database (at this time, anyway!))
By the way, a user contacted the software company, and they gave her instructions to rebuild an index - on a table that's not even in the query used by that function :w00t:
Thanks,
Pat
August 6, 2012 at 1:31 pm
If any component of the data isn't an exact match, an equality comparison will fail. I wrote that bit when I thought they were looking for whole-day-ranges (my 112 vs 120 misread).
- 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
August 6, 2012 at 2:17 pm
"..Their support wrote back to me with a script to add some indexes on some columns. Of course, the columns weren't in this particular query, but hey, adding an index must be good, huh? Then they said I should run my maintenance plans more often and archive the data so there's no so much to search through..."
I'm just shocked that they made any attempt to address the issue at all, even as bad as the advice is.
At least they didn't tell you to upgrade the hardware or to upgrade to the latest version of their software.
Viewing 15 posts - 1 through 15 (of 15 total)
You must be logged in to reply to this topic. Login to reply