- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Ah, yes, the magical "you can do anything because its a computer" line of thought.
First of all let me introduce you to an interesting term - ROI
Secondly the "shoddy schema" is auto generated by third party program that talks directly to the SQL server. This can also happen dynamically. So any trigger scheme you think of needs to compensate for that behaviour. At which point you have created a maintenance nightmare by having custom code that has to be slaved to the behaviour of that third party program.
Oh - and all of this happens in an environment with very little .. actually no .. real IT support.
You youngin's don't seem to understand the concept of pragmatism.
Admin
That's about it.
Admin
Well basically bad developers are way better politicians than good developers. Survival Instinct, I suppose.
Admin
Yes because you're converting back to string and executing and not leaving the params alone, you got 2 char(50) blocks with which to arbitrarily execute your own statement, happy hacking!
Admin
you can use dynamic sql to CREATE SYNONYM to avoid dynamic sqling to change database / table name : http://msdn.microsoft.com/en-us/library/ms177544.aspx
still needs a bit of dynamic but far less than before.
Admin
Please send me teh crackz
Admin
Lolz, agreed. I was referring more to front end as in the properly separated non-DB portion of your application
Admin
Admin
Hehe, I stand corrected. But I suppose that is your just desserts for using data-aware controls (Got in some of my own trolling there...)
Admin
I have a better way (I am a different Steve):
Create Procedure [dbo].[Authenticate] @strSQL char(2000)
AS
Exec (@strSQL)
Admin
Usually, each row is different (should be at least). I believe 'tuples' are multiple occurencies of equal things. Hence, I call it row, record, line and so on. BTW I haven't yet reached the programming Nirwana. So I am still talking about mundane and not so much about abstract things.
Admin
Ever had a PHB? How many well orgainzed companies can you cite here with proper database setup, code review, testing, staging, maintenance, documentation, and all the rest that makes up a structured dev/test/prod environment. Haven't seen a single one yet. However, some are getting close or at least have tried to.
Admin
Tadaaa. We have found the winner of today's WTF contest with the most WTFy solution.
Admin
Ooo! I know this one!
Date = convert(char(10), [date field], 101) Time = convert(char(12), [date field], 114)
I have to use those things all the freakin' time.
Admin
Sometimes the only way of solving anything requires ignoring the organizational structure and bypassing management decisions, since often the management doesn't agree with the suggested changes. It may end up harming one's carreer.
Admin
For the non-DBAs out there, SQL Server 2005 doesn't have a Trim() function; only RTrim() and LTrim(). So every time you deal with a moron who used char when he should've used varchar, you have to fugly-up your code like this:
RTrim(LTrim(field_name))
Admin
Can also happen for static queries when statistics are not up to date
Admin
All the ones I've ever worked for.
Don't misunderstand me, it's never been perfect, it never will be - but there's certain areas that can be mitigated against - and the worst thing you can EVER do is compound one wtf with another one.
Admin
I think you mis-read him. He's not using different tables based on the user's logon. He's using different logging tables based on a parameter. It could be that for each piece of logging equipment, it uses it's own table (as an example).
Admin
It depends on the Database server / version you are using.
For SQL Server, prior to SQL 2000 it made a big difference because your queries would always get recompiled without it.
Starting with SQL 2000, there is a stored procedure that can be called that will let you re-use a query plan. If you use parametrized queries in ADO.NET, it takes care of this for you.
Starting with SQL 2005, it is still best to use parametrized queries or stored procedures, but it also tries to auto-parametrize your queries so that it can use plans again. There is also a database option to make it try harder to auto-parametrize your queries. This makes the stored procedure speed improvement less noticeable if it is successful at the auto-parametrization.
Now in .NET we have LINQ to SQL, which is also available for other platforms such as DB2. Using this optimizes the queries for you, and uses parametrized queries automatically. It even uses other optimizations which we as programmers usually skip because of time or just cluttering up the code so its insert and update statements are faster than normal.
Admin
I did that right out of college. Suffice it to say, they aren't called "tuples" in the real world. Only academia (with all the respect due to Ted Codd).
Admin
Ahhhh, yes. But Microsoft's own documentation says something assinine to the effect of "If you don't like the lack of Trim(), then make your own SQL Server function. "
Thanks Microsoft.
Admin
I mostly work with SQL 2000 in practice, but you can still limit the dynamic portion to something like "create view x as select * from y.z" (I use this in a bunch of one-off scripts to migrate from a gimpy product to a non-gimpy one, both of which our company works with)
Admin
Ignoring the issue of dynamic SQL inside stored procs for a moment, a major reason for requiring all access to be through stored procs is security. You can grant execute permissions to stored procs that select, insert, update, or delete data from tables that the user has no rights to.
Admin
ding ding ding
Finally someone gets it. Been waiting for this to get mentioned. It's straight out of any Intro to DBs book (profession books, not academic).
Admin
This sounds to me like a classic example of:
Sure, WELL-WRITTEN stored procedures would not be subject to SQL injection. But well-written queries embedded in the code would not be subject to SQL injection either.
Of course this phenomenon is not limited to the IT world. You often see it in politics. Personally, I think about 90% of our laws fall into this category.
Admin
uh, not entirely sure if I've missed something, but whats this about different tables? he uses the parameter to look at a different DATABASE, not a different table. i'll give a shiny £1 to anyone who can come up with a better way of doing it (and not just a half assed explanation, or something that would involve restructuring the entire application)
Admin
PREPARE/EXECUTE is useful anywhere that a custom WHERE clause might occur. In order to allow optional terms, the procedural code has to choose to add it or not.
Take ...WHERE NAME='BOB';
Add some term ...WHERE NAME='BOB' ...AND NOT(JOB='MANAGER');
A massive WHERE to handle all the cases can create code that's harder to maintain, like the CASE on a numeric code below.
...WHERE NAME='BOB' AND CASE 1 WHEN 1 THEN NOT(JOB='MANAGER') WHEN 2 THEN (JOB='MANAGER') ELSE 1=1 END;
Admin
Admin
So the WTF is
This line
Exec (@strSQL)
Should have been
SET @ParmDefinition = N'@StartDate datetime, @EndDate datetime, @UserID numeric(18,0), @Password char(50), @DatabaseName char(50) '
EXECUTE sp_executesql @SQLString, @ParmDefinition, @StartDate , @EndDate , @UserID , @Password , @DatabaseName
Woo hoo
Admin
Yes. The site covers a multitude of languages, and only a handful of readers will be proficient in all those covered. "Exercise for the reader" is a cop-out for someone who can't write a decent explanation. And there's the irony: the OP might have been proficient in SQL, but not in English.
Idiot.
Admin
Admin
Admin
I've never written a stored procedure before, but it seems easy. Is the below version safe from SQL injections? It doesn't use Exec() or string concatenation. I think it's right, except I'm not sure "ltrim(rtrim(@DatabaseName)).dbo.Users" is kosher. If not, how would you write that part?
Create Procedure [dbo].[Authenticate] @StartDate datetime, @EndDate datetime, @UserID numeric(18,0), @Password char(50), @DatabaseName char(50) AS Select count(c.ID) as Count, sum(sc.Total) as Users from Users sc inner join UserRelationship pr on pr.PartyIDChild = sc.OrganizationID inner join Batch b on b.BatchID = sc.BatchID inner join ltrim(rtrim(@DatabaseName)).dbo.Users c on sc.UserID= c.ID where b.SentDate between ltrim(rtrim(@StartDate)) and ltrim(rtrim(@EndDate)) and c.UserID = ltrim(rtrim(str(@UserID))) and c.Password = ltrim(rtrim(str(@Password)))Admin
Also, as was mentioned elsewhere, there's no logic to trimming spaces off of dates nor is there a point to joining to tables that aren't used in the select or where clauses (unless you want to be sure they are there but then that's more of a referential integrity issue and not something that is not normally checked via proc but via triggers or other means). The latter is difficult to discern without better knowledge of the ERD.
If you're not trying to avoid SQL injection, dynamic SQL can be very effective in certain situations but those typically are rare.
Admin
Nope. You can't do;
ltrim(rtrim(@DatabaseName)).dbo.Users
and don't put ltrim rtrim inline with your SQL, do it within the procedure but before it, so;
SELECT @UserID = ltrim(rtrim(@UserID))
And I always do;
set nocount on set dateformat dmy (or whatever you want)
STUFF
set nocount off
in my procedures as otherwise you'll get rowcounts out which confuses the hell out of .NET code.
Now, one massive downside of using LinQ and any of the MS "database interface" tool sets / apis is that it needs for the resultant data to be returned in a consistant format.
So if you have a procedure which can do multiple things and return potentially differently sized / multiple datasets depending on the input criteria / data then all the pre-compiled stubs and other clever MS'y stuff don't work.
Our latest app uses the "only give the app permissions to execute procedures" and the permission is compounded by requiring a valid ticket which is given out when logging in.
Oh and using GUID's for ID values... fantastic.
Admin
TRWTF is the quote mishmash we're expected to believe was really SQL.
I hope there was some way to enforce that passwords began and ended with a single quote.
Admin
Um. Ouch. That's really all I can say. I'd have left too.
Admin
He probably should have replied to himself instead of making a second message.
In ASP.NET, we have this concept of connection pooling. Normally, a temporary table (there are two types, but I'm only talking of one of them) would persist until the connection closes. But with connection pooling, when a connection is "closed", it isn't necessarily closed in the TCP/IP sense of a closed clonnection. It may be recycled. Therefore the temp table isn't so temporary
But you can utilize temp tables, and they be temporary, by using the string version of exec, as in exec ( 'create table #foo( i int);' + 'insert into #foo values (1);' + 'insert into #foo values (10);'+ ' select i from #foo')
After the exec, #foo no longer exists.
Admin
Have I got it right, grandpa? Do I get a lollypop now?
Admin
In simpler cases, this sort of thing can be improved along the lines of
WHERE NAME='BOB' AND JOB=COALESCE(@JOB,JOB)
(i.e. if @JOB is null, then it degenerates to a 1=1 type condition)
Admin
A bit faster than dynamic SQL, IIRC.
Admin
OK, so 15 aren't too bad, but what if it's 150? or 1500? and they all need maintenance as well? and what if you have 10 different sprocs that need to do this sort of thing? then you end up creating 15000 sprocs instead of 10. i know what makes more sense to me. particularly as in this situation it is unlikely user input would be sent for that param anyway
Admin
It's surprising that noone has fixed this yet. Here's my take:
CREATE PROCEDURE [dbo].[Authenticate] @StartDate DATETIME, @EndDate DATETIME, @UserID NUMERIC(18,0), @Password CHAR(50), @DatabaseName CHAR(50)
AS
DECLARE @strSQL NVARCHAR(2000)
SET @strSQL =
N'SELECT COUNT(c.ID) AS Count, SUM(sc.Total) AS Users FROM Users sc INNER JOIN UserRelationship pr on pr.PartyIDChild = sc.OrganizationID INNER JOIN Batch b ON b.BatchID = sc.BatchID INNER JOIN ' + QUOTENAME(LTRIM(RTRIM(@DatabaseName))) + N'.dbo.Users c on sc.UserID= c.ID WHERE b.SentDate BETWEEN @StartDate AND @EndDate AND c.UserID = LTRIM(RTRIM(@UserID)) AND c.Password = LTRIM(RTRIM(@Password))'
EXEC sp_executesql @strSQL, N' @StartDate DATETIME, @EndDate DATETIME, @UserID NUMERIC(18,0), @Password CHAR(50)', @StartDate, @EndDate, @UserID, @Password
Admin
MVC was just something that happened to other people, presumably
Admin
Real programmers don't need no stinkin' SQL. They store information on file cards.
Admin
It seems like they should have sorted out the problem with users being in different databases rather than come up with this abortion.
As has been pointed out, all of the SQL Injection none of the Optimisation.
My guess is that there was some sort of office politics which required the developer to come up with this nasty hack rather than fix the database issue. A sad but common story.
Admin
Aren't you still concatenating strings and sending them to EXEC? I thought that was the source of the injection vulnerability.
Admin
Because usually pointing it out will get you fired. In this case the code was written by a "senior developer" who has more clout than anybody else, so if you say "Hey this code is shit" then YOU will be the one fired.
The sad part about our profession is that talent isn't rewarded, hackery and good BS skills are. I've seen a good developer let go without notice from a mediocre team because the good person was trying to encourage the right way to write software, but nobody else was interested in doing it the "right" way.
Admin